[PATCH] rust: miscdevice: change how f_ops vtable is constructed

[PATCH] rust: miscdevice: change how f_ops vtable is constructed

[Alice Ryhl]

I was helping someone with writing a new Rust abstraction, and we were
using the miscdevice abstraction as an example. While doing this, it
became clear to me that the way I implemented the f_ops vtable is
confusing to new Rust users, and that the approach used by the block
abstractions is less confusing.

Thus, update the miscdevice abstractions to use the same approach as
rust/kernel/block/mq/operations.rs.

Sorry about the large diff. This changes the indentation of a large
amount of code.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/miscdevice.rs | 295 ++++++++++++++++++++++------------------------
 1 file changed, 141 insertions(+), 154 deletions(-)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index dfb363630c70..ad02434cbeaf 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -39,7 +39,7 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
         let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
         result.minor = bindings::MISC_DYNAMIC_MINOR as _;
         result.name = self.name.as_char_ptr();
-        result.fops = create_vtable::<T>();
+        result.fops = MiscdeviceVTable::<T>::build();
         result
     }
 }
@@ -164,171 +164,158 @@ fn show_fdinfo(
     }
 }
 
-const fn create_vtable<T: MiscDevice>() -> &'static bindings::file_operations {
-    const fn maybe_fn<T: Copy>(check: bool, func: T) -> Option<T> {
-        if check {
-            Some(func)
-        } else {
-            None
+/// A vtable for the file operations of a Rust miscdevice.
+struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);
+
+impl<T: MiscDevice> MiscdeviceVTable<T> {
+    /// # Safety
+    ///
+    /// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
+    /// The file must be associated with a `MiscDeviceRegistration<T>`.
+    unsafe extern "C" fn open(inode: *mut bindings::inode, raw_file: *mut bindings::file) -> c_int {
+        // SAFETY: The pointers are valid and for a file being opened.
+        let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
+        if ret != 0 {
+            return ret;
         }
-    }
 
-    struct VtableHelper<T: MiscDevice> {
-        _t: PhantomData<T>,
-    }
-    impl<T: MiscDevice> VtableHelper<T> {
-        const VTABLE: bindings::file_operations = bindings::file_operations {
-            open: Some(fops_open::<T>),
-            release: Some(fops_release::<T>),
-            unlocked_ioctl: maybe_fn(T::HAS_IOCTL, fops_ioctl::<T>),
-            #[cfg(CONFIG_COMPAT)]
-            compat_ioctl: if T::HAS_COMPAT_IOCTL {
-                Some(fops_compat_ioctl::<T>)
-            } else if T::HAS_IOCTL {
-                Some(bindings::compat_ptr_ioctl)
-            } else {
-                None
-            },
-            show_fdinfo: maybe_fn(T::HAS_SHOW_FDINFO, fops_show_fdinfo::<T>),
-            // SAFETY: All zeros is a valid value for `bindings::file_operations`.
-            ..unsafe { MaybeUninit::zeroed().assume_init() }
-        };
-    }
+        // SAFETY: The open call of a file can access the private data.
+        let misc_ptr = unsafe { (*raw_file).private_data };
 
-    &VtableHelper::<T>::VTABLE
-}
+        // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
+        // associated `struct miscdevice` before calling into this method. Furthermore, `misc_open()`
+        // ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`.
+        let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
 
-/// # Safety
-///
-/// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
-/// The file must be associated with a `MiscDeviceRegistration<T>`.
-unsafe extern "C" fn fops_open<T: MiscDevice>(
-    inode: *mut bindings::inode,
-    raw_file: *mut bindings::file,
-) -> c_int {
-    // SAFETY: The pointers are valid and for a file being opened.
-    let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
-    if ret != 0 {
-        return ret;
-    }
+        // SAFETY:
+        // * This underlying file is valid for (much longer than) the duration of `T::open`.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(raw_file) };
 
-    // SAFETY: The open call of a file can access the private data.
-    let misc_ptr = unsafe { (*raw_file).private_data };
-
-    // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
-    // associated `struct miscdevice` before calling into this method. Furthermore, `misc_open()`
-    // ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`.
-    let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
+        let ptr = match T::open(file, misc) {
+            Ok(ptr) => ptr,
+            Err(err) => return err.to_errno(),
+        };
 
-    // SAFETY:
-    // * This underlying file is valid for (much longer than) the duration of `T::open`.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(raw_file) };
+        // This overwrites the private data with the value specified by the user, changing the type of
+        // this file's private data. All future accesses to the private data is performed by other
+        // fops_* methods in this file, which all correctly cast the private data to the new type.
+        //
+        // SAFETY: The open call of a file can access the private data.
+        unsafe { (*raw_file).private_data = ptr.into_foreign().cast_mut() };
 
-    let ptr = match T::open(file, misc) {
-        Ok(ptr) => ptr,
-        Err(err) => return err.to_errno(),
-    };
-
-    // This overwrites the private data with the value specified by the user, changing the type of
-    // this file's private data. All future accesses to the private data is performed by other
-    // fops_* methods in this file, which all correctly cast the private data to the new type.
-    //
-    // SAFETY: The open call of a file can access the private data.
-    unsafe { (*raw_file).private_data = ptr.into_foreign().cast_mut() };
+        0
+    }
 
-    0
-}
+    /// # Safety
+    ///
+    /// `file` and `inode` must be the file and inode for a file that is being released. The file must
+    /// be associated with a `MiscDeviceRegistration<T>`.
+    unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int {
+        // SAFETY: The release call of a file owns the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: The release call of a file owns the private data.
+        let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
+
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        T::release(ptr, unsafe { File::from_raw_file(file) });
+
+        0
+    }
 
