render: replace wlr_renderer_get_drm_fd() with drm_dev_id

wlr_renderer_get_drm_fd() forces the renderer to open the DRM node
and keep the FD around for no good reason. Instead, use dev_t to
refer to a DRM device.

This also removes footguns related to FD ownership.

include/render/egl.h

@@ -88,7 +88,10 @@ const struct wlr_drm_format_set *wlr_egl_get_dmabuf_render_formats(
  */
 bool wlr_egl_destroy_image(struct wlr_egl *egl, EGLImageKHR image);
 
-int wlr_egl_dup_drm_fd(struct wlr_egl *egl);
+/**
+ * Get the DRM device ID used by EGL.
+ */
+bool wlr_egl_get_drm_dev_id(struct wlr_egl *egl, dev_t *devid);
 
 /**
  * Restore EGL context that was previously saved using wlr_egl_save_current().

include/render/gles2.h

@@ -41,8 +41,8 @@ struct wlr_gles2_tex_shader {
 struct wlr_gles2_renderer {
 	struct wlr_renderer wlr_renderer;
 
+	dev_t drm_dev_id;
 	struct wlr_egl *egl;
-	int drm_fd;
 
 	struct wlr_drm_format_set shm_texture_formats;
 

include/render/vulkan.h

@@ -38,8 +38,6 @@ struct wlr_vk_device {
 	VkPhysicalDevice phdev;
 	VkDevice dev;
 
-	int drm_fd;
-
 	bool implicit_sync_interop;
 	bool sampler_ycbcr_conversion;
 
@@ -66,7 +64,7 @@ struct wlr_vk_device {
 // Tries to find the VkPhysicalDevice for the given drm fd.
 // Might find none and return VK_NULL_HANDLE.
 VkPhysicalDevice vulkan_find_drm_phdev(struct wlr_vk_instance *ini, int drm_fd);
-int vulkan_open_phdev_drm_fd(VkPhysicalDevice phdev);
+bool vulkan_get_phdev_drm_dev_id(VkPhysicalDevice phdev, dev_t *dev_id);
 
 // Creates a device for the given instance and physical device.
 struct wlr_vk_device *vulkan_device_create(struct wlr_vk_instance *ini,
@@ -231,6 +229,7 @@ struct wlr_vk_renderer {
 	struct wlr_renderer wlr_renderer;
 	struct wlr_backend *backend;
 	struct wlr_vk_device *dev;
+	dev_t drm_dev_id;
 
 	VkCommandPool command_pool;
 

include/wlr/render/interface.h

@@ -25,7 +25,6 @@ struct wlr_renderer_impl {
 	const struct wlr_drm_format_set *(*get_render_formats)(
 		struct wlr_renderer *renderer);
 	void (*destroy)(struct wlr_renderer *renderer);
-	int (*get_drm_fd)(struct wlr_renderer *renderer);
 	struct wlr_texture *(*texture_from_buffer)(struct wlr_renderer *renderer,
 		struct wlr_buffer *buffer);
 	struct wlr_render_pass *(*begin_buffer_pass)(struct wlr_renderer *renderer,

include/wlr/render/wlr_renderer.h

@@ -29,6 +29,8 @@ struct wlr_renderer {
 	// Capabilities required for the buffer used as a render target (bitmask of
 	// enum wlr_buffer_cap)
 	uint32_t render_buffer_caps;
+	// DRM device used for rendering, if any
+	const dev_t *drm_dev_id;
 
 	struct {
 		struct wl_signal destroy;
@@ -78,13 +80,6 @@ bool wlr_renderer_init_wl_display(struct wlr_renderer *r,
 bool wlr_renderer_init_wl_shm(struct wlr_renderer *r,
 	struct wl_display *wl_display);
 
-/**
- * Obtains the FD of the DRM device used for rendering, or -1 if unavailable.
- *
- * The caller doesn't have ownership of the FD, it must not close it.
- */
-int wlr_renderer_get_drm_fd(struct wlr_renderer *r);
-
 /**
  * Destroys the renderer.
  *

render/allocator/allocator.c

@@ -150,11 +150,40 @@ struct wlr_allocator *wlr_allocator_autocreate(struct wlr_backend *backend,
 	uint32_t backend_caps = backend_get_buffer_caps(backend);
 	// Note, drm_fd may be negative if unavailable
 	int drm_fd = wlr_backend_get_drm_fd(backend);
-	if (drm_fd < 0) {
+
-		drm_fd = wlr_renderer_get_drm_fd(renderer);
+	int render_drm_fd = -1;
+	if (drm_fd < 0 && renderer->drm_dev_id != NULL) {
+		drmDevice *dev = NULL;
+		if (drmGetDeviceFromDevId(*renderer->drm_dev_id, 0, &dev) != 0) {
+			wlr_log(WLR_ERROR, "drmGetDeviceFromDevId failed");
+			return NULL;
+		}
+
+		const char *node_name = NULL;
+		if (dev->available_nodes & (1 << DRM_NODE_RENDER)) {
+			node_name = dev->nodes[DRM_NODE_RENDER];
+		} else {
+			assert(dev->available_nodes & (1 << DRM_NODE_PRIMARY));
+			wlr_log(WLR_DEBUG, "No DRM render node available, "
+				"falling back to primary node '%s'", dev->nodes[DRM_NODE_PRIMARY]);
+			node_name = dev->nodes[DRM_NODE_PRIMARY];
+		}
+		render_drm_fd = open(node_name, O_RDWR | O_NONBLOCK | O_CLOEXEC);
+		drmFreeDevice(&dev);
+		if (render_drm_fd < 0) {
+			wlr_log_errno(WLR_ERROR, "Failed to open DRM node %s", node_name);
+			return NULL;
+		}
+
+		drm_fd = render_drm_fd;
 	}
 
-	return allocator_autocreate_with_drm_fd(backend_caps, renderer, drm_fd);
+	struct wlr_allocator *alloc =
+		allocator_autocreate_with_drm_fd(backend_caps, renderer, drm_fd);
+	if (render_drm_fd >= 0) {
+		close(render_drm_fd);
+	}
+	return alloc;
 }
 
 void wlr_allocator_destroy(struct wlr_allocator *alloc) {

render/egl.c

@@ -5,6 +5,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <gbm.h>
+#include <sys/stat.h>
 #include <wlr/render/egl.h>
 #include <wlr/util/log.h>
 #include <wlr/util/region.h>
@@ -949,68 +950,53 @@ static char *get_render_name(const char *name) {
 	return render_name;
 }
 
-static int dup_egl_device_drm_fd(struct wlr_egl *egl) {
+static char *get_drm_node_path(struct wlr_egl *egl) {
 	if (egl->device == EGL_NO_DEVICE_EXT || (!egl->exts.EXT_device_drm &&
 			!egl->exts.EXT_device_drm_render_node)) {
-		return -1;
+		return NULL;
 	}
 
-	char *render_name = NULL;
 #ifdef EGL_EXT_device_drm_render_node
 	if (egl->exts.EXT_device_drm_render_node) {
 		const char *name = egl->procs.eglQueryDeviceStringEXT(egl->device,
 			EGL_DRM_RENDER_NODE_FILE_EXT);
 		if (name == NULL) {
 			wlr_log(WLR_DEBUG, "EGL device has no render node");
-			return -1;
+			return false;
 		}
-		render_name = strdup(name);
+		return strdup(name);
 	}
 #endif
 
-	if (render_name == NULL) {
+	const char *primary_name = egl->procs.eglQueryDeviceStringEXT(egl->device,
-		const char *primary_name = egl->procs.eglQueryDeviceStringEXT(egl->device,
+		EGL_DRM_DEVICE_FILE_EXT);
-			EGL_DRM_DEVICE_FILE_EXT);
+	if (primary_name == NULL) {
-		if (primary_name == NULL) {
+		wlr_log(WLR_ERROR,
-			wlr_log(WLR_ERROR,
+			"eglQueryDeviceStringEXT(EGL_DRM_DEVICE_FILE_EXT) failed");
-				"eglQueryDeviceStringEXT(EGL_DRM_DEVICE_FILE_EXT) failed");
+		return false;
-			return -1;
-		}
-
-		render_name = get_render_name(primary_name);
-		if (render_name == NULL) {
-			wlr_log(WLR_ERROR, "Can't find render node name for device %s",
-				primary_name);
-			return -1;
-		}
 	}
 
-	int render_fd = open(render_name, O_RDWR | O_NONBLOCK | O_CLOEXEC);
+	return get_render_name(primary_name);
-	if (render_fd < 0) {
+}
-		wlr_log_errno(WLR_ERROR, "Failed to open DRM render node %s",
+
-			render_name);
+bool wlr_egl_get_drm_dev_id(struct wlr_egl *egl, dev_t *dev_id) {
+	char *render_name = get_drm_node_path(egl);
+	// Fallback to GBM's FD if we can't use EGLDevice
+	if (render_name == NULL && egl->gbm_device != NULL) {
+		render_name = drmGetDeviceNameFromFd2(gbm_device_get_fd(egl->gbm_device));
+	}
+	if (render_name == NULL) {
+		return false;
+	}
+
+	struct stat render_stat = {0};
+	if (stat(render_name, &render_stat) != 0) {
+		wlr_log_errno(WLR_ERROR, "stat failed");
 		free(render_name);
-		return -1;
+		return false;
 	}
 	free(render_name);
 
-	return render_fd;
+	*dev_id = render_stat.st_rdev;
-}
+	return true;
-
-int wlr_egl_dup_drm_fd(struct wlr_egl *egl) {
-	int fd = dup_egl_device_drm_fd(egl);
-	if (fd >= 0) {
-		return fd;
-	}
-
-	// Fallback to GBM's FD if we can't use EGLDevice
-	if (egl->gbm_device == NULL) {
-		return -1;
-	}
-
-	fd = fcntl(gbm_device_get_fd(egl->gbm_device), F_DUPFD_CLOEXEC, 0);
-	if (fd < 0) {
-		wlr_log_errno(WLR_ERROR, "Failed to dup GBM FD");
-	}
-	return fd;
 }

render/gles2/renderer.c

@@ -183,17 +183,6 @@ static const struct wlr_drm_format_set *gles2_get_render_formats(
 	return wlr_egl_get_dmabuf_render_formats(renderer->egl);
 }
 
-static int gles2_get_drm_fd(struct wlr_renderer *wlr_renderer) {
-	struct wlr_gles2_renderer *renderer =
-		gles2_get_renderer(wlr_renderer);
-
-	if (renderer->drm_fd < 0) {
-		renderer->drm_fd = wlr_egl_dup_drm_fd(renderer->egl);
-	}
-
-	return renderer->drm_fd;
-}
-
 struct wlr_egl *wlr_gles2_renderer_get_egl(struct wlr_renderer *wlr_renderer) {
 	struct wlr_gles2_renderer *renderer =
 		gles2_get_renderer(wlr_renderer);
@@ -231,11 +220,6 @@ static void gles2_destroy(struct wlr_renderer *wlr_renderer) {
 	wlr_egl_destroy(renderer->egl);
 
 	wlr_drm_format_set_finish(&renderer->shm_texture_formats);
-
-	if (renderer->drm_fd >= 0) {
-		close(renderer->drm_fd);
-	}
-
 	free(renderer);
 }
 
@@ -356,7 +340,6 @@ static const struct wlr_renderer_impl renderer_impl = {
 	.destroy = gles2_destroy,
 	.get_texture_formats = gles2_get_texture_formats,
 	.get_render_formats = gles2_get_render_formats,
-	.get_drm_fd = gles2_get_drm_fd,
 	.texture_from_buffer = gles2_texture_from_buffer,
 	.begin_buffer_pass = gles2_begin_buffer_pass,
 	.render_timer_create = gles2_render_timer_create,
@@ -532,7 +515,6 @@ struct wlr_renderer *wlr_gles2_renderer_create(struct wlr_egl *egl) {
 
 	renderer->egl = egl;
 	renderer->exts_str = exts_str;
-	renderer->drm_fd = -1;
 
 	wlr_log(WLR_INFO, "Creating GLES2 renderer");
 	wlr_log(WLR_INFO, "Using %s", glGetString(GL_VERSION));
@@ -629,6 +611,10 @@ struct wlr_renderer *wlr_gles2_renderer_create(struct wlr_egl *egl) {
 			GL_DEBUG_TYPE_PUSH_GROUP_KHR, GL_DONT_CARE, 0, NULL, GL_FALSE);
 	}
 
+	if (wlr_egl_get_drm_dev_id(renderer->egl, &renderer->drm_dev_id)) {
+		renderer->wlr_renderer.drm_dev_id = &renderer->drm_dev_id;
+	}
+
 	push_gles2_debug(renderer);
 
 	GLuint prog;

render/vulkan/renderer.c

@@ -1294,11 +1294,6 @@ destroy_image:
 	return false;
 }
 
-static int vulkan_get_drm_fd(struct wlr_renderer *wlr_renderer) {
-	struct wlr_vk_renderer *renderer = vulkan_get_renderer(wlr_renderer);
-	return renderer->dev->drm_fd;
-}
-
 static struct wlr_render_pass *vulkan_begin_buffer_pass(struct wlr_renderer *wlr_renderer,
 		struct wlr_buffer *buffer, const struct wlr_buffer_pass_options *options) {
 	struct wlr_vk_renderer *renderer = vulkan_get_renderer(wlr_renderer);
@@ -1322,7 +1317,6 @@ static const struct wlr_renderer_impl renderer_impl = {
 	.get_texture_formats = vulkan_get_texture_formats,
 	.get_render_formats = vulkan_get_render_formats,
 	.destroy = vulkan_destroy,
-	.get_drm_fd = vulkan_get_drm_fd,
 	.texture_from_buffer = vulkan_texture_from_buffer,
 	.begin_buffer_pass = vulkan_begin_buffer_pass,
 };
@@ -2156,6 +2150,10 @@ struct wlr_renderer *vulkan_renderer_create_for_device(struct wlr_vk_device *dev
 	wl_list_init(&renderer->render_buffers);
 	wl_list_init(&renderer->pipeline_layouts);
 
+	if (vulkan_get_phdev_drm_dev_id(dev->phdev, &renderer->drm_dev_id)) {
+		renderer->wlr_renderer.drm_dev_id = &renderer->drm_dev_id;
+	}
+
 	if (!init_static_render_data(renderer)) {
 		goto error;
 	}
@@ -2221,15 +2219,6 @@ struct wlr_renderer *wlr_vk_renderer_create_with_drm_fd(int drm_fd) {
 		return NULL;
 	}
 
-	// Do not use the drm_fd that was passed in: we should prefer the render
-	// node even if a primary node was provided
-	dev->drm_fd = vulkan_open_phdev_drm_fd(phdev);
-	if (dev->drm_fd < 0) {
-		vulkan_device_destroy(dev);
-		vulkan_instance_destroy(ini);
-		return NULL;
-	}
-
 	return vulkan_renderer_create_for_device(dev);
 }
 

render/vulkan/vulkan.c

@@ -2,7 +2,6 @@
 #undef _POSIX_C_SOURCE
 #endif
 #include <assert.h>
-#include <fcntl.h>
 #include <math.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -364,7 +363,7 @@ VkPhysicalDevice vulkan_find_drm_phdev(struct wlr_vk_instance *ini, int drm_fd)
 	return VK_NULL_HANDLE;
 }
 
-int vulkan_open_phdev_drm_fd(VkPhysicalDevice phdev) {
+bool vulkan_get_phdev_drm_dev_id(VkPhysicalDevice phdev, dev_t *dev_id) {
 	// vulkan_find_drm_phdev() already checks that VK_EXT_physical_device_drm
 	// is supported
 	VkPhysicalDeviceDrmPropertiesEXT drm_props = {
@@ -376,38 +375,16 @@ int vulkan_open_phdev_drm_fd(VkPhysicalDevice phdev) {
 	};
 	vkGetPhysicalDeviceProperties2(phdev, &props);
 
-	dev_t devid;
 	if (drm_props.hasRender) {
-		devid = makedev(drm_props.renderMajor, drm_props.renderMinor);
+		*dev_id = makedev(drm_props.renderMajor, drm_props.renderMinor);
 	} else if (drm_props.hasPrimary) {
-		devid = makedev(drm_props.primaryMajor, drm_props.primaryMinor);
+		*dev_id = makedev(drm_props.primaryMajor, drm_props.primaryMinor);
 	} else {
 		wlr_log(WLR_ERROR, "Physical device is missing both render and primary nodes");
-		return -1;
+		return false;
 	}
 
-	drmDevice *device = NULL;
+	return true;
-	if (drmGetDeviceFromDevId(devid, 0, &device) != 0) {
-		wlr_log_errno(WLR_ERROR, "drmGetDeviceFromDevId failed");
-		return -1;
-	}
-
-	const char *name = NULL;
-	if (device->available_nodes & (1 << DRM_NODE_RENDER)) {
-		name = device->nodes[DRM_NODE_RENDER];
-	} else {
-		assert(device->available_nodes & (1 << DRM_NODE_PRIMARY));
-		name = device->nodes[DRM_NODE_PRIMARY];
-		wlr_log(WLR_DEBUG, "DRM device %s has no render node, "
-			"falling back to primary node", name);
-	}
-
-	int drm_fd = open(name, O_RDWR | O_NONBLOCK | O_CLOEXEC);
-	if (drm_fd < 0) {
-		wlr_log_errno(WLR_ERROR, "Failed to open DRM node %s", name);
-	}
-	drmFreeDevice(&device);
-	return drm_fd;
 }
 
 static void load_device_proc(struct wlr_vk_device *dev, const char *name,
@@ -452,7 +429,6 @@ struct wlr_vk_device *vulkan_device_create(struct wlr_vk_instance *ini,
 
 	dev->phdev = phdev;
 	dev->instance = ini;
-	dev->drm_fd = -1;
 
 	// For dmabuf import we require at least the external_memory_fd,
 	// external_memory_dma_buf, queue_family_foreign,
@@ -650,10 +626,6 @@ void vulkan_device_destroy(struct wlr_vk_device *dev) {
 		vkDestroyDevice(dev->dev, NULL);
 	}
 
-	if (dev->drm_fd > 0) {
-		close(dev->drm_fd);
-	}
-
 	wlr_drm_format_set_finish(&dev->dmabuf_render_formats);
 	wlr_drm_format_set_finish(&dev->dmabuf_texture_formats);
 	wlr_drm_format_set_finish(&dev->shm_texture_formats);

render/wlr_renderer.c

@@ -83,7 +83,7 @@ bool wlr_renderer_init_wl_display(struct wlr_renderer *r,
 	}
 
 	if (wlr_renderer_get_texture_formats(r, WLR_BUFFER_CAP_DMABUF) != NULL &&
-			wlr_renderer_get_drm_fd(r) >= 0 &&
+			r->drm_dev_id != NULL &&
 			wlr_linux_dmabuf_v1_create_with_renderer(wl_display, 4, r) == NULL) {
 		return false;
 	}
@@ -287,13 +287,6 @@ struct wlr_renderer *wlr_renderer_autocreate(struct wlr_backend *backend) {
 	return renderer_autocreate(backend, -1);
 }
 
-int wlr_renderer_get_drm_fd(struct wlr_renderer *r) {
-	if (!r->impl->get_drm_fd) {
-		return -1;
-	}
-	return r->impl->get_drm_fd(r);
-}
-
 struct wlr_render_pass *wlr_renderer_begin_buffer_pass(struct wlr_renderer *renderer,
 		struct wlr_buffer *buffer, const struct wlr_buffer_pass_options *options) {
 	struct wlr_buffer_pass_options default_options = {0};

types/wlr_drm.c

@@ -201,15 +201,14 @@ static void handle_display_destroy(struct wl_listener *listener, void *data) {
 
 struct wlr_drm *wlr_drm_create(struct wl_display *display,
 		struct wlr_renderer *renderer) {
-	int drm_fd = wlr_renderer_get_drm_fd(renderer);
+	if (renderer->drm_dev_id == NULL) {
-	if (drm_fd < 0) {
+		wlr_log(WLR_ERROR, "Failed to get DRM device from renderer");
-		wlr_log(WLR_ERROR, "Failed to get DRM FD from renderer");
 		return NULL;
 	}
 
 	drmDevice *dev = NULL;
-	if (drmGetDevice2(drm_fd, 0, &dev) != 0) {
+	if (drmGetDeviceFromDevId(*renderer->drm_dev_id, 0, &dev) != 0) {
-		wlr_log(WLR_ERROR, "drmGetDevice2 failed");
+		wlr_log(WLR_ERROR, "drmGetDeviceFromDevId failed");
 		return NULL;
 	}
 

types/wlr_linux_dmabuf_v1.c

@@ -1087,16 +1087,11 @@ bool wlr_linux_dmabuf_feedback_v1_init_with_options(struct wlr_linux_dmabuf_feed
 
 	*feedback = (struct wlr_linux_dmabuf_feedback_v1){0};
 
-	int renderer_drm_fd = wlr_renderer_get_drm_fd(options->main_renderer);
+	if (options->main_renderer->drm_dev_id == NULL) {
-	if (renderer_drm_fd < 0) {
-		wlr_log(WLR_ERROR, "Failed to get renderer DRM FD");
-		goto error;
-	}
-	dev_t renderer_dev;
-	if (!devid_from_fd(renderer_drm_fd, &renderer_dev)) {
 		goto error;
 	}
 
+	dev_t renderer_dev = *options->main_renderer->drm_dev_id;
 	feedback->main_device = renderer_dev;
 
 	const struct wlr_drm_format_set *renderer_formats =