* [PATCH v3 0/3] Additional miscdevice fops parameters
@ 2024-12-10 9:38 Alice Ryhl
2024-12-10 9:39 ` [PATCH v3 1/3] rust: miscdevice: access file in fops Alice Ryhl
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Alice Ryhl @ 2024-12-10 9:38 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman
Cc: Alexander Viro, Christian Brauner, Jan Kara, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Lee Jones,
Danilo Krummrich, rust-for-linux, linux-kernel, linux-fsdevel,
Alice Ryhl
This could not land with the base miscdevice abstractions due to the
dependency on File.
The last two patches enable you to use the `dev_*` macros to print
messages in miscdevice drivers.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v3:
- Fix build error in fops->open() patch.
- Improve wording of some comments in fops->open() patch.
- Update commit message with more info on why `struct miscdevice` is
only made available in fops->open() and not other hooks.
- Include Lee's device accessor patch, since it's a needed component to
use the `dev_*` printing macros with miscdevice.
- Link to v2: https://lore.kernel.org/r/20241209-miscdevice-file-param-v2-0-83ece27e9ff6@google.com
Changes in v2:
- Access the `struct miscdevice` from fops->open().
- Link to v1: https://lore.kernel.org/r/20241203-miscdevice-file-param-v1-1-1d6622978480@google.com
---
Alice Ryhl (2):
rust: miscdevice: access file in fops
rust: miscdevice: access the `struct miscdevice` from fops->open()
Lee Jones (1):
rust: miscdevice: Provide accessor to pull out miscdevice::this_device
rust/kernel/miscdevice.rs | 66 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 55 insertions(+), 11 deletions(-)
---
base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
change-id: 20241203-miscdevice-file-param-5df7f75861da
Best regards,
--
Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/3] rust: miscdevice: access file in fops
2024-12-10 9:38 [PATCH v3 0/3] Additional miscdevice fops parameters Alice Ryhl
@ 2024-12-10 9:39 ` Alice Ryhl
2024-12-11 11:56 ` Lee Jones
2024-12-10 9:39 ` [PATCH v3 2/3] rust: miscdevice: access the `struct miscdevice` from fops->open() Alice Ryhl
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Alice Ryhl @ 2024-12-10 9:39 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman
Cc: Alexander Viro, Christian Brauner, Jan Kara, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Lee Jones,
Danilo Krummrich, rust-for-linux, linux-kernel, linux-fsdevel,
Alice Ryhl
This allows fops to access information about the underlying struct file
for the miscdevice. For example, the Binder driver needs to inspect the
O_NONBLOCK flag inside the fops->ioctl() hook.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/miscdevice.rs | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 7e2a79b3ae26..0cb79676c139 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -11,6 +11,7 @@
use crate::{
bindings,
error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
+ fs::File,
prelude::*,
str::CStr,
types::{ForeignOwnable, Opaque},
@@ -103,10 +104,10 @@ pub trait MiscDevice {
/// Called when the misc device is opened.
///
/// The returned pointer will be stored as the private data for the file.
- fn open() -> Result<Self::Ptr>;
+ fn open(_file: &File) -> Result<Self::Ptr>;
/// Called when the misc device is released.
- fn release(device: Self::Ptr) {
+ fn release(device: Self::Ptr, _file: &File) {
drop(device);
}
@@ -117,6 +118,7 @@ fn release(device: Self::Ptr) {
/// [`kernel::ioctl`]: mod@crate::ioctl
fn ioctl(
_device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+ _file: &File,
_cmd: u32,
_arg: usize,
) -> Result<isize> {
@@ -133,6 +135,7 @@ fn ioctl(
#[cfg(CONFIG_COMPAT)]
fn compat_ioctl(
_device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+ _file: &File,
_cmd: u32,
_arg: usize,
) -> Result<isize> {
@@ -187,7 +190,10 @@ impl<T: MiscDevice> VtableHelper<T> {
return ret;
}
- let ptr = match T::open() {
+ // 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 ptr = match T::open(unsafe { File::from_raw_file(file) }) {
Ok(ptr) => ptr,
Err(err) => return err.to_errno(),
};
@@ -211,7 +217,10 @@ impl<T: MiscDevice> VtableHelper<T> {
// SAFETY: The release call of a file owns the private data.
let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
- T::release(ptr);
+ // 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
}
@@ -229,7 +238,12 @@ impl<T: MiscDevice> VtableHelper<T> {
// SAFETY: Ioctl calls can borrow the private data of the file.
let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
- match T::ioctl(device, cmd, arg as usize) {
+ // 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,
}
@@ -249,7 +263,12 @@ impl<T: MiscDevice> VtableHelper<T> {
// SAFETY: Ioctl calls can borrow the private data of the file.
let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
- match T::compat_ioctl(device, cmd, arg as usize) {
+ // 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,
}
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] rust: miscdevice: access the `struct miscdevice` from fops->open()
2024-12-10 9:38 [PATCH v3 0/3] Additional miscdevice fops parameters Alice Ryhl
2024-12-10 9:39 ` [PATCH v3 1/3] rust: miscdevice: access file in fops Alice Ryhl
@ 2024-12-10 9:39 ` Alice Ryhl
2024-12-11 11:57 ` Lee Jones
2024-12-10 9:39 ` [PATCH v3 3/3] rust: miscdevice: Provide accessor to pull out miscdevice::this_device Alice Ryhl
2024-12-16 12:11 ` [PATCH v3 0/3] Additional miscdevice fops parameters Danilo Krummrich
3 siblings, 1 reply; 10+ messages in thread
From: Alice Ryhl @ 2024-12-10 9:39 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman
Cc: Alexander Viro, Christian Brauner, Jan Kara, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Lee Jones,
Danilo Krummrich, rust-for-linux, linux-kernel, linux-fsdevel,
Alice Ryhl
Providing access to the underlying `struct miscdevice` is useful for
various reasons. For example, this allows you access the miscdevice's
internal `struct device` for use with the `dev_*` printing macros.
Note that since the underlying `struct miscdevice` could get freed at
any point after the fops->open() call (if misc_deregister is called),
only the open call is given access to it. To use `dev_*` printing macros
from other fops hooks, take a refcount on `miscdevice->this_device` to
keep it alive. See the linked thread for further discussion on the
lifetime of `struct miscdevice`.
Link: https://lore.kernel.org/r/2024120951-botanist-exhale-4845@gregkh
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/miscdevice.rs | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 0cb79676c139..75a9d26c8001 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -97,14 +97,14 @@ fn drop(self: Pin<&mut Self>) {
/// Trait implemented by the private data of an open misc device.
#[vtable]
-pub trait MiscDevice {
+pub trait MiscDevice: Sized {
/// What kind of pointer should `Self` be wrapped in.
type Ptr: ForeignOwnable + Send + Sync;
/// Called when the misc device is opened.
///
/// The returned pointer will be stored as the private data for the file.
- fn open(_file: &File) -> Result<Self::Ptr>;
+ fn open(_file: &File, _misc: &MiscDeviceRegistration<Self>) -> Result<Self::Ptr>;
/// Called when the misc device is released.
fn release(device: Self::Ptr, _file: &File) {
@@ -182,24 +182,38 @@ impl<T: MiscDevice> VtableHelper<T> {
/// The file must be associated with a `MiscDeviceRegistration<T>`.
unsafe extern "C" fn fops_open<T: MiscDevice>(
inode: *mut bindings::inode,
- file: *mut bindings::file,
+ 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, file) };
+ let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
if ret != 0 {
return ret;
}
+ // 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>>() };
+
// SAFETY:
- // * The file is valid for the duration of this call.
+ // * 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 ptr = match T::open(unsafe { File::from_raw_file(file) }) {
+ let file = unsafe { File::from_raw_file(raw_file) };
+
+ let ptr = match T::open(file, misc) {
Ok(ptr) => ptr,
Err(err) => return err.to_errno(),
};
- // SAFETY: The open call of a file owns the private data.
- unsafe { (*file).private_data = ptr.into_foreign().cast_mut() };
+ // 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
}
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] rust: miscdevice: Provide accessor to pull out miscdevice::this_device
2024-12-10 9:38 [PATCH v3 0/3] Additional miscdevice fops parameters Alice Ryhl
2024-12-10 9:39 ` [PATCH v3 1/3] rust: miscdevice: access file in fops Alice Ryhl
2024-12-10 9:39 ` [PATCH v3 2/3] rust: miscdevice: access the `struct miscdevice` from fops->open() Alice Ryhl
@ 2024-12-10 9:39 ` Alice Ryhl
2024-12-13 16:48 ` Lee Jones
2024-12-16 12:11 ` [PATCH v3 0/3] Additional miscdevice fops parameters Danilo Krummrich
3 siblings, 1 reply; 10+ messages in thread
From: Alice Ryhl @ 2024-12-10 9:39 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman
Cc: Alexander Viro, Christian Brauner, Jan Kara, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Lee Jones,
Danilo Krummrich, rust-for-linux, linux-kernel, linux-fsdevel,
Alice Ryhl
From: Lee Jones <lee@kernel.org>
There are situations where a pointer to a `struct device` will become
necessary (e.g. for calling into dev_*() functions). This accessor
allows callers to pull this out from the `struct miscdevice`.
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/miscdevice.rs | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 75a9d26c8001..20895e809607 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -10,6 +10,7 @@
use crate::{
bindings,
+ device::Device,
error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
fs::File,
prelude::*,
@@ -85,6 +86,16 @@ pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
pub fn as_raw(&self) -> *mut bindings::miscdevice {
self.inner.get()
}
+
+ /// Access the `this_device` field.
+ pub fn device(&self) -> &Device {
+ // SAFETY: This can only be called after a successful register(), which always
+ // initialises `this_device` with a valid device. Furthermore, the signature of this
+ // function tells the borrow-checker that the `&Device` reference must not outlive the
+ // `&MiscDeviceRegistration<T>` used to obtain it, so the last use of the reference must be
+ // before the underlying `struct miscdevice` is destroyed.
+ unsafe { Device::as_ref((*self.as_raw()).this_device) }
+ }
}
#[pinned_drop]
--
2.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] rust: miscdevice: access file in fops
2024-12-10 9:39 ` [PATCH v3 1/3] rust: miscdevice: access file in fops Alice Ryhl
@ 2024-12-11 11:56 ` Lee Jones
2024-12-13 16:48 ` Lee Jones
0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2024-12-11 11:56 UTC (permalink / raw)
To: Alice Ryhl
Cc: Arnd Bergmann, Greg Kroah-Hartman, Alexander Viro,
Christian Brauner, Jan Kara, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, rust-for-linux,
linux-kernel, linux-fsdevel
On Tue, 10 Dec 2024, Alice Ryhl wrote:
> This allows fops to access information about the underlying struct file
> for the miscdevice. For example, the Binder driver needs to inspect the
> O_NONBLOCK flag inside the fops->ioctl() hook.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/miscdevice.rs | 31 +++++++++++++++++++++++++------
> 1 file changed, 25 insertions(+), 6 deletions(-)
Reviewed-by: Lee Jones <lee@kernel.org>
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index 7e2a79b3ae26..0cb79676c139 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -11,6 +11,7 @@
> use crate::{
> bindings,
> error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
> + fs::File,
> prelude::*,
> str::CStr,
> types::{ForeignOwnable, Opaque},
> @@ -103,10 +104,10 @@ pub trait MiscDevice {
> /// Called when the misc device is opened.
> ///
> /// The returned pointer will be stored as the private data for the file.
> - fn open() -> Result<Self::Ptr>;
> + fn open(_file: &File) -> Result<Self::Ptr>;
>
> /// Called when the misc device is released.
> - fn release(device: Self::Ptr) {
> + fn release(device: Self::Ptr, _file: &File) {
> drop(device);
> }
>
> @@ -117,6 +118,7 @@ fn release(device: Self::Ptr) {
> /// [`kernel::ioctl`]: mod@crate::ioctl
> fn ioctl(
> _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
> + _file: &File,
> _cmd: u32,
> _arg: usize,
> ) -> Result<isize> {
> @@ -133,6 +135,7 @@ fn ioctl(
> #[cfg(CONFIG_COMPAT)]
> fn compat_ioctl(
> _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
> + _file: &File,
> _cmd: u32,
> _arg: usize,
> ) -> Result<isize> {
> @@ -187,7 +190,10 @@ impl<T: MiscDevice> VtableHelper<T> {
> return ret;
> }
>
> - let ptr = match T::open() {
> + // 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 ptr = match T::open(unsafe { File::from_raw_file(file) }) {
> Ok(ptr) => ptr,
> Err(err) => return err.to_errno(),
> };
> @@ -211,7 +217,10 @@ impl<T: MiscDevice> VtableHelper<T> {
> // SAFETY: The release call of a file owns the private data.
> let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
>
> - T::release(ptr);
> + // 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
> }
> @@ -229,7 +238,12 @@ impl<T: MiscDevice> VtableHelper<T> {
> // SAFETY: Ioctl calls can borrow the private data of the file.
> let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
>
> - match T::ioctl(device, cmd, arg as usize) {
> + // 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,
> }
> @@ -249,7 +263,12 @@ impl<T: MiscDevice> VtableHelper<T> {
> // SAFETY: Ioctl calls can borrow the private data of the file.
> let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
>
> - match T::compat_ioctl(device, cmd, arg as usize) {
> + // 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,
> }
>
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] rust: miscdevice: access the `struct miscdevice` from fops->open()
2024-12-10 9:39 ` [PATCH v3 2/3] rust: miscdevice: access the `struct miscdevice` from fops->open() Alice Ryhl
@ 2024-12-11 11:57 ` Lee Jones
2024-12-13 16:47 ` Lee Jones
0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2024-12-11 11:57 UTC (permalink / raw)
To: Alice Ryhl
Cc: Arnd Bergmann, Greg Kroah-Hartman, Alexander Viro,
Christian Brauner, Jan Kara, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, rust-for-linux,
linux-kernel, linux-fsdevel
On Tue, 10 Dec 2024, Alice Ryhl wrote:
> Providing access to the underlying `struct miscdevice` is useful for
> various reasons. For example, this allows you access the miscdevice's
> internal `struct device` for use with the `dev_*` printing macros.
>
> Note that since the underlying `struct miscdevice` could get freed at
> any point after the fops->open() call (if misc_deregister is called),
> only the open call is given access to it. To use `dev_*` printing macros
> from other fops hooks, take a refcount on `miscdevice->this_device` to
> keep it alive. See the linked thread for further discussion on the
> lifetime of `struct miscdevice`.
>
> Link: https://lore.kernel.org/r/2024120951-botanist-exhale-4845@gregkh
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/miscdevice.rs | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
Reviewed-by: Lee Jones <lee@kernel.org>
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index 0cb79676c139..75a9d26c8001 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -97,14 +97,14 @@ fn drop(self: Pin<&mut Self>) {
>
> /// Trait implemented by the private data of an open misc device.
> #[vtable]
> -pub trait MiscDevice {
> +pub trait MiscDevice: Sized {
> /// What kind of pointer should `Self` be wrapped in.
> type Ptr: ForeignOwnable + Send + Sync;
>
> /// Called when the misc device is opened.
> ///
> /// The returned pointer will be stored as the private data for the file.
> - fn open(_file: &File) -> Result<Self::Ptr>;
> + fn open(_file: &File, _misc: &MiscDeviceRegistration<Self>) -> Result<Self::Ptr>;
>
> /// Called when the misc device is released.
> fn release(device: Self::Ptr, _file: &File) {
> @@ -182,24 +182,38 @@ impl<T: MiscDevice> VtableHelper<T> {
> /// The file must be associated with a `MiscDeviceRegistration<T>`.
> unsafe extern "C" fn fops_open<T: MiscDevice>(
> inode: *mut bindings::inode,
> - file: *mut bindings::file,
> + 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, file) };
> + let ret = unsafe { bindings::generic_file_open(inode, raw_file) };
> if ret != 0 {
> return ret;
> }
>
> + // 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>>() };
> +
> // SAFETY:
> - // * The file is valid for the duration of this call.
> + // * 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 ptr = match T::open(unsafe { File::from_raw_file(file) }) {
> + let file = unsafe { File::from_raw_file(raw_file) };
> +
> + let ptr = match T::open(file, misc) {
> Ok(ptr) => ptr,
> Err(err) => return err.to_errno(),
> };
>
> - // SAFETY: The open call of a file owns the private data.
> - unsafe { (*file).private_data = ptr.into_foreign().cast_mut() };
> + // 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
> }
>
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] rust: miscdevice: access the `struct miscdevice` from fops->open()
2024-12-11 11:57 ` Lee Jones
@ 2024-12-13 16:47 ` Lee Jones
0 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2024-12-13 16:47 UTC (permalink / raw)
To: Alice Ryhl
Cc: Arnd Bergmann, Greg Kroah-Hartman, Alexander Viro,
Christian Brauner, Jan Kara, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, rust-for-linux,
linux-kernel, linux-fsdevel
On Wed, 11 Dec 2024, Lee Jones wrote:
> On Tue, 10 Dec 2024, Alice Ryhl wrote:
>
> > Providing access to the underlying `struct miscdevice` is useful for
> > various reasons. For example, this allows you access the miscdevice's
> > internal `struct device` for use with the `dev_*` printing macros.
> >
> > Note that since the underlying `struct miscdevice` could get freed at
> > any point after the fops->open() call (if misc_deregister is called),
> > only the open call is given access to it. To use `dev_*` printing macros
> > from other fops hooks, take a refcount on `miscdevice->this_device` to
> > keep it alive. See the linked thread for further discussion on the
> > lifetime of `struct miscdevice`.
> >
> > Link: https://lore.kernel.org/r/2024120951-botanist-exhale-4845@gregkh
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > rust/kernel/miscdevice.rs | 30 ++++++++++++++++++++++--------
> > 1 file changed, 22 insertions(+), 8 deletions(-)
>
> Reviewed-by: Lee Jones <lee@kernel.org>
Tested-by: Lee Jones <lee@kernel.org>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] rust: miscdevice: access file in fops
2024-12-11 11:56 ` Lee Jones
@ 2024-12-13 16:48 ` Lee Jones
0 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2024-12-13 16:48 UTC (permalink / raw)
To: Alice Ryhl
Cc: Arnd Bergmann, Greg Kroah-Hartman, Alexander Viro,
Christian Brauner, Jan Kara, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, rust-for-linux,
linux-kernel, linux-fsdevel
jn Wed, 11 Dec 2024, Lee Jones wrote:
> On Tue, 10 Dec 2024, Alice Ryhl wrote:
>
> > This allows fops to access information about the underlying struct file
> > for the miscdevice. For example, the Binder driver needs to inspect the
> > O_NONBLOCK flag inside the fops->ioctl() hook.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > rust/kernel/miscdevice.rs | 31 +++++++++++++++++++++++++------
> > 1 file changed, 25 insertions(+), 6 deletions(-)
>
> Reviewed-by: Lee Jones <lee@kernel.org>
Tested-by: Lee Jones <lee@kernel.org>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] rust: miscdevice: Provide accessor to pull out miscdevice::this_device
2024-12-10 9:39 ` [PATCH v3 3/3] rust: miscdevice: Provide accessor to pull out miscdevice::this_device Alice Ryhl
@ 2024-12-13 16:48 ` Lee Jones
0 siblings, 0 replies; 10+ messages in thread
From: Lee Jones @ 2024-12-13 16:48 UTC (permalink / raw)
To: Alice Ryhl
Cc: Arnd Bergmann, Greg Kroah-Hartman, Alexander Viro,
Christian Brauner, Jan Kara, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, rust-for-linux,
linux-kernel, linux-fsdevel
On Tue, 10 Dec 2024, Alice Ryhl wrote:
> From: Lee Jones <lee@kernel.org>
>
> There are situations where a pointer to a `struct device` will become
> necessary (e.g. for calling into dev_*() functions). This accessor
> allows callers to pull this out from the `struct miscdevice`.
>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/miscdevice.rs | 11 +++++++++++
> 1 file changed, 11 insertions(+)
This might be superfluous, but:
Tested-by: Lee Jones <lee@kernel.org>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/3] Additional miscdevice fops parameters
2024-12-10 9:38 [PATCH v3 0/3] Additional miscdevice fops parameters Alice Ryhl
` (2 preceding siblings ...)
2024-12-10 9:39 ` [PATCH v3 3/3] rust: miscdevice: Provide accessor to pull out miscdevice::this_device Alice Ryhl
@ 2024-12-16 12:11 ` Danilo Krummrich
3 siblings, 0 replies; 10+ messages in thread
From: Danilo Krummrich @ 2024-12-16 12:11 UTC (permalink / raw)
To: Alice Ryhl
Cc: Arnd Bergmann, Greg Kroah-Hartman, Alexander Viro,
Christian Brauner, Jan Kara, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Lee Jones, rust-for-linux,
linux-kernel, linux-fsdevel
On Tue, Dec 10, 2024 at 09:38:59AM +0000, Alice Ryhl wrote:
> This could not land with the base miscdevice abstractions due to the
> dependency on File.
>
> The last two patches enable you to use the `dev_*` macros to print
> messages in miscdevice drivers.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
For the series,
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
> ---
> Changes in v3:
> - Fix build error in fops->open() patch.
> - Improve wording of some comments in fops->open() patch.
> - Update commit message with more info on why `struct miscdevice` is
> only made available in fops->open() and not other hooks.
> - Include Lee's device accessor patch, since it's a needed component to
> use the `dev_*` printing macros with miscdevice.
> - Link to v2: https://lore.kernel.org/r/20241209-miscdevice-file-param-v2-0-83ece27e9ff6@google.com
>
> Changes in v2:
> - Access the `struct miscdevice` from fops->open().
> - Link to v1: https://lore.kernel.org/r/20241203-miscdevice-file-param-v1-1-1d6622978480@google.com
>
> ---
> Alice Ryhl (2):
> rust: miscdevice: access file in fops
> rust: miscdevice: access the `struct miscdevice` from fops->open()
>
> Lee Jones (1):
> rust: miscdevice: Provide accessor to pull out miscdevice::this_device
>
> rust/kernel/miscdevice.rs | 66 +++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 55 insertions(+), 11 deletions(-)
> ---
> base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
> change-id: 20241203-miscdevice-file-param-5df7f75861da
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-12-16 12:11 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-10 9:38 [PATCH v3 0/3] Additional miscdevice fops parameters Alice Ryhl
2024-12-10 9:39 ` [PATCH v3 1/3] rust: miscdevice: access file in fops Alice Ryhl
2024-12-11 11:56 ` Lee Jones
2024-12-13 16:48 ` Lee Jones
2024-12-10 9:39 ` [PATCH v3 2/3] rust: miscdevice: access the `struct miscdevice` from fops->open() Alice Ryhl
2024-12-11 11:57 ` Lee Jones
2024-12-13 16:47 ` Lee Jones
2024-12-10 9:39 ` [PATCH v3 3/3] rust: miscdevice: Provide accessor to pull out miscdevice::this_device Alice Ryhl
2024-12-13 16:48 ` Lee Jones
2024-12-16 12:11 ` [PATCH v3 0/3] Additional miscdevice fops parameters Danilo Krummrich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).