BACKPORT: rust: miscdevice: change how f_ops vtable is constructed

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>
Link: https://lore.kernel.org/r/20250227-miscdevice-fops-change-v1-1-c9e9b75d67eb@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Bug: 429146594
[ aliceryhl: also adjust llseek and similar to new style ]
(cherry picked from commit 74fc34937d72d04e89e4f75ea66147cdc9b785f5)
Change-Id: Ie87c5015d483a4c5a27ab17ae8a836a8956d1092
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
This commit is contained in:
Alice Ryhl
2025-02-27 13:22:31 +00:00
parent 1acd3b312f
commit 2ef6dbc73e
+70 -69
View File
@@ -39,7 +39,7 @@ impl MiscDeviceOptions {
let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() }; let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
result.minor = bindings::MISC_DYNAMIC_MINOR as _; result.minor = bindings::MISC_DYNAMIC_MINOR as _;
result.name = self.name.as_char_ptr(); result.name = self.name.as_char_ptr();
result.fops = create_vtable::<T>(); result.fops = MiscdeviceVTable::<T>::build();
result result
} }
} }
@@ -253,51 +253,15 @@ impl IovIter {
} }
} }
const fn create_vtable<T: MiscDevice>() -> &'static bindings::file_operations { /// A vtable for the file operations of a Rust miscdevice.
const fn maybe_fn<T: Copy>(check: bool, func: T) -> Option<T> { struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);
if check {
Some(func)
} else {
None
}
}
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>),
llseek: maybe_fn(T::HAS_LLSEEK, fops_llseek::<T>),
read_iter: maybe_fn(T::HAS_READ_ITER, fops_read_iter::<T>),
write_iter: maybe_fn(T::HAS_WRITE_ITER, fops_write_iter::<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() }
};
}
&VtableHelper::<T>::VTABLE
}
impl<T: MiscDevice> MiscdeviceVTable<T> {
/// # Safety /// # Safety
/// ///
/// `file` and `inode` must be the file and inode for a file that is undergoing initialization. /// `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>`. /// The file must be associated with a `MiscDeviceRegistration<T>`.
unsafe extern "C" fn fops_open<T: MiscDevice>( unsafe extern "C" fn open(inode: *mut bindings::inode, raw_file: *mut bindings::file) -> c_int {
inode: *mut bindings::inode,
raw_file: *mut bindings::file,
) -> c_int {
// SAFETY: The pointers are valid and for a file being opened. // SAFETY: The pointers are valid and for a file being opened.
let ret = unsafe { bindings::generic_file_open(inode, raw_file) }; let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
if ret != 0 { if ret != 0 {
@@ -308,8 +272,9 @@ unsafe extern "C" fn fops_open<T: MiscDevice>(
let misc_ptr = unsafe { (*raw_file).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 // 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()` // associated `struct miscdevice` before calling into this method. Furthermore,
// ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`. // `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 misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
// SAFETY: // SAFETY:
@@ -322,9 +287,10 @@ unsafe extern "C" fn fops_open<T: MiscDevice>(
Err(err) => return err.to_errno(), Err(err) => return err.to_errno(),
}; };
// This overwrites the private data with the value specified by the user, changing the type of // This overwrites the private data with the value specified by the user, changing the type
// this file's private data. All future accesses to the private data is performed by other // of this file's private data. All future accesses to the private data is performed by
// fops_* methods in this file, which all correctly cast the private data to the new type. // 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. // SAFETY: The open call of a file can access the private data.
unsafe { (*raw_file).private_data = ptr.into_foreign().cast_mut() }; unsafe { (*raw_file).private_data = ptr.into_foreign().cast_mut() };
@@ -334,12 +300,9 @@ unsafe extern "C" fn fops_open<T: MiscDevice>(
/// # Safety /// # Safety
/// ///
/// `file` and `inode` must be the file and inode for a file that is being released. The file must /// `file` and `inode` must be the file and inode for a file that is being released. The file
/// be associated with a `MiscDeviceRegistration<T>`. /// must be associated with a `MiscDeviceRegistration<T>`.
unsafe extern "C" fn fops_release<T: MiscDevice>( unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int {
_inode: *mut bindings::inode,
file: *mut bindings::file,
) -> c_int {
// SAFETY: The release call of a file owns the private data. // SAFETY: The release call of a file owns the private data.
let private = unsafe { (*file).private_data }; let private = unsafe { (*file).private_data };
// SAFETY: The release call of a file owns the private data. // SAFETY: The release call of a file owns the private data.
@@ -356,7 +319,7 @@ unsafe extern "C" fn fops_release<T: MiscDevice>(
/// # Safety /// # Safety
/// ///
/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`. /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
unsafe extern "C" fn fops_llseek<T: MiscDevice>( unsafe extern "C" fn llseek(
file: *mut bindings::file, file: *mut bindings::file,
offset: loff_t, offset: loff_t,
whence: c_int, whence: c_int,
@@ -380,7 +343,7 @@ unsafe extern "C" fn fops_llseek<T: MiscDevice>(
/// # Safety /// # Safety
/// ///
/// Arguments must be valid. /// Arguments must be valid.
unsafe extern "C" fn fops_read_iter<T: MiscDevice>( unsafe extern "C" fn read_iter(
kiocb: *mut bindings::kiocb, kiocb: *mut bindings::kiocb,
iter: *mut bindings::iov_iter, iter: *mut bindings::iov_iter,
) -> isize { ) -> isize {
@@ -399,7 +362,7 @@ unsafe extern "C" fn fops_read_iter<T: MiscDevice>(
/// # Safety /// # Safety
/// ///
/// Arguments must be valid. /// Arguments must be valid.
unsafe extern "C" fn fops_write_iter<T: MiscDevice>( unsafe extern "C" fn write_iter(
kiocb: *mut bindings::kiocb, kiocb: *mut bindings::kiocb,
iter: *mut bindings::iov_iter, iter: *mut bindings::iov_iter,
) -> isize { ) -> isize {
@@ -418,11 +381,7 @@ unsafe extern "C" fn fops_write_iter<T: MiscDevice>(
/// # Safety /// # Safety
/// ///
/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`. /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
unsafe extern "C" fn fops_ioctl<T: MiscDevice>( unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {
file: *mut bindings::file,
cmd: c_uint,
arg: c_ulong,
) -> c_long {
// SAFETY: The ioctl call of a file can access the private data. // SAFETY: The ioctl call of a file can access the private data.
let private = unsafe { (*file).private_data }; let private = unsafe { (*file).private_data };
// SAFETY: Ioctl calls can borrow the private data of the file. // SAFETY: Ioctl calls can borrow the private data of the file.
@@ -433,7 +392,7 @@ unsafe extern "C" fn fops_ioctl<T: MiscDevice>(
// * There is no active fdget_pos region on the file on this thread. // * There is no active fdget_pos region on the file on this thread.
let file = unsafe { File::from_raw_file(file) }; let file = unsafe { File::from_raw_file(file) };
match T::ioctl(device, file, cmd, arg as usize) { match T::ioctl(device, file, cmd, arg) {
Ok(ret) => ret as c_long, Ok(ret) => ret as c_long,
Err(err) => err.to_errno() as c_long, Err(err) => err.to_errno() as c_long,
} }
@@ -443,7 +402,7 @@ unsafe extern "C" fn fops_ioctl<T: MiscDevice>(
/// ///
/// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`. /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
#[cfg(CONFIG_COMPAT)] #[cfg(CONFIG_COMPAT)]
unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>( unsafe extern "C" fn compat_ioctl(
file: *mut bindings::file, file: *mut bindings::file,
cmd: c_uint, cmd: c_uint,
arg: c_ulong, arg: c_ulong,
@@ -458,7 +417,7 @@ unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>(
// * There is no active fdget_pos region on the file on this thread. // * There is no active fdget_pos region on the file on this thread.
let file = unsafe { File::from_raw_file(file) }; let file = unsafe { File::from_raw_file(file) };
match T::compat_ioctl(device, file, cmd, arg as usize) { match T::compat_ioctl(device, file, cmd, arg) {
Ok(ret) => ret as c_long, Ok(ret) => ret as c_long,
Err(err) => err.to_errno() as c_long, Err(err) => err.to_errno() as c_long,
} }
@@ -468,10 +427,7 @@ unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>(
/// ///
/// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`. /// - `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. /// - `seq_file` must be a valid `struct seq_file` that we can write to.
unsafe extern "C" fn fops_show_fdinfo<T: MiscDevice>( unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) {
seq_file: *mut bindings::seq_file,
file: *mut bindings::file,
) {
// SAFETY: The release call of a file owns the private data. // SAFETY: The release call of a file owns the private data.
let private = unsafe { (*file).private_data }; let private = unsafe { (*file).private_data };
// SAFETY: Ioctl calls can borrow the private data of the file. // SAFETY: Ioctl calls can borrow the private data of the file.
@@ -480,9 +436,54 @@ unsafe extern "C" fn fops_show_fdinfo<T: MiscDevice>(
// * The file is valid for the duration of this call. // * The file is valid for the duration of this call.
// * There is no active fdget_pos region on the file on this thread. // * There is no active fdget_pos region on the file on this thread.
let file = unsafe { File::from_raw_file(file) }; let file = unsafe { File::from_raw_file(file) };
// SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in
// this method is called. // which this method is called.
let m = unsafe { SeqFile::from_raw(seq_file) }; let m = unsafe { SeqFile::from_raw(seq_file) };
T::show_fdinfo(device, m, file); T::show_fdinfo(device, m, file);
} }
const VTABLE: bindings::file_operations = bindings::file_operations {
open: Some(Self::open),
release: Some(Self::release),
llseek: if T::HAS_LLSEEK {
Some(Self::llseek)
} else {
None
},
read_iter: if T::HAS_READ_ITER {
Some(Self::read_iter)
} else {
None
},
write_iter: if T::HAS_WRITE_ITER {
Some(Self::write_iter)
} else {
None
},
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
}
}