Re: [PATCH 1/4] rust: debugfs: drop 'static bound from ScopedDir file creation methods

["Gary Guo" <gary@garyguo.net>]

On Thu Sep 3, 2026 at 4:07 PM BST, Danilo Krummrich wrote:
> On Thu Sep 3, 2026 at 3:12 PM CEST, Gary Guo wrote:
>> This can be better done by storing `&'static bindings::file_operations` in
>> `FileOps<T>` instead of just by value. That is actually better than the current
>> impl, IMO, because `mode` for example doesn't have to be in static storage. (You
>> can also then make `FileOps<T>` `Copy`).
>
> That's a great suggestion, thanks. It simplifies the patch to:
>
> Author: Danilo Krummrich <dakr@kernel.org>
> Date:   Sat Aug 29 14:42:54 2026 +0200
>
>     rust: debugfs: drop 'static bound from ScopedDir file creation methods
>
>     Drop the T: 'static bound from ScopedDir's file creation methods
>     (read_binary_file(), read_only_file(), etc.) to support registering
>     debugfs files backed by types that contain non-'static references, such
>     as dma::Coherent<'a, T>.
>
>     The previous 'static bound existed because ScopedDir::create_file() took
>     &'static FileOps<T>, and &'static requires T: 'static for well-
>     formedness. However, this was overly conservative: the file_operations
>     pointer passed to the C debugfs API just needs to be 'static, not the
>     entire FileOps<T>.
>
>     Store &'static bindings::file_operations in FileOps<T> instead of the
>     file_operations by value. In each trait impl, take a reference to the
>     file_operations struct within the const block; since
>     bindings::file_operations does not mention T, the reference is promoted
>     to 'static regardless of T's lifetime parameters.
>
>     Replace the Deref impl with an explicit fops() method that returns the
>     stored &'static reference.
>
>     Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Gary Guo <gary@garyguo.net>

Some nits below.

>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index d7b8014a6474..2beb55d444ca 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -538,7 +538,7 @@ pub fn dir<'dir2>(&'dir2 self, name: &CStr) -> ScopedDir<'data, 'dir2> {
>          }
>      }
>
> -    fn create_file<T: Sync>(&self, name: &CStr, data: &'data T, vtable: &'static FileOps<T>) {
> +    fn create_file<T: Sync>(&self, name: &CStr, data: &'data T, vtable: &FileOps<T>) {

I suppose this doesn't need to use reference anymore, but you want to keep the
diff small.

>          #[cfg(CONFIG_DEBUG_FS)]
>          core::mem::forget(Entry::file(name, &self.entry, data, vtable));
>      }
> @@ -550,7 +550,7 @@ fn create_file<T: Sync>(&self, name: &CStr, data: &'data T, vtable: &'static Fil
>      /// This function does not produce an owning handle to the file. The created
>      /// file is removed when the [`Scope`] that this directory belongs
>      /// to is dropped.
> -    pub fn read_only_file<T: Writer + Send + Sync + 'static>(&self, name: &CStr, data: &'data T) {
> +    pub fn read_only_file<T: Writer + Send + Sync>(&self, name: &CStr, data: &'data T) {
>          self.create_file(name, data, &T::FILE_OPS)
>      }
>
> [snip]
>
> diff --git a/rust/kernel/debugfs/file_ops.rs b/rust/kernel/debugfs/file_ops.rs
> index f15908f71c4a..5c16a3196ca2 100644
> --- a/rust/kernel/debugfs/file_ops.rs
> +++ b/rust/kernel/debugfs/file_ops.rs
> @@ -20,9 +20,6 @@
>
>  use core::marker::PhantomData;
>
> -#[cfg(CONFIG_DEBUG_FS)]
> -use core::ops::Deref;
> -
>  /// # Invariant
>  ///
>  /// `FileOps<T>` will always contain an `operations` which is safe to use for a file backed
> @@ -30,7 +27,7 @@
>  /// into a reference.
>  pub(super) struct FileOps<T> {
>      #[cfg(CONFIG_DEBUG_FS)]
> -    operations: bindings::file_operations,
> +    operations: &'static bindings::file_operations,
>      #[cfg(CONFIG_DEBUG_FS)]
>      mode: u16,
>      _phantom: PhantomData<T>,
> @@ -41,7 +38,7 @@ impl<T> FileOps<T> {
>      ///
>      /// The caller asserts that the provided `operations` is safe to use for a file whose
>      /// inode has a pointer to `T` in its private data that is safe to convert into a reference.
> -    const unsafe fn new(operations: bindings::file_operations, mode: u16) -> Self {
> +    const unsafe fn new(operations: &'static bindings::file_operations, mode: u16) -> Self {
>          Self {
>              #[cfg(CONFIG_DEBUG_FS)]
>              operations,
> @@ -65,11 +62,11 @@ pub(super) const fn adapt(&self) -> &FileOps<T::Inner> {
>  }
>
>  #[cfg(CONFIG_DEBUG_FS)]
> -impl<T> Deref for FileOps<T> {
> -    type Target = bindings::file_operations;
> -
> -    fn deref(&self) -> &Self::Target {
> -        &self.operations
> +impl<T> FileOps<T> {
> +    /// Returns a `'static` reference to the inner `file_operations`.
> +    #[inline]
> +    pub(crate) fn fops(&self) -> &'static bindings::file_operations {
> +        self.operations
>      }
>  }
>
> @@ -130,11 +127,11 @@ pub(crate) trait ReadFile<T> {
>
>  impl<T: Writer + Sync> ReadFile<T> for T {
>      const FILE_OPS: FileOps<T> = {
> -        let operations = bindings::file_operations {
> +        let operations = &bindings::file_operations {
>              read: Some(bindings::seq_read),
>              llseek: Some(bindings::seq_lseek),
>              release: Some(bindings::single_release),
> -            open: Some(writer_open::<Self>),
> +            open: Some(writer_open::<T>),

Is this change needed?

Best,
Gary

>              ..pin_init::zeroed()
>          };
>          // SAFETY: `operations` is all stock `seq_file` implementations except for `writer_open`.