-/// # Safety
-///
-/// `file` and `inode` must be the file and inode for a file that is being released. The file must
-/// be associated with a `MiscDeviceRegistration<T>`.
-unsafe extern "C" fn fops_release<T: MiscDevice>(
-    _inode: *mut bindings::inode,
-    file: *mut bindings::file,
-) -> c_int {
-    // SAFETY: The release call of a file owns the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: The release call of a file owns the private data.
-    let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
-
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    T::release(ptr, unsafe { File::from_raw_file(file) });
-
-    0
-}
+    /// # Safety
+    ///
+    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
+    unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {
+        // SAFETY: The ioctl call of a file can access the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: Ioctl calls can borrow the private data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(file) };
+
+        match T::ioctl(device, file, cmd, arg as usize) {
+            Ok(ret) => ret as c_long,
+            Err(err) => err.to_errno() as c_long,
+        }
+    }
 
-/// # Safety
-///
-/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
-unsafe extern "C" fn fops_ioctl<T: MiscDevice>(
-    file: *mut bindings::file,
-    cmd: c_uint,
-    arg: c_ulong,
-) -> c_long {
-    // SAFETY: The ioctl call of a file can access the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: Ioctl calls can borrow the private data of the file.
-    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
-
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(file) };
-
-    match T::ioctl(device, file, cmd, arg as usize) {
-        Ok(ret) => ret as c_long,
-        Err(err) => err.to_errno() as c_long,
+    /// # Safety
+    ///
+    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
+    #[cfg(CONFIG_COMPAT)]
+    unsafe extern "C" fn compat_ioctl(
+        file: *mut bindings::file,
+        cmd: c_uint,
+        arg: c_ulong,
+    ) -> c_long {
+        // SAFETY: The compat ioctl call of a file can access the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: Ioctl calls can borrow the private data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(file) };
+
+        match T::compat_ioctl(device, file, cmd, arg as usize) {
+            Ok(ret) => ret as c_long,
+            Err(err) => err.to_errno() as c_long,
+        }
     }
-}
 
-/// # Safety
-///
-/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
-#[cfg(CONFIG_COMPAT)]
-unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>(
-    file: *mut bindings::file,
-    cmd: c_uint,
-    arg: c_ulong,
-) -> c_long {
-    // SAFETY: The compat ioctl call of a file can access the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: Ioctl calls can borrow the private data of the file.
-    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
-
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(file) };
-
-    match T::compat_ioctl(device, file, cmd, arg as usize) {
-        Ok(ret) => ret as c_long,
-        Err(err) => err.to_errno() as c_long,
+    /// # Safety
+    ///
+    /// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
+    /// - `seq_file` must be a valid `struct seq_file` that we can write to.
+    unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) {
+        // SAFETY: The release call of a file owns the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: Ioctl calls can borrow the private data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(file) };
+        // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which
+        // this method is called.
+        let m = unsafe { SeqFile::from_raw(seq_file) };
+
+        T::show_fdinfo(device, m, file);
     }
-}
 
-/// # Safety
-///
-/// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
-/// - `seq_file` must be a valid `struct seq_file` that we can write to.
-unsafe extern "C" fn fops_show_fdinfo<T: MiscDevice>(
-    seq_file: *mut bindings::seq_file,
-    file: *mut bindings::file,
-) {
-    // SAFETY: The release call of a file owns the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: Ioctl calls can borrow the private data of the file.
-    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(file) };
-    // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which
-    // this method is called.
-    let m = unsafe { SeqFile::from_raw(seq_file) };
-
-    T::show_fdinfo(device, m, file);
+    const VTABLE: bindings::file_operations = bindings::file_operations {
+        open: Some(Self::open),
+        release: Some(Self::release),
+        unlocked_ioctl: if T::HAS_IOCTL {
+            Some(Self::ioctl)
+        } else {
+            None
+        },
+        #[cfg(CONFIG_COMPAT)]
+        compat_ioctl: if T::HAS_COMPAT_IOCTL {
+            Some(Self::compat_ioctl)
+        } else if T::HAS_IOCTL {
+            Some(Self::bindings::compat_ptr_ioctl)
+        } else {
+            None
+        },
+        show_fdinfo: if T::HAS_SHOW_FDINFO {
+            Some(Self::show_fdinfo)
+        } else {
+            None
+        },
+        // SAFETY: All zeros is a valid value for `bindings::file_operations`.
+        ..unsafe { MaybeUninit::zeroed().assume_init() }
+    };
+
+    const fn build() -> &'static bindings::file_operations {
+        &Self::VTABLE
+    }
 }

