rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).