All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.