---
base-commit: 01b3cb620815fc3feb90ee117d9445a5b608a9f7
change-id: 20250117-miscdevice-fops-change-260352fd8957

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

Re: [PATCH] rust: miscdevice: change how f_ops vtable is constructed

[Christian Schrefl]

On 17.01.25 3:22 PM, Alice Ryhl wrote:
> I was helping someone with writing a new Rust abstraction, and we were
> using the miscdevice abstraction as an example. While doing this, it
> became clear to me that the way I implemented the f_ops vtable is
> confusing to new Rust users, and that the approach used by the block
> abstractions is less confusing.
> 
> Thus, update the miscdevice abstractions to use the same approach as
> rust/kernel/block/mq/operations.rs.
> 
> Sorry about the large diff. This changes the indentation of a large
> amount of code.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>

>  rust/kernel/miscdevice.rs | 295 ++++++++++++++++++++++------------------------
>  1 file changed, 141 insertions(+), 154 deletions(-)
> 
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index dfb363630c70..ad02434cbeaf 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -39,7 +39,7 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
>          let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
>          result.minor = bindings::MISC_DYNAMIC_MINOR as _;
>          result.name = self.name.as_char_ptr();
> -        result.fops = create_vtable::<T>();
> +        result.fops = MiscdeviceVTable::<T>::build();
>          result
>      }
>  }
> @@ -164,171 +164,158 @@ fn show_fdinfo(
>      }
>  }
>  
> -const fn create_vtable<T: MiscDevice>() -> &'static bindings::file_operations {
> -    const fn maybe_fn<T: Copy>(check: bool, func: T) -> Option<T> {
> -        if check {
> -            Some(func)
> -        } else {
> -            None
> +/// A vtable for the file operations of a Rust miscdevice.
> +struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);
> +
> +impl<T: MiscDevice> MiscdeviceVTable<T> {
> +    /// # Safety
> +    ///
> +    /// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
> +    /// The file must be associated with a `MiscDeviceRegistration<T>`.
> +    unsafe extern "C" fn open(inode: *mut bindings::inode, raw_file: *mut bindings::file) -> c_int {
> +        // SAFETY: The pointers are valid and for a file being opened.
> +        let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
> +        if ret != 0 {
> +            return ret;
>          }
> -    }
>  
> -    struct VtableHelper<T: MiscDevice> {
> -        _t: PhantomData<T>,
> -    }
> -    impl<T: MiscDevice> VtableHelper<T> {
> -        const VTABLE: bindings::file_operations = bindings::file_operations {
> -            open: Some(fops_open::<T>),
> -            release: Some(fops_release::<T>),
> -            unlocked_ioctl: maybe_fn(T::HAS_IOCTL, fops_ioctl::<T>),
> -            #[cfg(CONFIG_COMPAT)]
> -            compat_ioctl: if T::HAS_COMPAT_IOCTL {
> -                Some(fops_compat_ioctl::<T>)
> -            } else if T::HAS_IOCTL {
> -                Some(bindings::compat_ptr_ioctl)
> -            } else {
> -                None
> -            },
> -            show_fdinfo: maybe_fn(T::HAS_SHOW_FDINFO, fops_show_fdinfo::<T>),
> -            // SAFETY: All zeros is a valid value for `bindings::file_operations`.
> -            ..unsafe { MaybeUninit::zeroed().assume_init() }
> -        };
> -    }
> +        // SAFETY: The open call of a file can access the private data.
> +        let misc_ptr = unsafe { (*raw_file).private_data };
>  
> -    &VtableHelper::<T>::VTABLE
> -}
> +        // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
> +        // associated `struct miscdevice` before calling into this method. Furthermore, `misc_open()`
> +        // ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`.
> +        let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
>  
> -/// # Safety
> -///
> -/// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
> -/// The file must be associated with a `MiscDeviceRegistration<T>`.
> -unsafe extern "C" fn fops_open<T: MiscDevice>(
> -    inode: *mut bindings::inode,
> -    raw_file: *mut bindings::file,
> -) -> c_int {
> -    // SAFETY: The pointers are valid and for a file being opened.
> -    let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
> -    if ret != 0 {
> -        return ret;
> -    }
> +        // SAFETY:
> +        // * This underlying file is valid for (much longer than) the duration of `T::open`.
> +        // * There is no active fdget_pos region on the file on this thread.
> +        let file = unsafe { File::from_raw_file(raw_file) };
>  
> -    // SAFETY: The open call of a file can access the private data.
> -    let misc_ptr = unsafe { (*raw_file).private_data };
> -
> -    // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
> -    // associated `struct miscdevice` before calling into this method. Furthermore, `misc_open()`
> -    // ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`.
> -    let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
> +        let ptr = match T::open(file, misc) {
> +            Ok(ptr) => ptr,
> +            Err(err) => return err.to_errno(),
> +        };
>  
> -    // SAFETY:
> -    // * This underlying file is valid for (much longer than) the duration of `T::open`.
> -    // * There is no active fdget_pos region on the file on this thread.
> -    let file = unsafe { File::from_raw_file(raw_file) };
> +        // This overwrites the private data with the value specified by the user, changing the type of
> +        // this file's private data. All future accesses to the private data is performed by other
> +        // fops_* methods in this file, which all correctly cast the private data to the new type.
> +        //
> +        // SAFETY: The open call of a file can access the private data.
> +        unsafe { (*raw_file).private_data = ptr.into_foreign().cast_mut() };
>  
> -    let ptr = match T::open(file, misc) {
> -        Ok(ptr) => ptr,
> -        Err(err) => return err.to_errno(),
> -    };
> -
> -    // This overwrites the private data with the value specified by the user, changing the type of
> -    // this file's private data. All future accesses to the private data is performed by other
> -    // fops_* methods in this file, which all correctly cast the private data to the new type.
> -    //
> -    // SAFETY: The open call of a file can access the private data.
> -    unsafe { (*raw_file).private_data = ptr.into_foreign().cast_mut() };
> +        0
> +    }
>  
> -    0
> -}
> +    /// # Safety
> +    ///
> +    /// `file` and `inode` must be the file and inode for a file that is being released. The file must
> +    /// be associated with a `MiscDeviceRegistration<T>`.
> +    unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int {
> +        // SAFETY: The release call of a file owns the private data.
> +        let private = unsafe { (*file).private_data };
> +        // SAFETY: The release call of a file owns the private data.
> +        let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
> +
> +        // SAFETY:
> +        // * The file is valid for the duration of this call.
> +        // * There is no active fdget_pos region on the file on this thread.
> +        T::release(ptr, unsafe { File::from_raw_file(file) });
> +
> +        0
> +    }
>  
> -/// # Safety
> -///
> -/// `file` and `inode` must be the file and inode for a file that is being released. The file must
> -/// be associated with a `MiscDeviceRegistration<T>`.
> -unsafe extern "C" fn fops_release<T: MiscDevice>(
> -    _inode: *mut bindings::inode,
> -    file: *mut bindings::file,
> -) -> c_int {
> -    // SAFETY: The release call of a file owns the private data.
> -    let private = unsafe { (*file).private_data };
> -    // SAFETY: The release call of a file owns the private data.
> -    let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
> -
> -    // SAFETY:
> -    // * The file is valid for the duration of this call.
> -    // * There is no active fdget_pos region on the file on this thread.
> -    T::release(ptr, unsafe { File::from_raw_file(file) });
> -
> -    0
> -}
> +    /// # Safety
> +    ///
> +    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
> +    unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {
> +        // SAFETY: The ioctl call of a file can access the private data.
> +        let private = unsafe { (*file).private_data };
> +        // SAFETY: Ioctl calls can borrow the private data of the file.
> +        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> +
> +        // SAFETY:
> +        // * The file is valid for the duration of this call.
> +        // * There is no active fdget_pos region on the file on this thread.
> +        let file = unsafe { File::from_raw_file(file) };
> +
> +        match T::ioctl(device, file, cmd, arg as usize) {
> +            Ok(ret) => ret as c_long,
> +            Err(err) => err.to_errno() as c_long,
> +        }
> +    }
>  
> -/// # Safety
> -///
> -/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
> -unsafe extern "C" fn fops_ioctl<T: MiscDevice>(
> -    file: *mut bindings::file,
> -    cmd: c_uint,
> -    arg: c_ulong,
> -) -> c_long {
> -    // SAFETY: The ioctl call of a file can access the private data.
> -    let private = unsafe { (*file).private_data };
> -    // SAFETY: Ioctl calls can borrow the private data of the file.
> -    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> -
> -    // SAFETY:
> -    // * The file is valid for the duration of this call.
> -    // * There is no active fdget_pos region on the file on this thread.
> -    let file = unsafe { File::from_raw_file(file) };
> -
> -    match T::ioctl(device, file, cmd, arg as usize) {
> -        Ok(ret) => ret as c_long,
> -        Err(err) => err.to_errno() as c_long,
> +    /// # Safety
> +    ///
> +    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
> +    #[cfg(CONFIG_COMPAT)]
> +    unsafe extern "C" fn compat_ioctl(
> +        file: *mut bindings::file,
> +        cmd: c_uint,
> +        arg: c_ulong,
> +    ) -> c_long {
> +        // SAFETY: The compat ioctl call of a file can access the private data.
> +        let private = unsafe { (*file).private_data };
> +        // SAFETY: Ioctl calls can borrow the private data of the file.
> +        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> +
> +        // SAFETY:
> +        // * The file is valid for the duration of this call.
> +        // * There is no active fdget_pos region on the file on this thread.
> +        let file = unsafe { File::from_raw_file(file) };
> +
> +        match T::compat_ioctl(device, file, cmd, arg as usize) {
> +            Ok(ret) => ret as c_long,
> +            Err(err) => err.to_errno() as c_long,
> +        }
>      }
> -}
>  
> -/// # Safety
> -///
> -/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
> -#[cfg(CONFIG_COMPAT)]
> -unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>(
> -    file: *mut bindings::file,
> -    cmd: c_uint,
> -    arg: c_ulong,
> -) -> c_long {
> -    // SAFETY: The compat ioctl call of a file can access the private data.
> -    let private = unsafe { (*file).private_data };
> -    // SAFETY: Ioctl calls can borrow the private data of the file.
> -    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> -
> -    // SAFETY:
> -    // * The file is valid for the duration of this call.
> -    // * There is no active fdget_pos region on the file on this thread.
> -    let file = unsafe { File::from_raw_file(file) };
> -
> -    match T::compat_ioctl(device, file, cmd, arg as usize) {
> -        Ok(ret) => ret as c_long,
> -        Err(err) => err.to_errno() as c_long,
> +    /// # Safety
> +    ///
> +    /// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
> +    /// - `seq_file` must be a valid `struct seq_file` that we can write to.
> +    unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) {
> +        // SAFETY: The release call of a file owns the private data.
> +        let private = unsafe { (*file).private_data };
> +        // SAFETY: Ioctl calls can borrow the private data of the file.
> +        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> +        // SAFETY:
> +        // * The file is valid for the duration of this call.
> +        // * There is no active fdget_pos region on the file on this thread.
> +        let file = unsafe { File::from_raw_file(file) };
> +        // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which
> +        // this method is called.
> +        let m = unsafe { SeqFile::from_raw(seq_file) };
> +
> +        T::show_fdinfo(device, m, file);
>      }
> -}
>  
> -/// # Safety
> -///
> -/// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
> -/// - `seq_file` must be a valid `struct seq_file` that we can write to.
> -unsafe extern "C" fn fops_show_fdinfo<T: MiscDevice>(
> -    seq_file: *mut bindings::seq_file,
> -    file: *mut bindings::file,
> -) {
> -    // SAFETY: The release call of a file owns the private data.
> -    let private = unsafe { (*file).private_data };
> -    // SAFETY: Ioctl calls can borrow the private data of the file.
> -    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> -    // SAFETY:
> -    // * The file is valid for the duration of this call.
> -    // * There is no active fdget_pos region on the file on this thread.
> -    let file = unsafe { File::from_raw_file(file) };
> -    // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which
> -    // this method is called.
> -    let m = unsafe { SeqFile::from_raw(seq_file) };
> -
> -    T::show_fdinfo(device, m, file);
> +    const VTABLE: bindings::file_operations = bindings::file_operations {
> +        open: Some(Self::open),
> +        release: Some(Self::release),
> +        unlocked_ioctl: if T::HAS_IOCTL {
> +            Some(Self::ioctl)
> +        } else {
> +            None
> +        },
> +        #[cfg(CONFIG_COMPAT)]
> +        compat_ioctl: if T::HAS_COMPAT_IOCTL {
> +            Some(Self::compat_ioctl)
> +        } else if T::HAS_IOCTL {
> +            Some(Self::bindings::compat_ptr_ioctl)
> +        } else {
> +            None
> +        },
> +        show_fdinfo: if T::HAS_SHOW_FDINFO {
> +            Some(Self::show_fdinfo)
> +        } else {
> +            None
> +        },
> +        // SAFETY: All zeros is a valid value for `bindings::file_operations`.
> +        ..unsafe { MaybeUninit::zeroed().assume_init() }
> +    };
> +
> +    const fn build() -> &'static bindings::file_operations {
> +        &Self::VTABLE
> +    }
>  }
> 
> ---
> base-commit: 01b3cb620815fc3feb90ee117d9445a5b608a9f7
> change-id: 20250117-miscdevice-fops-change-260352fd8957
> 
> Best regards,

