The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/7] lifetime-parameterized DRM File private data
@ 2026-08-14 23:08 Danilo Krummrich
  2026-08-14 23:08 ` [PATCH 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:08 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

DRM file private data is currently 'static, preventing drivers from
borrowing registration-scoped resources in their per-file state. This
series makes Driver::File lifetime-parameterized through ForLt, so file
data can borrow from RegistrationData.

File data is guaranteed to be dropped before registration data, either
when the file is closed normally or when the device is unregistered,
whichever comes first. To enforce this during unbind, Registration::drop()
revokes all open file data and waits for in-flight closes before dropping
registration data.

fops.open is wrapped in a RegistrationGuard (drm_dev_enter/exit) so that
after drm_dev_unplug() all successfully opened files are accounted for.
GEM open/close callbacks are similarly gated to prevent access to stale
file data after unbind.

The series is based on drm-rust-next with [1] applied and [2] merged, a branch
with the patches is available in [3].

[1] https://lore.kernel.org/all/20260726223613.1242940-1-dakr@kernel.org/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/tag/?h=dd-lifetimes-7.3-rc1
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=drm/file

Danilo Krummrich (7):
  rust: drm: rename Ioctl device context to Userspace
  rust: drm: gem: gate open/close callbacks with RegistrationGuard
  rust: drm: move file_operations from gem to device
  rust: fs: add iminor() helper
  rust: drm: wrap fops open with RegistrationGuard
  rust: drm: make Driver::File lifetime-parameterized
  rust: drm: return impl PinInit from DriverFile::open()

 drivers/gpu/drm/nova/driver.rs |   5 +-
 drivers/gpu/drm/nova/file.rs   |  13 ++-
 drivers/gpu/drm/nova/gem.rs    |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |   5 +-
 drivers/gpu/drm/tyr/file.rs    |  13 +--
 rust/helpers/fs.c              |   5 ++
 rust/kernel/drm/device.rs      | 139 ++++++++++++++++++++++++++----
 rust/kernel/drm/driver.rs      |  69 ++++++++++++++-
 rust/kernel/drm/file.rs        | 151 ++++++++++++++++++++++-----------
 rust/kernel/drm/gem/mod.rs     |  48 +++++------
 rust/kernel/drm/gem/shmem.rs   |  14 +--
 rust/kernel/drm/ioctl.rs       |  14 +--
 rust/kernel/drm/mod.rs         |   2 +-
 13 files changed, 356 insertions(+), 130 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/7] rust: drm: rename Ioctl device context to Userspace
  2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
@ 2026-08-14 23:08 ` Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard Danilo Krummrich
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:08 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

The Ioctl DeviceContext typestate represents a device that has been
registered with userspace at some point. This context is not specific to
ioctl dispatch; it applies equally to GEM handle callbacks, mmap,
fdinfo, and any other operation triggered by userspace on a registered
device.

Rename it to Userspace to accurately reflect its semantics.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs | 21 ++++++++++++---------
 rust/kernel/drm/ioctl.rs  | 12 ++++++------
 rust/kernel/drm/mod.rs    |  2 +-
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index f43c6887ad23..be83287fe161 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -79,12 +79,12 @@ macro_rules! drm_legacy_fields {
 ///
 /// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
 ///   or may not be registered with userspace.
-/// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl
-///   dispatch context.
+/// - [`Userspace`]: The device has been registered with userspace at some point; used in
+///   callbacks triggered by userspace operations.
 /// - [`Registered`]: The device is currently registered with userspace and the parent bus device
 ///   is bound.
 ///
-/// Both `Device<T, Ioctl>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
+/// Both `Device<T, Userspace>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
 /// so any method available on a [`Normal`] device is also available in the other contexts.
 pub trait DeviceContext: Sealed + Send + Sync + 'static {}
 
@@ -120,14 +120,17 @@ impl DeviceContext for Registered {}
 /// unregistering or already unregistered. `drm_dev_enter()` can guard against this, ensuring the
 /// device remains registered for the duration of the critical section.
 ///
+/// This context is used for all callbacks triggered by userspace operations: ioctls, GEM handle
+/// management, mmap, fdinfo, etc.
+///
 /// # Invariants
 ///
 /// A [`Device`] in this context has been registered with userspace via `drm_dev_register()` at
 /// some point.
-pub struct Ioctl;
+pub struct Userspace;
 
-impl Sealed for Ioctl {}
-impl DeviceContext for Ioctl {}
+impl Sealed for Userspace {}
+impl DeviceContext for Userspace {}
 
 /// A [`Device`] which is known at compile-time to be unregistered with userspace.
 ///
@@ -343,7 +346,7 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
     }
 }
 
