Error logging for just a single error condition will eat the errno so lets
just remove it. All callers use pr_perror() and rely on the helper not
munching it up so no information is lost by removing the internal log.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
We remove some function entry and exit markers, some interim messages
which provide no real value, and only log virtual address allocation
failures on the failure path. For the last one we also fix incorrect
usage of pr_perror.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
The extra debug build log helper does not seem very useful and some of
them do not even compile. Lets just remove it and replace with the
standard pr_debug. In case of too much noise, we can later re-evaluate
to remove some of the not very useful log messages.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Instead of open coding the same ioctl twice we can put it in a loop from
which we break out once we have allocated enough space for all objects.
While at it we add error handling for the memory allocation.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
---
v2:
* s/for(;;)/while(1)/ (David)
Both CRIU and plugin are built with 64-bit off_t and most of the parallel
bo restore path uses uint64_t for storing offsets.
Lets consolidate by changing int to off_t, and by doing so remove any
possibility that the call to fseeko can overflow the return type.
While at it, add error checking to both seek operations.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com> # v1
---
v2:
- Correct offset type as well.
Shallower indentation makes a heavily indented function hopefully a bit
more readable.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Consolidate to one fclose() call with the extra bonus the inconsequential
file descriptor leak is eliminated.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Close the directory when probing images during restore init fails.
While at it, consolidate the identical code between the DRM and KFD images
into a common helper.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Currently the failure to save content of a buffer object is only logged,
while the checkpointing will continue unaware due the error code being
"eaten" inside sdma_copy_bo() cleanup path, together with the caller not
checking the return value to begin with.
Fix it by preserving and returning the original error code inside
sdma_copy_bo(), and abort the checkpointing process in case of a failure.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Currently failure to open files logs an unitialized stack string as the
failed path. Lets just log the basename and drop the unused stack buffer.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Size needs to be initialized before opening the file for write, otherwise
garbage is written into the file.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
The new_minor field in struct vma_metadata is effectively unused.
Remove it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Remove unused current_pid global variable.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Instead of logging a zero failure code lets log the actual GPU id which
was not found.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-By: David Francis <David.Francis@amd.com>
Check that the image unpacking worked before deferencing the returned
pointer during plugin init in order to fail gracefully.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Allocate shared memory for the whole struct shared_handle_ids and not just
for the pointer to it, and by doing so stop relying on mmap rounding up to
page size.
While at it add error checking that the allocations actually succeeded.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Adjusted object sizes need to be used to unmap on the cleanup path so that
the unmap operations match with map for the case sizes were adjusted in
the function prologue.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
v2:
* Improve commit message and use adjusted size in error logs too.
The function was not closing the drm fd duplicated by the libdrm call,
could attempt to deinitialize the libdrm device which wasn't successfully
openened, and was using errno based log functions when libdrm functions
used return a negative errno. Fix it all.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Close the file when either memory allocation or a file read fails in
amdgpu_plugin_dmabuf_restore.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Docstring refers to size_t as 8-byte value while in reality this is
architecture dependent. Clarify it.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
In amdgpu_plugin_drm_restore_file(), dmabuf_fd can reach the
drmPrimeFDToHandle() call as -1 if fdstore_get() fails or if
boinfo->handle is -1 while boinfo->is_import is true. Passing
an invalid fd to libdrm causes a hard restore failure instead
of a retry. Check for -1 and set retry_needed so the restore
coordinator can retry later.
When retry_needed is set, some dmabufs entries are still invalid,
so skip record_completed_work() and restore_bo_contents_drm()
to avoid operating on incomplete state.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Several error paths in amdgpu_plugin_drm_dump_file() leak
resources. The drmPrimeHandleToFD() and open_img_file() failures
use break instead of goto exit, which skips per-iteration cleanup
and lets the function continue dumping metadata with a stale ret
value. Replace break with proper resource cleanup and goto exit.
The vm_info_entries buffer is not freed when the ioctl or
allocate_vm_entries() calls fail after allocating it, and the
buffer from posix_memalign() is never freed after sdma_copy_bo(),
leaking memory on every loop iteration. Add the missing xfree()
calls.
Check the amdgpu_device_initialize() return value because h_dev
is undefined on failure. Move list_handles_entries cleanup into
the exit block because error paths that goto exit skip the
existing xfree() call after the loop. Add a NULL check before
free_e(rd) because rd allocation can fail before the struct is
initialized.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Initialize gpu_ids, fds_write, and entries to NULL before
allocating them in init_parallel_restore_cmd() and
init_parallel_restore_cmd_by_head(). The caller declares the
struct on the stack without an initializer, so these members
hold undefined values. If a later xzalloc() fails and the
caller invokes free_parallel_restore_cmd(), it will call
xfree() on those undefined pointers.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Call amdgpu_device_deinitialize() before continuing when
get_gem_handle() fails and before returning when a matching
handle is found. Both paths skip the existing deinitialize
call at the end of the loop body, leaking the device handle.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
open_img_file() can return NULL after a read_fp() or write_fp()
failure without setting errno. The openat() and fdopen() paths
set errno, but the read/write path does not, so callers using
-errno get zero or a stale value. Set errno to EIO on this path
so errno is always valid when the function returns NULL.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Close the file descriptor when fdopen() fails, as fdopen does not
take ownership of the fd on failure.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
The exit label in amdgpu_plugin_drm_restore_file() returns
immediately when ret < 0 without freeing the dmabufs array or
deinitializing the amdgpu device handle. Move cleanup before the
error check so resources are always released.
Replace xzalloc() with xmalloc() and memset(0xff) for the dmabufs
array because zero is a valid fd number and would cause the cleanup
loop to close fd 0 (stdin) for unpopulated entries. Initializing
to KFD_INVALID_FD (0xffffffff) lets the exit block tell which fds
were actually opened. Guard amdgpu_device_deinitialize() behind a
dev_initialized flag to avoid calling it when initialization failed,
and close dmabuf_fd when drmPrimeFDToHandle() fails. To prevent
double-closing fds that restore_bo_contents_drm() already closed,
only run the close loop when the BO content restore was not reached.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Free the protobuf message and buffer when returning early from
amdgpu_plugin_dmabuf_restore() on failure to find a dmabuf fd.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
posix_memalign() returns an error code on failure and does not
guarantee setting the pointer to NULL. Check the return value
instead of the pointer. Since posix_memalign() does not set
errno, assign it explicitly so pr_perror() prints the correct
message. Negate the return code instead of hardcoding -ENOMEM
to preserve the actual error (e.g. EINVAL for bad alignment).
In amdgpu_plugin_drm_dump_file(), replace break with cleanup
and goto exit so per-BO resources are freed on failure.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
When the amdgpu plugin is called for restore of a file img,
we don't initially know if it's a kfd or drm file. So we first
try to open a kfd file, and if that fails, we assume it's
a drm file. That failure leads to a lot of error messages like
Error amdgpu_plugin: amdgpu-kfd-141.img: Failed to open for read
For this normal behaviour.
Add an option on open_img_file to suppress this error when
we aren't sure if the file we're trying to open exists.
Signed-off-by: David Francis <David.Francis@amd.com>
open_img_file can return null if, for example, the dump file
isn't present. Tehre were three places where this wasn't checked.
Check it.
Signed-off-by: David Francis <David.Francis@amd.com>
pr_perror already appends a newline; the explicit \n in the
format string violates the lint rule and breaks CI.
Fixes: 555b257c5e77 ("plugin/amdgpu: Catch error for failure to open drm device")
Signed-off-by: Ahmed Elaidy <elaidya225@gmail.com>
kfd_criu_device_bucket.drm_fd is an unsigned int, so the comparison
device_bucket->drm_fd < 0
was always false.
Fix that, and also make the error message a bit more descriptive.
Signed-off-by: David Francis <David.Francis@amd.com>
In container setups, particularly k8s, cgroup may be used to
restrict what GPUs a process has access to. CRIU may then be used
to checkpoint that process while CRIU itself has full access to
all GPUs.
In this case, the kfd CRIU ioctl will return only a subset of the
ioctls visible through sysfs.
Don't write these devices or any links connected to them to the
tology dump files.
Signed-off-by: David Francis <David.Francis@amd.com>
As of Alpine Linux 3.19, musl libc no longer contains separate
fopen64(), fseeko64(), or ftello64() functions. This causes building
CRIU with amdgpu plugin to fail with the following error:
amdgpu_plugin.c: In function 'parallel_restore_bo_contents':
amdgpu_plugin.c:2286:17: error: implicit declaration of function 'fseeko64'; did you mean 'fseeko'? [-Wimplicit-function-declaration]
2286 | fseeko64(bo_contents_fp, entry->read_offset + offset, SEEK_SET);
| ^~~~~~~~
| fseeko
make[2]: *** [Makefile:31: amdgpu_plugin.so] Error 1
make[1]: *** [Makefile:363: amdgpu_plugin] Error 2
To fix this, add the missing $(DEFINES) to plugin builds, and since we
always compile with _FILE_OFFSET_BITS=64, we don't need the 64 suffix.
Fixes: #2826
Suggested-by: Andrei Vagin <avagin@google.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
On AMD Instinct MI300 systems, restoring a large GPU application can
fail because the checkpoint size is too large and the maximum value of
an offset (with integer type) is insufficient. This problem occurs when
the total size of all buffer objects exceeds int max, not because any
single buffer is too large, but it can also happen with a large number
of small buffers.
Fixes: #2812
Signed-off-by: Yanning Yang <yangyanning@sjtu.edu.cn>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
This patch updates drm.h and amdgpu_drm.h kernel headers,
and adds drm_mode.h (included by drm.h) from the rocm-7.1.0
release tag.
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
amdgpu_plugin_drm.c:167:6: error: variable 'num_bos' set but not used [-Werror,-Wunused-but-set-variable]
167 | int num_bos = 0;
|
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Add a comment that explains the purpose of `retry_needed`.
Co-authored-by: Andrei Vagin <avagin@google.com>
Signed-off-by: David Francis <David.Francis@amd.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Co-authored-by: Andrei Vagin <avagin@google.com>
Signed-off-by: David Francis <David.Francis@amd.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Use `return 0` on success in `post_dump_dmabuf_check()` for consistency
with other functions.
Co-authored-by: Andrei Vagin <avagin@google.com>
Signed-off-by: David Francis <David.Francis@amd.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
These pr_info lines begin with "CC3" and "TWI" were not meant to be
included in the patch.
Co-authored-by: Andrei Vagin <avagin@google.com>
Signed-off-by: David Francis <David.Francis@amd.com>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
amdgpu libraries that use dmabuf fd to share GPU memory between
processes close the dmabuf fds immediately after using them.
However, it is possible that checkpoint of a process catches one
of the dmabuf fds open. In that case, the amdgpu plugin needs
to handle it.
The checkpoint of the dmabuf fd does require the device file
it was exported from to have already been dumped
To identify which device this dmabuf fd was exprted from, attempt
to import it on each device, then record the dmabuf handle
it imports as. This handle can be used to restore it.
Signed-off-by: David Francis <David.Francis@amd.com>
The amdgpu plugin was counting how many files were checkpointed
to determine when it should close the device files.
The number of device files is not consistent; a process may
have multiple copies of the drm device files open.
Instead of doing this counting, add a new callback after all
files are checkpointed, so plugins can clean up their
resources at an appropriate time.
Signed-off-by: David Francis <David.Francis@amd.com>
Buffer objects held by the amdgpu drm driver are checkpointed with
the new BO_INFO and MAPPING_INFO ioctls/ioctl options. Handling
is in amdgpu_plugin_drm.h
Handling of imported buffer objects may require dmabuf fds to be
transferred between processes. These occur over fdstore, with the
handle-fstore id relationships kept in shread memory. There is a
new plugin callback: RESTORE_INIT to create the shared memory.
During checkpoint, track shared buffer objects, so that buffer objects
that are shared across processes can be identified.
During restore, track which buffer objects have been restored. Retry
restore of a drm file if a buffer object is imported and the
original has not been exported yet. Skip buffer objects that have
already been completed or cannot be completed in the current restore.
So drm code can use sdma_copy_bo, that function no longer requires
kfd bo structs
Update the protobuf messages with new amdgpu drm information.
Signed-off-by: David Francis <David.Francis@amd.com>
The amdgpu plugin usually calls drm ioctls through the libdrm
wrappers. However, amdgpu restore requires dealing with dmabufs
and gem handles directly, which means drm ioctls must be
called directly.
Add the drm.h header (from the kernel's uapi).
Signed-off-by: David Francis <David.Francis@amd.com>
For amdgpu plugin to call the new amdgpu drm CRIU ioctls, it needs
the amdgpu drm header file, copied from the kernel's includes.
Signed-off-by: David Francis <David.Francis@amd.com>