Re: [PATCH] rust: miscdevice: change how f_ops vtable is constructed

[Greg Kroah-Hartman]

On Fri, Jan 17, 2025 at 02:22:32PM +0000, Alice Ryhl wrote:
> I was helping someone with writing a new Rust abstraction, and we were
> using the miscdevice abstraction as an example. While doing this, it
> became clear to me that the way I implemented the f_ops vtable is
> confusing to new Rust users, and that the approach used by the block
> abstractions is less confusing.
> 
> Thus, update the miscdevice abstractions to use the same approach as
> rust/kernel/block/mq/operations.rs.
> 
> Sorry about the large diff. This changes the indentation of a large
> amount of code.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/miscdevice.rs | 295 ++++++++++++++++++++++------------------------
>  1 file changed, 141 insertions(+), 154 deletions(-)

This doesn't apply against a clean 6.14-rc2 tree, what is is made
against?

thanks,

greg k-h

Re: [PATCH] rust: miscdevice: change how f_ops vtable is constructed

[Alice Ryhl]

On Wed, Feb 19, 2025 at 4:58 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jan 17, 2025 at 02:22:32PM +0000, Alice Ryhl wrote:
> > I was helping someone with writing a new Rust abstraction, and we were
> > using the miscdevice abstraction as an example. While doing this, it
> > became clear to me that the way I implemented the f_ops vtable is
> > confusing to new Rust users, and that the approach used by the block
> > abstractions is less confusing.
> >
> > Thus, update the miscdevice abstractions to use the same approach as
> > rust/kernel/block/mq/operations.rs.
> >
> > Sorry about the large diff. This changes the indentation of a large
> > amount of code.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  rust/kernel/miscdevice.rs | 295 ++++++++++++++++++++++------------------------
> >  1 file changed, 141 insertions(+), 154 deletions(-)
>
> This doesn't apply against a clean 6.14-rc2 tree, what is is made
> against?