-impl<T: drm::Driver> Device<T, Ioctl> {
+impl<T: drm::Driver> Device<T, Userspace> {
     /// Guard against the parent bus device being unbound.
     ///
     /// Returns a [`RegistrationGuard`] if the device has not been unplugged, [`None`] otherwise.
@@ -466,12 +469,12 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: drm::Driver> Deref for Device<T, Ioctl> {
+impl<T: drm::Driver> Deref for Device<T, Userspace> {
     type Target = Device<T>;
 
     #[inline]
     fn deref(&self) -> &Self::Target {
-        // SAFETY: The caller holds a `Device<T, Ioctl>`, which guarantees all invariants
+        // SAFETY: The caller holds a `Device<T, Userspace>`, which guarantees all invariants
         // of the weaker `Normal` context.
         unsafe { self.assume_ctx() }
     }
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 64af9eacc306..9934b23c36eb 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -71,14 +71,14 @@ pub mod internal {
     pub use bindings::drm_file;
     pub use bindings::drm_ioctl_desc;
 
-    /// Cast an [`Ioctl`] DRM device pointer to [`Registered`], preserving the driver type
+    /// Cast a [`Userspace`] DRM device pointer to [`Registered`], preserving the driver type
     /// parameter `T`.
     ///
     /// Used by [`declare_drm_ioctls!`] to anchor type inference.
     #[doc(hidden)]
     #[inline]
     pub const fn __dev_ctx_cast<T: crate::drm::Driver>(
-        ptr: *const crate::drm::Device<T, crate::drm::Ioctl>,
+        ptr: *const crate::drm::Device<T, crate::drm::Userspace>,
     ) -> *const crate::drm::Device<T, crate::drm::Registered> {
         ptr.cast()
     }
@@ -144,14 +144,14 @@ macro_rules! declare_drm_ioctls {
                             // - The DRM device must have been registered when we're called through
                             //   an IOCTL.
                             //
-                            // INVARIANT: The `Ioctl` context requires that the device has been
-                            // registered via `drm_dev_register()` at some point; the DRM core
-                            // guarantees this for ioctl dispatch callbacks.
+                            // INVARIANT: The `Userspace` context requires that the device has
+                            // been registered via `drm_dev_register()` at some point; the DRM
+                            // core guarantees this for ioctl dispatch callbacks.
                             //
                             // FIXME: Currently there is nothing enforcing that the types of the
                             // dev/file match the current driver these ioctls are being declared
                             // for, and it's not clear how to enforce this within the type system.
-                            let dev: &$crate::drm::device::Device<_, $crate::drm::Ioctl> =
+                            let dev: &$crate::drm::device::Device<_, $crate::drm::Userspace> =
                                 $crate::drm::device::Device::from_raw(raw_dev);
 
                             // Type-inference anchor: the closure is never called but ties `dev`'s
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index fd6ed35bc35a..7fcf2465a82e 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -11,11 +11,11 @@
 
 pub use self::device::Device;
 pub use self::device::DeviceContext;
-pub use self::device::Ioctl;
 pub use self::device::Normal;
 pub use self::device::Registered;
 pub use self::device::RegistrationGuard;
 pub use self::device::UnregisteredDevice;
+pub use self::device::Userspace;
 pub use self::driver::Driver;
 pub use self::driver::DriverInfo;
 pub use self::driver::Registration;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard
  2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
  2026-08-14 23:08 ` [PATCH 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
@ 2026-08-14 23:09 ` Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 3/7] rust: drm: move file_operations from gem to device Danilo Krummrich
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:09 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Wrap the GEM object open and close callbacks with a RegistrationGuard
(drm_dev_enter / drm_dev_exit) to ensure the driver callbacks only run
while the parent bus device is bound.

If the device has been unbound, open returns -ENODEV and close silently
returns. This prevents driver code from accessing device resources after
unbind and is a prerequisite for making drm::Driver::File
lifetime-parameterized, since GEM callbacks receive a &drm::File that
could otherwise be used to access invalidated file private data.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/gem/mod.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 80d8f524f9d5..560403ca8e38 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -128,6 +128,14 @@ extern "C" fn open_callback<T: DriverObject>(
     raw_obj: *mut bindings::drm_gem_object,
     raw_file: *mut bindings::drm_file,
 ) -> core::ffi::c_int {
+    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
+    let dev: &drm::Device<T::Driver, drm::Userspace> =
+        unsafe { drm::Device::from_raw((*raw_obj).dev) };
+
+    let Some(_guard) = dev.registration_guard() else {
+        return ENODEV.to_errno();
+    };
+
     // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
@@ -148,6 +156,14 @@ extern "C" fn close_callback<T: DriverObject>(
     raw_obj: *mut bindings::drm_gem_object,
     raw_file: *mut bindings::drm_file,
 ) {
+    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
+    let dev: &drm::Device<T::Driver, drm::Userspace> =
+        unsafe { drm::Device::from_raw((*raw_obj).dev) };
+
+    let Some(_guard) = dev.registration_guard() else {
+        return;
+    };
+
     // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/7] rust: drm: move file_operations from gem to device
  2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
  2026-08-14 23:08 ` [PATCH 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard Danilo Krummrich
@ 2026-08-14 23:09 ` Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 4/7] rust: fs: add iminor() helper Danilo Krummrich
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:09 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Move the file_operations definition from drm::gem::create_fops() into
UnregisteredDevice in drm::device. This is the file_operations of the
DRM minor device, set through the drm_driver struct. It is not specific
to GEM.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs  | 22 ++++++++++++++++++++--
 rust/kernel/drm/gem/mod.rs | 20 --------------------
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index be83287fe161..a2940e172073 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -195,10 +195,28 @@ const fn compute_features() -> u32 {
         driver_features: Self::compute_features(),
         ioctls: T::IOCTLS.as_ptr(),
         num_ioctls: T::IOCTLS.len() as i32,
-        fops: &Self::GEM_FOPS,
+        fops: &Self::FOPS,
     };
 
-    const GEM_FOPS: bindings::file_operations = drm::gem::create_fops();
+    const FOPS: bindings::file_operations = {
+        let mut fops: bindings::file_operations = pin_init::zeroed();
+
+        fops.owner = core::ptr::null_mut();
+        fops.open = Some(bindings::drm_open);
+        fops.release = Some(bindings::drm_release);
+        fops.unlocked_ioctl = Some(bindings::drm_ioctl);
+        #[cfg(CONFIG_COMPAT)]
+        {
+            fops.compat_ioctl = Some(bindings::drm_compat_ioctl);
+        }
+        fops.poll = Some(bindings::drm_poll);
+        fops.read = Some(bindings::drm_read);
+        fops.llseek = Some(bindings::noop_llseek);
+        fops.mmap = Some(bindings::drm_gem_mmap);
+        fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET;
+
+        fops
+    };
 
     /// Create a new `UnregisteredDevice` for a `drm::Driver`.
     ///
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 560403ca8e38..ee048febfbdc 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -399,23 +399,3 @@ impl<T: DriverObject, Ctx: DeviceContext> AllocImpl for Object<T, Ctx> {
         dumb_map_offset: None,
     };
 }
-
-pub(super) const fn create_fops() -> bindings::file_operations {
-    let mut fops: bindings::file_operations = pin_init::zeroed();
-
-    fops.owner = core::ptr::null_mut();
-    fops.open = Some(bindings::drm_open);
-    fops.release = Some(bindings::drm_release);
-    fops.unlocked_ioctl = Some(bindings::drm_ioctl);
-    #[cfg(CONFIG_COMPAT)]
-    {
-        fops.compat_ioctl = Some(bindings::drm_compat_ioctl);
-    }
-    fops.poll = Some(bindings::drm_poll);
-    fops.read = Some(bindings::drm_read);
-    fops.llseek = Some(bindings::noop_llseek);
-    fops.mmap = Some(bindings::drm_gem_mmap);
-    fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET;
-
-    fops
-}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/7] rust: fs: add iminor() helper
  2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-08-14 23:09 ` [PATCH 3/7] rust: drm: move file_operations from gem to device Danilo Krummrich
@ 2026-08-14 23:09 ` Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 5/7] rust: drm: wrap fops open with RegistrationGuard Danilo Krummrich
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:09 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Add a Rust helper for iminor(), which extracts the minor number from a
struct inode. This is a static inline function in the C headers, so
bindgen does not generate a binding for it.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/helpers/fs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/helpers/fs.c b/rust/helpers/fs.c
index 789d60fb8908..d6f9782db178 100644
--- a/rust/helpers/fs.c
+++ b/rust/helpers/fs.c
@@ -10,3 +10,8 @@ __rust_helper struct file *rust_helper_get_file(struct file *f)
 {
 	return get_file(f);
 }
+
+__rust_helper unsigned int rust_helper_iminor(const struct inode *inode)
+{
+	return iminor(inode);
+}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/7] rust: drm: wrap fops open with RegistrationGuard
  2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (3 preceding siblings ...)
  2026-08-14 23:09 ` [PATCH 4/7] rust: fs: add iminor() helper Danilo Krummrich
@ 2026-08-14 23:09 ` Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 6/7] rust: drm: make Driver::File lifetime-parameterized Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 7/7] rust: drm: return impl PinInit from DriverFile::open() Danilo Krummrich
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:09 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Add a Minor abstraction with RAII release and a fops_open() wrapper that
holds a RegistrationGuard (drm_dev_enter / drm_dev_exit) across the
entire drm_open() call.

