From 69c734eaa2509e615d13fcd81d45fed3ae9ccb7e Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Fri, 10 Apr 2026 19:55:06 +0100 Subject: [PATCH] plugins/amdgpu: Fix open_drm_render_device() Open_drm_render_device() is a little bit confused. It can log confusing errors using errno which hasn't been set, or it has been overwritten. It can also inspect errno after it had reset it creating effectively unreachable code. And finally it sometimes returns the negative errno, and sometimes a plain -1. Fix it all up and make it consistent, while at the same time correcting two calls site incorrectly using pr_perror(), while the function does not guarantee a valid errno. Signed-off-by: Tvrtko Ursulin Reviewed-By: David Francis --- plugins/amdgpu/amdgpu_plugin.c | 9 +++++---- plugins/amdgpu/amdgpu_plugin_topology.c | 21 ++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/plugins/amdgpu/amdgpu_plugin.c b/plugins/amdgpu/amdgpu_plugin.c index 2f63dcfa3..f2d1bcf44 100644 --- a/plugins/amdgpu/amdgpu_plugin.c +++ b/plugins/amdgpu/amdgpu_plugin.c @@ -1599,7 +1599,8 @@ static int restore_devices(struct kfd_ioctl_criu_args *args, CriuKfd *e) drm_fd = node_get_drm_render_device(tp_node); if (drm_fd < 0) { - pr_perror("Can't open drm render fd %d", tp_node->drm_render_minor); + pr_err("Can't open drm render fd for minor %d - %s", + tp_node->drm_render_minor, strerror(-drm_fd)); goto exit; } else { pr_info("passing drm render fd = %d to driver\n", drm_fd); @@ -1907,7 +1908,8 @@ int amdgpu_plugin_restore_file(int id, bool *retry_needed) fd = node_get_drm_render_device(tp_node); if (fd < 0) { - pr_err("Failed to open render device (minor:%d)\n", tp_node->drm_render_minor); + pr_err("Failed to open render device (minor:%d) - %s\n", + tp_node->drm_render_minor, strerror(-fd)); return -1; } @@ -2201,9 +2203,8 @@ int init_dev(int dev_minor, amdgpu_device_handle *h_dev, uint64_t *max_copy_size struct amdgpu_gpu_info gpu_info = { 0 }; drm_fd = open_drm_render_device(dev_minor); - if (drm_fd < 0) { + if (drm_fd < 0) return drm_fd; - } ret = amdgpu_device_initialize(drm_fd, &major, &minor, h_dev); if (ret) { diff --git a/plugins/amdgpu/amdgpu_plugin_topology.c b/plugins/amdgpu/amdgpu_plugin_topology.c index 730f2e028..c576cdfa4 100644 --- a/plugins/amdgpu/amdgpu_plugin_topology.c +++ b/plugins/amdgpu/amdgpu_plugin_topology.c @@ -51,31 +51,30 @@ int open_drm_render_device(int minor) int fd, ret_fd; if (minor < DRM_FIRST_RENDER_NODE || minor > DRM_LAST_RENDER_NODE) { - pr_perror("DRM render minor %d out of range [%d, %d]", minor, DRM_FIRST_RENDER_NODE, - DRM_LAST_RENDER_NODE); + pr_err("DRM render minor %d out of range [%d, %d]", + minor, DRM_FIRST_RENDER_NODE, DRM_LAST_RENDER_NODE); return -EINVAL; } snprintf(path, sizeof(path), "/dev/dri/renderD%d", minor); fd = open(path, O_RDWR | O_CLOEXEC); if (fd < 0) { - if (errno != ENOENT && errno != EPERM) { - pr_err("Failed to open %s: %s\n", path, strerror(errno)); - if (errno == EACCES) - pr_err("Check user is in \"video\" group\n"); - } - return -EBADFD; + fd = -errno; + pr_perror("Failed to open %s", path); + return fd; } if (fd_next < 0) return fd; ret_fd = fcntl(fd, F_DUPFD, fd_next++); + if (ret_fd < 0) { + ret_fd = -errno; + pr_perror("Failed to duplicate fd for minor:%d (fd_next:%d)", + minor, fd_next); + } close(fd); - if (ret_fd < 0) - pr_perror("Failed to duplicate fd for minor:%d (fd_next:%d)", minor, fd_next); - return ret_fd; }