I will rebase.

Are there any other miscdevice commits that have landed this cycle
that it might conflict with? If so, I can base it on your branch to
avoid such conflicts.

Alice

Re: [PATCH] rust: miscdevice: change how f_ops vtable is constructed

[Greg Kroah-Hartman]

On Tue, Feb 25, 2025 at 11:10:17AM +0100, Alice Ryhl wrote:
> On Wed, Feb 19, 2025 at 4:58 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Jan 17, 2025 at 02:22:32PM +0000, Alice Ryhl wrote:
> > > I was helping someone with writing a new Rust abstraction, and we were
> > > using the miscdevice abstraction as an example. While doing this, it
> > > became clear to me that the way I implemented the f_ops vtable is
> > > confusing to new Rust users, and that the approach used by the block
> > > abstractions is less confusing.
> > >
> > > Thus, update the miscdevice abstractions to use the same approach as
> > > rust/kernel/block/mq/operations.rs.
> > >
> > > Sorry about the large diff. This changes the indentation of a large
> > > amount of code.
> > >
> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > ---
> > >  rust/kernel/miscdevice.rs | 295 ++++++++++++++++++++++------------------------
> > >  1 file changed, 141 insertions(+), 154 deletions(-)
> >
> > This doesn't apply against a clean 6.14-rc2 tree, what is is made
> > against?
> 
> I will rebase.
> 
> Are there any other miscdevice commits that have landed this cycle
> that it might conflict with? If so, I can base it on your branch to
> avoid such conflicts.