This guarantees that drm_dev_unplug() in Registration::drop() waits for
the full open sequence to complete, so all files are visible in the
filelist when iterating for cleanup.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs | 63 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index a2940e172073..09903ed783e1 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -42,6 +42,42 @@
     },
 };
 
+/// A reference to a `struct drm_minor` with RAII release.
+struct Minor(NonNull<bindings::drm_minor>);
+
+// Methods use `#[inline(never)]` to prevent the unexported `drm_minor_acquire()` /
+// `drm_minor_release()` symbols from being inlined into driver modules.
+impl Minor {
+    /// Acquire a minor by ID. Increments the underlying device's refcount.
+    #[inline(never)]
+    fn acquire(minor_id: u32) -> Result<Self> {
+        // SAFETY: `drm_minors_xa` is a valid global xarray; any `minor_id` is safe to
+        // look up (returns ERR_PTR on failure).
+        let ptr =
+            unsafe { bindings::drm_minor_acquire(&raw mut bindings::drm_minors_xa, minor_id) };
+        Ok(Self(NonNull::new(from_err_ptr(ptr)?).ok_or(ENODEV)?))
+    }
+
+    /// Returns a reference to the DRM device for this minor.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the minor belongs to a `Device<T>`.
+    unsafe fn device<T: drm::Driver>(&self) -> &Device<T, Userspace> {
+        // SAFETY: The minor is valid (from `drm_minor_acquire()`) and `minor->dev`
+        // is a valid `drm_device`. The caller guarantees it is a `Device<T>`.
+        unsafe { Device::from_raw((*self.0.as_ptr()).dev) }
+    }
+}
+
+impl Drop for Minor {
+    #[inline(never)]
+    fn drop(&mut self) {
+        // SAFETY: `self.0` came from `drm_minor_acquire()` and has not been released yet.
+        unsafe { bindings::drm_minor_release(self.0.as_ptr()) }
+    }
+}
+
 #[cfg(CONFIG_DRM_LEGACY)]
 macro_rules! drm_legacy_fields {
     ( $($field:ident: $val:expr),* $(,)? ) => {
@@ -198,11 +234,36 @@ const fn compute_features() -> u32 {
         fops: &Self::FOPS,
     };
 
+    /// Wrapper for `fops.open` that holds a [`RegistrationGuard`] across the entire `drm_open()`
+    /// call. This guarantees that `drm_dev_unplug()` in `Registration::drop()` waits for the full
+    /// open sequence.
+    extern "C" fn fops_open(inode: *mut bindings::inode, filp: *mut bindings::file) -> c_int {
+        let f = || -> Result<c_int> {
+            // SAFETY: `inode` is valid.
+            let minor_id = unsafe { bindings::iminor(inode) };
+            let minor = Minor::acquire(minor_id)?;
+
+            // SAFETY: `fops_open` is only installed for devices of type `T` (via `FOPS`).
+            let _guard = (unsafe { minor.device::<T>() })
+                .registration_guard()
+                .ok_or(ENODEV)?;
+
+            // SAFETY: `inode` and `filp` are valid. The RegistrationGuard ensures the entire
+            // `drm_open()` runs within the SRCU critical section.
+            Ok(unsafe { bindings::drm_open(inode, filp) })
+        };
+
+        match f() {
+            Ok(ret) => ret,
+            Err(e) => e.to_errno(),
+        }
+    }
+
     const FOPS: bindings::file_operations = {
         let mut fops: bindings::file_operations = pin_init::zeroed();
 
         fops.owner = core::ptr::null_mut();
-        fops.open = Some(bindings::drm_open);
+        fops.open = Some(Self::fops_open);
         fops.release = Some(bindings::drm_release);
         fops.unlocked_ioctl = Some(bindings::drm_ioctl);
         #[cfg(CONFIG_COMPAT)]
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/7] rust: drm: make Driver::File lifetime-parameterized
  2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (4 preceding siblings ...)
  2026-08-14 23:09 ` [PATCH 5/7] rust: drm: wrap fops open with RegistrationGuard Danilo Krummrich
@ 2026-08-14 23:09 ` Danilo Krummrich
  2026-08-14 23:09 ` [PATCH 7/7] rust: drm: return impl PinInit from DriverFile::open() Danilo Krummrich
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:09 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Make the DRM file private data lifetime-parameterized, allowing drivers
to borrow from RegistrationData in their per-file data.

Introduce DriverFile<'a> as a lifetime-parameterized trait that receives
both the device and registration data in open(). Parametrize File<D> on
the driver type rather than the file data type, deriving the concrete
file type through ForLt.

Add inner() for covariant file types and inner_with() for invariant ones
to access the driver file data from a File<D> reference.

Ensure file data is always dropped before registration data:

  - In Registration::drop(), iterate the filelist under filelist_mutex
    and drop driver_priv for all open files, then wait for in-flight
    postclose_callback() calls to complete via an open_count / WaitQueue
    pair on drm::Device.

  - In postclose_callback(), skip the drop if driver_priv has already
    been NULLed by the filelist iteration. The NULL write is visible
    through the filelist_mutex acquire/release chain in
    drm_close_helper().

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/drm/nova/driver.rs |   5 +-
 drivers/gpu/drm/nova/file.rs   |  11 ++-
 drivers/gpu/drm/nova/gem.rs    |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |   5 +-
 drivers/gpu/drm/tyr/file.rs    |  11 ++-
 rust/kernel/drm/device.rs      |  35 ++++++--
 rust/kernel/drm/driver.rs      |  69 ++++++++++++++-
 rust/kernel/drm/file.rs        | 150 ++++++++++++++++++++++-----------
 rust/kernel/drm/gem/mod.rs     |  12 ++-
 rust/kernel/drm/gem/shmem.rs   |  12 ++-
 rust/kernel/drm/ioctl.rs       |   2 +-
 11 files changed, 231 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 739690bc2db5..a8f61086773d 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -12,7 +12,8 @@
         ioctl, //
     },
     prelude::*,
-    sync::aref::ARef, //
+    sync::aref::ARef,
+    types::CovariantForLt, //
 };
 
 use crate::file::File;
@@ -75,7 +76,7 @@ fn probe<'bound>(
 impl drm::Driver for NovaDriver {
     type Data = ();
     type RegistrationData<'a> = ();
-    type File = File;
+    type File = CovariantForLt!(File);
     type Object = gem::Object<NovaObject>;
     type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
 
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 298c02bacb4b..1f94201af92b 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -3,7 +3,6 @@
 use crate::driver::{NovaDevice, NovaDriver};
 use crate::gem::NovaObject;
 use kernel::{
-    alloc::flags::*,
     auxiliary,
     device::Bound,
     drm::{
@@ -18,10 +17,10 @@
 
 pub(crate) struct File;
 
-impl drm::file::DriverFile for File {
+impl drm::file::DriverFile<'_> for File {
     type Driver = NovaDriver;
 
-    fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
+    fn open(_device: &NovaDevice<Registered>, _reg_data: &()) -> Result<Pin<KBox<Self>>> {
         Ok(KBox::new(Self, GFP_KERNEL)?.into())
     }
 }
@@ -32,7 +31,7 @@ pub(crate) fn get_param(
         dev: &NovaDevice<Registered>,
         _reg_data: &(),
         getparam: &mut uapi::drm_nova_getparam,
-        _file: &drm::File<File>,
+        _file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let adev: &auxiliary::Device<Bound> = dev.as_ref();
         let pdev: &pci::Device<Bound> = adev.parent().try_into()?;
@@ -52,7 +51,7 @@ pub(crate) fn gem_create(
         dev: &NovaDevice<Registered>,
         _reg_data: &(),
         req: &mut uapi::drm_nova_gem_create,
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let obj = NovaObject::new(dev, req.size.try_into()?)?;
 
@@ -66,7 +65,7 @@ pub(crate) fn gem_info(
         _dev: &NovaDevice<Registered>,
         _reg_data: &(),
         req: &mut uapi::drm_nova_gem_info,
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let bo = NovaObject::lookup_handle(file, req.handle)?;
 
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index 2b6fe9dc0bfa..2a21ff8ba579 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -11,9 +11,9 @@
     sync::aref::ARef,
 };
 
-use crate::{
-    driver::{NovaDevice, NovaDriver},
-    file::File,
+use crate::driver::{
+    NovaDevice,
+    NovaDriver, //
 };
 
 /// GEM Object inner driver data
@@ -43,7 +43,7 @@ pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self
     /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it.
     #[inline]
     pub(crate) fn lookup_handle(
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
         handle: u32,
     ) -> Result<ARef<gem::Object<Self>>> {
         gem::Object::lookup_handle(file, handle)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index d78ad9d292ff..94bc85635725 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -32,7 +32,8 @@
         Arc,
         Mutex, //
     },
-    time, //
+    time,
+    types::CovariantForLt, //
 };
 
 use crate::{
@@ -206,7 +207,7 @@ fn drop(self: Pin<&mut Self>) {}
 impl drm::Driver for TyrDrmDriver {
     type Data = ();
     type RegistrationData<'drm> = TyrDrmRegistrationData<'drm>;
-    type File = TyrDrmFileData;
+    type File = CovariantForLt!(TyrDrmFileData);
     type Object = Bo;
     type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
 
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 9f60a90d4948..0e0878090de6 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -19,13 +19,16 @@
 #[pin_data]
 pub(crate) struct TyrDrmFileData {}
 
-/// Convenience type alias for our DRM `File` type
-pub(crate) type TyrDrmFile = drm::file::File<TyrDrmFileData>;
+/// Convenience type alias for our DRM `File` type.
+pub(crate) type TyrDrmFile = drm::file::File<TyrDrmDriver>;
 
-impl drm::file::DriverFile for TyrDrmFileData {
+impl drm::file::DriverFile<'_> for TyrDrmFileData {
     type Driver = TyrDrmDriver;
 
-    fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
+    fn open(
+        _device: &TyrDrmDevice<Registered>,
+        _reg_data: &TyrDrmRegistrationData<'_>,
+    ) -> Result<Pin<KBox<Self>>> {
         KBox::try_pin_init(try_pin_init!(Self {}), GFP_KERNEL)
     }
 }
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 09903ed783e1..29512d0e2ddb 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -15,11 +15,16 @@
     },
     error::from_err_ptr,
     prelude::*,
-    sync::aref::{
-        ARef,
-        AlwaysRefCounted, //
+    sync::{
+        aref::{
+            ARef,
+            AlwaysRefCounted, //
+        },
+        atomic::Atomic,
+        WaitQueue,
     },
     types::{
+        ForLt,
         NotThreadSafe,
         Opaque, //
     },
@@ -190,7 +195,10 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: drm::Driver> UnregisteredDevice<T> {
+impl<T: drm::Driver> UnregisteredDevice<T>
+where
+    for<'a> <T::File as ForLt>::Of<'a>: drm::file::DriverFile<'a, Driver = T>,
+{
     const fn compute_features() -> u32 {
         let mut features = drm::driver::FEAT_GEM;
 
@@ -203,8 +211,8 @@ const fn compute_features() -> u32 {
 
     const VTABLE: bindings::drm_driver = drm_legacy_fields! {
         load: None,
-        open: Some(drm::File::<T::File>::open_callback),
-        postclose: Some(drm::File::<T::File>::postclose_callback),
+        open: Some(drm::File::<T>::open_callback),
+        postclose: Some(drm::File::<T>::postclose_callback),
         unload: None,
         release: Some(Device::<T>::release),
         master_set: None,
@@ -333,6 +341,19 @@ pub fn new(
         // SAFETY: `raw_drm` is valid; no concurrent access before registration.
         unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
 
+        // SAFETY: `raw_drm` is valid; no concurrent access before registration.
+        unsafe { (*raw_drm.as_ptr()).open_count = Atomic::new(0) };
+
+        // SAFETY:
+        // - `raw_drm` is valid; no concurrent access before registration.
+        // - The field is pinned because the Device is pinned (refcounted, allocated by
+        //   `__drm_dev_alloc()`, never moved).
+        // - The init is infallible.
+        let Ok(()) = unsafe {
+            crate::new_waitqueue!("drm_open_count")
+                .__pinned_init(&raw mut (*raw_drm.as_ptr()).open_count_wq)
+        };
+
         // SAFETY: The reference count is one, and now we take ownership of that reference as a
         // `drm::Device`.
         // INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
@@ -357,6 +378,8 @@ pub struct Device<T: drm::Driver, C: DeviceContext = Normal> {
     dev: Opaque<bindings::drm_device>,
     data: T::Data,
     pub(super) registration_data: UnsafeCell<NonNull<T::RegistrationData<'static>>>,
+    pub(super) open_count: Atomic<i32>,
+    pub(super) open_count_wq: WaitQueue,
     _ctx: PhantomData<C>,
 }
 
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 74f6ed690d8b..2aa534149d78 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -9,8 +9,17 @@
     device,
     drm,
     error::to_result,
+    interop::list::clist_create,
     prelude::*,
-    sync::aref::ARef, //
+    sync::{
+        aref::ARef,
+        atomic::{
+            Acquire,
+            Release, //
+        },
+        Mutex, //
+    },
+    types::ForLt, //
 };
 use core::ptr::NonNull;
 
@@ -117,8 +126,15 @@ pub trait Driver {
     /// The type used to manage memory for this driver.
     type Object: AllocImpl;
 
-    /// The type used to represent a DRM File (client)
-    type File: drm::file::DriverFile;
+    /// The type used to represent a DRM File (client).
+    ///
+    /// File data may borrow from [`RegistrationData`](Driver::RegistrationData). File data is
+    /// guaranteed to be dropped before registration data, either when the file is closed or
+    /// when the device is unregistered, whichever comes first.
+    ///
+    /// Drivers set this to `CovariantForLt!(MyFileData)` (or `ForLt!` for invariant types)
+    /// and implement [`DriverFile`](drm::file::DriverFile) for their file data type.
+    type File: ForLt + 'static;
 
     /// The bus device type of the parent device that the DRM device is associated with.
     type ParentDevice<Ctx: device::DeviceContext>: device::AsBusDevice<Ctx>;
@@ -221,6 +237,51 @@ fn drop(&mut self) {
         unsafe { bindings::drm_dev_unplug(self.drm.as_raw()) };
         // After drm_dev_unplug(), the SRCU barrier guarantees that all RegistrationGuard critical
         // sections have completed, so no one holds a reference to reg_data anymore.
-        // reg_data is dropped here automatically.
+
+        // Drop all remaining file private data before dropping registration data. This guarantees
+        // that file data (which may borrow from RegistrationData) is always dropped first.
+        let raw = self.drm.as_raw();
+
+        // SAFETY: `filelist_mutex` is initialized by `drm_dev_init()` and remains valid for
+        // the lifetime of the `struct drm_device`.
+        let filelist_mutex = unsafe { Mutex::from_raw(&raw mut (*raw).filelist_mutex) };
+        {
+            let _guard = filelist_mutex.lock();
+
+            // SAFETY: `filelist` is a valid, initialized sentinel `list_head`; the mutex
+            // guard prevents concurrent modification.
+            let filelist = unsafe {
+                clist_create!(
+                    &raw mut (*raw).filelist,
+                    drm::File<T>,
+                    bindings::drm_file,
+                    lhead
+                )
+            };
+
+            for file in filelist.iter() {
+                // SAFETY: `file` is a valid `drm_file` on this device's filelist.
+                let priv_ptr = unsafe { (*file.as_raw()).driver_priv };
+
+                // SAFETY: Setting `driver_priv` to NULL is visible to `postclose_callback()`
+                // through the `filelist_mutex` acquire/release chain in `drm_close_helper()`.
+                unsafe { (*file.as_raw()).driver_priv = core::ptr::null_mut() };
+
+                // SAFETY: `driver_priv` was created by `open_callback()` via `KBox::into_raw` and
+                // has not yet been freed (the file is still in the list, so `postclose_callback()`
+                // has not run).
+                drop(unsafe { KBox::from_raw(priv_ptr.cast::<<T::File as ForLt>::Of<'static>>()) });
+
+                self.drm.open_count.fetch_sub(1, Release);
+            }
+        }
+
+        // Wait for in-flight `postclose_callback()` calls to complete. After `drm_dev_unplug()`, no
+        // new opens can succeed, so `open_count` is monotonically decreasing.
+        self.drm
+            .open_count_wq
+            .wait_event(|| self.drm.open_count.load(Acquire) == 0);
+
+        // `_reg_data` is dropped here automatically, after all file data has been dropped.
     }
 }
diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
index 10160601ce5a..6491ec5707a0 100644
--- a/rust/kernel/drm/file.rs
+++ b/rust/kernel/drm/file.rs
@@ -8,17 +8,34 @@
     bindings,
     drm,
     prelude::*,
-    types::Opaque, //
+    sync::atomic::{
+        Relaxed,
+        Release, //
+    },
+    types::{
+        CovariantForLt,
+        ForLt,
+        Opaque, //
+    }, //
 };
 use core::marker::PhantomData;
 
 /// Trait that must be implemented by DRM drivers to represent a DRM File (a client instance).
-pub trait DriverFile {
+///
+/// The lifetime `'a` allows the file data to borrow from
+/// [`RegistrationData`](drm::Driver::RegistrationData).
+pub trait DriverFile<'a>: Sized {
     /// The parent `Driver` implementation for this `DriverFile`.
     type Driver: drm::Driver;
 
-    /// Open a new file (called when a client opens the DRM device).
-    fn open(device: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>>;
+    /// Open a new DRM file, creating the per-file driver data.
+    ///
+    /// Called when a client opens the DRM device. The returned file data may borrow from
+    /// `reg_data` with lifetime `'a`.
+    fn open(
+        device: &drm::Device<Self::Driver, drm::Registered>,
+        reg_data: &'a <Self::Driver as drm::Driver>::RegistrationData<'a>,
+    ) -> Result<Pin<KBox<Self>>>;
 }
 
 /// An open DRM File.
@@ -27,17 +44,17 @@ pub trait DriverFile {
 ///
 /// `self.0` is a valid instance of a `struct drm_file`.
 #[repr(transparent)]
-pub struct File<T: DriverFile>(Opaque<bindings::drm_file>, PhantomData<T>);
+pub struct File<D: drm::Driver>(Opaque<bindings::drm_file>, PhantomData<D>);
 
-impl<T: DriverFile> File<T> {
+impl<D: drm::Driver> File<D> {
     #[doc(hidden)]
     /// Not intended to be called externally, except via declare_drm_ioctls!()
     ///
     /// # Safety
     ///
-    /// `raw_file` must be a valid pointer to an open `struct drm_file`, opened through `T::open`.
-    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_file) -> &'a File<T> {
-        // SAFETY: `raw_file` is valid by the safety requirements of this function.
+    /// `ptr` must be a valid pointer to an open `struct drm_file`.
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_file) -> &'a File<D> {
+        // SAFETY: `ptr` is valid by the safety requirements of this function.
         unsafe { &*ptr.cast() }
     }
 
@@ -45,59 +62,94 @@ pub(super) fn as_raw(&self) -> *mut bindings::drm_file {
         self.0.get()
     }
 
-    fn driver_priv(&self) -> *mut T {
-        // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid.
-        unsafe { (*self.as_raw()).driver_priv }.cast()
+    /// Return a pinned reference to the driver file data.
+    ///
+    /// Only available when `D::File` implements [`trait@CovariantForLt`]. For invariant types, use
+    /// [`inner_with()`](Self::inner_with).
+    pub fn inner(&self) -> Pin<&<D::File as ForLt>::Of<'_>>
+    where
+        D::File: CovariantForLt,
+    {
+        // SAFETY: `driver_priv` was initialized by `open_callback()`. `CovariantForLt` guarantees
+        // the lifetime shortening from `'static` to `'_` is sound.
+        unsafe { Pin::new_unchecked(&*(*self.as_raw()).driver_priv.cast_const().cast()) }
     }
 
-    /// Return a pinned reference to the driver file structure.
-    pub fn inner(&self) -> Pin<&T> {
-        // SAFETY: By the type invariant the pointer `self.as_raw()` points to a valid and opened
-        // `struct drm_file`, hence `driver_priv` has been properly initialized by `open_callback`.
-        unsafe { Pin::new_unchecked(&*(self.driver_priv())) }
+    /// Access the driver file data through a closure.
+    ///
+    /// This works for all file data types, including invariant ones. For covariant types,
+    /// [`inner()`](Self::inner) provides direct access without a closure.
+    pub fn inner_with<R, F>(&self, f: F) -> R
+    where
+        F: for<'a> FnOnce(Pin<&'a <D::File as ForLt>::Of<'a>>) -> R,
+    {
+        // SAFETY: `driver_priv` was initialized by `open_callback()`. The HRTB `for<'a>` prevents
+        // the caller from choosing a concrete lifetime, making the lifetime shortening sound
+        // regardless of variance.
+        f(unsafe { Pin::new_unchecked(&*(*self.as_raw()).driver_priv.cast_const().cast()) })
     }
 
     /// The open callback of a `struct drm_file`.
+    ///
+    /// Called from `drm_open()`, which is itself called from `fops_open()`. The latter holds a
+    /// `RegistrationGuard`, so the device is guaranteed to be registered for the duration of this
+    /// callback.
     pub(crate) extern "C" fn open_callback(
         raw_dev: *mut bindings::drm_device,
         raw_file: *mut bindings::drm_file,
-    ) -> core::ffi::c_int {
-        // SAFETY: A callback from `struct drm_driver::open` guarantees that
-        // - `raw_dev` is valid pointer to a `struct drm_device`,
-        // - the corresponding `struct drm_device` has been registered.
-        let drm = unsafe { drm::Device::from_raw(raw_dev) };
-
-        // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
-        let file = unsafe { File::<T>::from_raw(raw_file) };
-
-        let inner = match T::open(drm) {
-            Err(e) => {
-                return e.to_errno();
-            }
-            Ok(i) => i,
-        };
-
-        // SAFETY: This pointer is treated as pinned, and the Drop guarantee is upheld in
-        // `postclose_callback()`.
-        let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) });
-
-        // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid.
-        unsafe { (*file.as_raw()).driver_priv = driver_priv.cast() };
-
-        0
+    ) -> core::ffi::c_int
+    where
+        for<'a> <D::File as ForLt>::Of<'a>: DriverFile<'a, Driver = D>,
+    {
+        // SAFETY: The DRM core guarantees that `raw_dev` is valid. `fops_open()` holds a
+        // `RegistrationGuard`, so the device is registered and the `Registered` context holds.
+        let dev: &drm::device::Device<D, drm::Registered> =
+            unsafe { drm::device::Device::from_raw(raw_dev) };
+
+        dev.registration_data_with(|reg_data| {
+            let inner = match <<D::File as ForLt>::Of<'_> as DriverFile<'_>>::open(dev, reg_data) {
+                Err(e) => return e.to_errno(),
+                Ok(i) => i,
+            };
+
+            // SAFETY: This pointer is treated as pinned, and the Drop guarantee is upheld in
+            // `postclose_callback()` or the filelist iteration in `Registration::drop()`.
+            let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) });
+
+            dev.open_count.fetch_add(1, Relaxed);
+
+            // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
+            unsafe { (*raw_file).driver_priv = driver_priv.cast() };
+
+            0
+        })
     }
 
     /// The postclose callback of a `struct drm_file`.
     pub(crate) extern "C" fn postclose_callback(
-        _raw_dev: *mut bindings::drm_device,
+        raw_dev: *mut bindings::drm_device,
         raw_file: *mut bindings::drm_file,
-    ) {
-        // SAFETY: This reference won't escape this function
-        let file = unsafe { File::<T>::from_raw(raw_file) };
-
-        // SAFETY: `file.driver_priv` has been created in `open_callback` through `KBox::into_raw`.
-        let _ = unsafe { KBox::from_raw(file.driver_priv()) };
+    ) where
+        for<'a> <D::File as ForLt>::Of<'a>: DriverFile<'a, Driver = D>,
+    {
+        // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
+        let driver_priv = unsafe { (*raw_file).driver_priv };
+
+        if driver_priv.is_null() {
+            return;
+        }
+
+        // SAFETY: `driver_priv` was created in `open_callback()` through `KBox::into_raw` and has
+        // not been dropped yet (the NULL check above guards against double-free from the filelist
+        // iteration in `Registration::drop()`).
+        let _ = unsafe { KBox::from_raw(driver_priv.cast::<<D::File as ForLt>::Of<'static>>()) };
+
+        // SAFETY: `raw_dev` is valid for the lifetime of the `struct drm_file`.
+        let dev: &drm::device::Device<D> = unsafe { drm::device::Device::from_raw(raw_dev) };
+        if dev.open_count.fetch_sub(1, Release) == 1 {
+            dev.open_count_wq.wake_up();
+        }
     }
 }
 
-impl<T: DriverFile> super::private::Sealed for File<T> {}
+impl<D: drm::Driver> super::private::Sealed for File<D> {}
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index ee048febfbdc..1aa18f0b172c 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -76,7 +76,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
 ///
 /// [`Driver`]: drm::Driver
 /// [`DriverFile`]: drm::file::DriverFile
-pub type DriverFile<T> = drm::File<<<T as DriverObject>::Driver as drm::Driver>::File>;
+pub type DriverFile<T> = drm::File<<T as DriverObject>::Driver>;
 
 /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
 ///
@@ -196,11 +196,10 @@ fn size(&self) -> usize {
 
     /// Creates a new handle for the object associated with a given `File`
     /// (or returns an existing one).
-    fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
+    fn create_handle<D>(&self, file: &drm::File<D>) -> Result<u32>
     where
         Self: AllocImpl<Driver = D>,
-        D: drm::Driver<Object = Self, File = F>,
-        F: drm::file::DriverFile<Driver = D>,
+        D: drm::Driver<Object = Self>,
     {
         let mut handle: u32 = 0;
         // SAFETY: The arguments are all valid per the type invariants.
@@ -211,11 +210,10 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
     }
 
     /// Looks up an object by its handle for a given `File`.
-    fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
+    fn lookup_handle<D>(file: &drm::File<D>, handle: u32) -> Result<ARef<Self>>
     where
         Self: AllocImpl<Driver = D> + AlwaysRefCounted,
-        D: drm::Driver<Object = Self, File = F>,
-        F: drm::file::DriverFile<Driver = D>,
+        D: drm::Driver<Object = Self>,
     {
         // SAFETY: The arguments are all valid per the type invariants.
         let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) };
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index a687d46d170d..793b4883de3a 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -584,7 +584,8 @@ mod tests {
         },
         faux,
         io::Io,
-        page::PAGE_SIZE, //
+        page::PAGE_SIZE,
+        types::CovariantForLt, //
     };
 
     // The bare minimum needed to create a fake drm driver for kunit
