From 8315b1a7ac019eb4d5c7574b624675de452d7779 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Tue, 10 Mar 2026 16:20:40 +0000 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 | 6 ++++-- plugins/amdgpu/amdgpu_plugin_topology.c | 21 ++++++++++----------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/plugins/amdgpu/amdgpu_plugin.c b/plugins/amdgpu/amdgpu_plugin.c index 51a1f694f..d73809e64 100644 --- a/plugins/amdgpu/amdgpu_plugin.c +++ b/plugins/amdgpu/amdgpu_plugin.c @@ -1628,7 +1628,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); @@ -1936,7 +1937,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; } diff --git a/plugins/amdgpu/amdgpu_plugin_topology.c b/plugins/amdgpu/amdgpu_plugin_topology.c index 730f2e028..cb861d50d 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]\n", + 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; }