Nope, I don't see any at the moment, or in my review queue, so it should
be fine.

thanks,

greg k-h

[PATCH] rust: miscdevice: change how f_ops vtable is constructed

[Alice Ryhl]

I was helping someone with writing a new Rust abstraction, and we were
using the miscdevice abstraction as an example. While doing this, it
became clear to me that the way I implemented the f_ops vtable is
confusing to new Rust users, and that the approach used by the block
abstractions is less confusing.

Thus, update the miscdevice abstractions to use the same approach as
rust/kernel/block/mq/operations.rs.

Sorry about the large diff. This changes the indentation of a large
amount of code.

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/miscdevice.rs | 297 ++++++++++++++++++++++------------------------
 1 file changed, 143 insertions(+), 154 deletions(-)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index e14433b2ab9d..fa9ecc42602a 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -35,7 +35,7 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
         let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
         result.minor = bindings::MISC_DYNAMIC_MINOR as _;
         result.name = self.name.as_char_ptr();
-        result.fops = create_vtable::<T>();
+        result.fops = MiscdeviceVTable::<T>::build();
         result
     }
 }
@@ -160,171 +160,160 @@ fn show_fdinfo(
     }
 }
 
-const fn create_vtable<T: MiscDevice>() -> &'static bindings::file_operations {
-    const fn maybe_fn<T: Copy>(check: bool, func: T) -> Option<T> {
-        if check {
-            Some(func)
-        } else {
-            None
+/// A vtable for the file operations of a Rust miscdevice.
+struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);
+
+impl<T: MiscDevice> MiscdeviceVTable<T> {
+    /// # Safety
+    ///
+    /// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
+    /// The file must be associated with a `MiscDeviceRegistration<T>`.
+    unsafe extern "C" fn open(inode: *mut bindings::inode, raw_file: *mut bindings::file) -> c_int {
+        // SAFETY: The pointers are valid and for a file being opened.
+        let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
+        if ret != 0 {
+            return ret;
         }
-    }
 
-    struct VtableHelper<T: MiscDevice> {
-        _t: PhantomData<T>,
-    }
-    impl<T: MiscDevice> VtableHelper<T> {
-        const VTABLE: bindings::file_operations = bindings::file_operations {
-            open: Some(fops_open::<T>),
-            release: Some(fops_release::<T>),
-            unlocked_ioctl: maybe_fn(T::HAS_IOCTL, fops_ioctl::<T>),
-            #[cfg(CONFIG_COMPAT)]
-            compat_ioctl: if T::HAS_COMPAT_IOCTL {
-                Some(fops_compat_ioctl::<T>)
-            } else if T::HAS_IOCTL {
-                Some(bindings::compat_ptr_ioctl)
-            } else {
-                None
-            },
-            show_fdinfo: maybe_fn(T::HAS_SHOW_FDINFO, fops_show_fdinfo::<T>),
-            // SAFETY: All zeros is a valid value for `bindings::file_operations`.
-            ..unsafe { MaybeUninit::zeroed().assume_init() }
-        };
-    }
+        // SAFETY: The open call of a file can access the private data.
+        let misc_ptr = unsafe { (*raw_file).private_data };
 
-    &VtableHelper::<T>::VTABLE
-}
+        // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
+        // associated `struct miscdevice` before calling into this method. Furthermore,
+        // `misc_open()` ensures that the miscdevice can't be unregistered and freed during this
+        // call to `fops_open`.
+        let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
 
-/// # Safety
-///
-/// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
-/// The file must be associated with a `MiscDeviceRegistration<T>`.
-unsafe extern "C" fn fops_open<T: MiscDevice>(
-    inode: *mut bindings::inode,
-    raw_file: *mut bindings::file,
-) -> c_int {
-    // SAFETY: The pointers are valid and for a file being opened.
-    let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
-    if ret != 0 {
-        return ret;
-    }
+        // SAFETY:
+        // * This underlying file is valid for (much longer than) the duration of `T::open`.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(raw_file) };
 
-    // SAFETY: The open call of a file can access the private data.
-    let misc_ptr = unsafe { (*raw_file).private_data };
-
-    // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
-    // associated `struct miscdevice` before calling into this method. Furthermore, `misc_open()`
-    // ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`.
-    let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
+        let ptr = match T::open(file, misc) {
+            Ok(ptr) => ptr,
+            Err(err) => return err.to_errno(),
+        };
 
-    // SAFETY:
-    // * This underlying file is valid for (much longer than) the duration of `T::open`.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(raw_file) };
+        // This overwrites the private data with the value specified by the user, changing the type
+        // of this file's private data. All future accesses to the private data is performed by
+        // other fops_* methods in this file, which all correctly cast the private data to the new
+        // type.
+        //
+        // SAFETY: The open call of a file can access the private data.
+        unsafe { (*raw_file).private_data = ptr.into_foreign() };
 
-    let ptr = match T::open(file, misc) {
-        Ok(ptr) => ptr,
-        Err(err) => return err.to_errno(),
-    };
-
-    // This overwrites the private data with the value specified by the user, changing the type of
-    // this file's private data. All future accesses to the private data is performed by other
-    // fops_* methods in this file, which all correctly cast the private data to the new type.
-    //
-    // SAFETY: The open call of a file can access the private data.
-    unsafe { (*raw_file).private_data = ptr.into_foreign() };
+        0
+    }
 
-    0
-}
+    /// # Safety
+    ///
+    /// `file` and `inode` must be the file and inode for a file that is being released. The file
+    /// must be associated with a `MiscDeviceRegistration<T>`.
+    unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int {
+        // SAFETY: The release call of a file owns the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: The release call of a file owns the private data.
+        let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
+
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        T::release(ptr, unsafe { File::from_raw_file(file) });
+
+        0
+    }
 
-/// # Safety
-///
-/// `file` and `inode` must be the file and inode for a file that is being released. The file must
-/// be associated with a `MiscDeviceRegistration<T>`.
-unsafe extern "C" fn fops_release<T: MiscDevice>(
-    _inode: *mut bindings::inode,
-    file: *mut bindings::file,
-) -> c_int {
-    // SAFETY: The release call of a file owns the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: The release call of a file owns the private data.
-    let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
-
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    T::release(ptr, unsafe { File::from_raw_file(file) });
-
-    0
-}
+    /// # Safety
+    ///
+    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
+    unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {
+        // SAFETY: The ioctl call of a file can access the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: Ioctl calls can borrow the private data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(file) };
+
+        match T::ioctl(device, file, cmd, arg) {
+            Ok(ret) => ret as c_long,
+            Err(err) => err.to_errno() as c_long,
+        }
+    }
 
-/// # Safety
-///
-/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
-unsafe extern "C" fn fops_ioctl<T: MiscDevice>(
-    file: *mut bindings::file,
-    cmd: c_uint,
-    arg: c_ulong,
-) -> c_long {
-    // SAFETY: The ioctl call of a file can access the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: Ioctl calls can borrow the private data of the file.
-    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
-
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(file) };
-
-    match T::ioctl(device, file, cmd, arg) {
-        Ok(ret) => ret as c_long,
-        Err(err) => err.to_errno() as c_long,
+    /// # Safety
+    ///
+    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
+    #[cfg(CONFIG_COMPAT)]
+    unsafe extern "C" fn compat_ioctl(
+        file: *mut bindings::file,
+        cmd: c_uint,
+        arg: c_ulong,
+    ) -> c_long {
+        // SAFETY: The compat ioctl call of a file can access the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: Ioctl calls can borrow the private data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(file) };
+
+        match T::compat_ioctl(device, file, cmd, arg) {
+            Ok(ret) => ret as c_long,
+            Err(err) => err.to_errno() as c_long,
+        }
     }
-}
 