@@ -604,10 +605,13 @@ struct KunitObject {}
         desc: c"Kunit",
     };
 
-    impl drm::file::DriverFile for KunitFile {
+    impl drm::file::DriverFile<'_> for KunitFile {
         type Driver = KunitDriver;
 
-        fn open(_dev: &drm::Device<KunitDriver>) -> Result<Pin<KBox<Self>>> {
+        fn open(
+            _device: &drm::Device<KunitDriver, drm::Registered>,
+            _reg_data: &(),
+        ) -> Result<Pin<KBox<Self>>> {
             Ok(KBox::new(Self, GFP_KERNEL)?.into())
         }
     }
@@ -629,7 +633,7 @@ fn new(
     impl drm::Driver for KunitDriver {
         type Data = KunitData;
         type RegistrationData<'a> = ();
-        type File = KunitFile;
+        type File = CovariantForLt!(KunitFile);
         type Object = Object<KunitObject>;
         type ParentDevice<Ctx: device::DeviceContext> = faux::Device<Ctx>;
 
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 9934b23c36eb..8ac7fb1d4ec2 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -97,7 +97,7 @@ pub const fn __dev_ctx_cast<T: crate::drm::Driver>(
 /// fn foo(device: &kernel::drm::Device<Self, kernel::drm::Registered>,
 ///        reg_data: &Self::RegistrationData<'_>,
 ///        data: &mut uapi::argument_type,
-///        file: &kernel::drm::File<Self::File>,
+///        file: &kernel::drm::File<Self>,
 /// ) -> Result<u32>
 /// ```
 /// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within.
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 7/7] rust: drm: return impl PinInit from DriverFile::open()
  2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (5 preceding siblings ...)
  2026-08-14 23:09 ` [PATCH 6/7] rust: drm: make Driver::File lifetime-parameterized Danilo Krummrich
@ 2026-08-14 23:09 ` Danilo Krummrich
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-08-14 23:09 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Change DriverFile::open() to return impl PinInit<Self, Error> instead of
Result<Pin<KBox<Self>>>, consistent with how bus device private data
works.

Drivers no longer need to allocate a Pin<KBox<_>> themselves; they just
return an initializer and the subsystem takes care of the allocation.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/drm/nova/file.rs | 4 ++--
 drivers/gpu/drm/tyr/file.rs  | 4 ++--
 rust/kernel/drm/file.rs      | 7 ++++---
 rust/kernel/drm/gem/shmem.rs | 4 ++--
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 1f94201af92b..30bbabe6ee78 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -20,8 +20,8 @@
 impl drm::file::DriverFile<'_> for File {
     type Driver = NovaDriver;
 
-    fn open(_device: &NovaDevice<Registered>, _reg_data: &()) -> Result<Pin<KBox<Self>>> {
-        Ok(KBox::new(Self, GFP_KERNEL)?.into())
+    fn open(_device: &NovaDevice<Registered>, _reg_data: &()) -> impl PinInit<Self, Error> {
+        Ok(Self)
     }
 }
 
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 0e0878090de6..933a365cb016 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -28,8 +28,8 @@ impl drm::file::DriverFile<'_> for TyrDrmFileData {
     fn open(
         _device: &TyrDrmDevice<Registered>,
         _reg_data: &TyrDrmRegistrationData<'_>,
-    ) -> Result<Pin<KBox<Self>>> {
-        KBox::try_pin_init(try_pin_init!(Self {}), GFP_KERNEL)
+    ) -> impl PinInit<Self, Error> {
+        Ok(Self {})
     }
 }
 
diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
index 6491ec5707a0..31fc318eb535 100644
--- a/rust/kernel/drm/file.rs
+++ b/rust/kernel/drm/file.rs
@@ -30,12 +30,12 @@ pub trait DriverFile<'a>: Sized {
 
     /// Open a new DRM file, creating the per-file driver data.
     ///
-    /// Called when a client opens the DRM device. The returned file data may borrow from
+    /// Called when a client opens the DRM device. The returned initializer may borrow from
     /// `reg_data` with lifetime `'a`.
     fn open(
         device: &drm::Device<Self::Driver, drm::Registered>,
         reg_data: &'a <Self::Driver as drm::Driver>::RegistrationData<'a>,
-    ) -> Result<Pin<KBox<Self>>>;
+    ) -> impl PinInit<Self, Error>;
 }
 
 /// An open DRM File.