-/// # Safety
-///
-/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
-#[cfg(CONFIG_COMPAT)]
-unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>(
-    file: *mut bindings::file,
-    cmd: c_uint,
-    arg: c_ulong,
-) -> c_long {
-    // SAFETY: The compat ioctl call of a file can access the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: Ioctl calls can borrow the private data of the file.
-    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
-
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(file) };
-
-    match T::compat_ioctl(device, file, cmd, arg) {
-        Ok(ret) => ret as c_long,
-        Err(err) => err.to_errno() as c_long,
+    /// # Safety
+    ///
+    /// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
+    /// - `seq_file` must be a valid `struct seq_file` that we can write to.
+    unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) {
+        // SAFETY: The release call of a file owns the private data.
+        let private = unsafe { (*file).private_data };
+        // SAFETY: Ioctl calls can borrow the private data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+        // SAFETY:
+        // * The file is valid for the duration of this call.
+        // * There is no active fdget_pos region on the file on this thread.
+        let file = unsafe { File::from_raw_file(file) };
+        // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in
+        // which this method is called.
+        let m = unsafe { SeqFile::from_raw(seq_file) };
+
+        T::show_fdinfo(device, m, file);
     }
-}
 
-/// # Safety
-///
-/// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
-/// - `seq_file` must be a valid `struct seq_file` that we can write to.
-unsafe extern "C" fn fops_show_fdinfo<T: MiscDevice>(
-    seq_file: *mut bindings::seq_file,
-    file: *mut bindings::file,
-) {
-    // SAFETY: The release call of a file owns the private data.
-    let private = unsafe { (*file).private_data };
-    // SAFETY: Ioctl calls can borrow the private data of the file.
-    let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
-    // SAFETY:
-    // * The file is valid for the duration of this call.
-    // * There is no active fdget_pos region on the file on this thread.
-    let file = unsafe { File::from_raw_file(file) };
-    // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which
-    // this method is called.
-    let m = unsafe { SeqFile::from_raw(seq_file) };
-
-    T::show_fdinfo(device, m, file);
+    const VTABLE: bindings::file_operations = bindings::file_operations {
+        open: Some(Self::open),
+        release: Some(Self::release),
+        unlocked_ioctl: if T::HAS_IOCTL {
+            Some(Self::ioctl)
+        } else {
+            None
+        },
+        #[cfg(CONFIG_COMPAT)]
+        compat_ioctl: if T::HAS_COMPAT_IOCTL {
+            Some(Self::compat_ioctl)
+        } else if T::HAS_IOCTL {
+            Some(bindings::compat_ptr_ioctl)
+        } else {
+            None
+        },
+        show_fdinfo: if T::HAS_SHOW_FDINFO {
+            Some(Self::show_fdinfo)
+        } else {
+            None
+        },
+        // SAFETY: All zeros is a valid value for `bindings::file_operations`.
+        ..unsafe { MaybeUninit::zeroed().assume_init() }
+    };
+
+    const fn build() -> &'static bindings::file_operations {
+        &Self::VTABLE
+    }
 }

---
base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
change-id: 20250117-miscdevice-fops-change-260352fd8957

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

Re: [PATCH] rust: miscdevice: change how f_ops vtable is constructed

[Alice Ryhl]

On Thu, Feb 27, 2025 at 2:23 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> I was helping someone with writing a new Rust abstraction, and we were
> using the miscdevice abstraction as an example. While doing this, it
> became clear to me that the way I implemented the f_ops vtable is
> confusing to new Rust users, and that the approach used by the block
> abstractions is less confusing.
>
> Thus, update the miscdevice abstractions to use the same approach as
> rust/kernel/block/mq/operations.rs.
>
> Sorry about the large diff. This changes the indentation of a large
> amount of code.
>
> Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

I think I must have misused b4 somehow, because this lost the v2 marker ...

This is version 2, and the previous version is here:
https://lore.kernel.org/all/20250117-miscdevice-fops-change-v1-1-ec04b701c076@google.com/

The only change is rebase on v6.14-rc2.

Alice