@@ -107,7 +107,8 @@ pub(crate) extern "C" fn open_callback(
             unsafe { drm::device::Device::from_raw(raw_dev) };
 
         dev.registration_data_with(|reg_data| {
-            let inner = match <<D::File as ForLt>::Of<'_> as DriverFile<'_>>::open(dev, reg_data) {
+            let init = <<D::File as ForLt>::Of<'_> as DriverFile<'_>>::open(dev, reg_data);
+            let inner = match KBox::try_pin_init(init, GFP_KERNEL) {
                 Err(e) => return e.to_errno(),
                 Ok(i) => i,
             };
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index 793b4883de3a..d882b505fe0f 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -611,8 +611,8 @@ impl drm::file::DriverFile<'_> for KunitFile {
         fn open(
             _device: &drm::Device<KunitDriver, drm::Registered>,
             _reg_data: &(),
-        ) -> Result<Pin<KBox<Self>>> {
-            Ok(KBox::new(Self, GFP_KERNEL)?.into())
+        ) -> impl PinInit<Self, Error> {
+            Ok(Self)
         }
     }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-14 23:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 23:08 [PATCH 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
2026-08-14 23:08 ` [PATCH 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
2026-08-14 23:09 ` [PATCH 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard Danilo Krummrich
2026-08-14 23:09 ` [PATCH 3/7] rust: drm: move file_operations from gem to device Danilo Krummrich
2026-08-14 23:09 ` [PATCH 4/7] rust: fs: add iminor() helper Danilo Krummrich
2026-08-14 23:09 ` [PATCH 5/7] rust: drm: wrap fops open with RegistrationGuard Danilo Krummrich
2026-08-14 23:09 ` [PATCH 6/7] rust: drm: make Driver::File lifetime-parameterized Danilo Krummrich
2026-08-14 23:09 ` [PATCH 7/7] rust: drm: return impl PinInit from DriverFile::open() Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox