All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sidong Yang <sidong.yang@furiosa.ai>
To: Caleb Sander Mateos <csander@purestorage.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	Daniel Almeida <daniel.almeida@collabora.com>,
	Benno Lossin <lossin@kernel.org>, Miguel Ojeda <ojeda@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	io-uring@vger.kernel.org
Subject: Re: [RFC PATCH v3 4/5] rust: miscdevice: Add `uring_cmd` support
Date: Tue, 2 Sep 2025 20:18:35 +0900	[thread overview]
Message-ID: <aLbSi5-a67i78BHl@sidongui-MacBookPro.local> (raw)
In-Reply-To: <CADUfDZoDvAp1yqFyB_SQiynqQfOQPkO_mnQ_pWAXpZJESecFFw@mail.gmail.com>

On Mon, Sep 01, 2025 at 06:12:44PM -0700, Caleb Sander Mateos wrote:
> On Fri, Aug 22, 2025 at 5:56 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >
> > This patch introduces support for `uring_cmd` to the `miscdevice`
> > framework. This is achieved by adding a new `uring_cmd` method to the
> > `MiscDevice` trait and wiring it up to the corresponding
> > `file_operations` entry.
> >
> > The `uring_cmd` function provides a mechanism for `io_uring` to issue
> > commands to a device driver.
> >
> > The new `uring_cmd` method takes the device, an `IoUringCmd` object,
> > and issue flags as arguments. The `IoUringCmd` object is a safe Rust
> > abstraction around the raw `io_uring_cmd` struct.
> >
> > To enable `uring_cmd` for a specific misc device, the `HAS_URING_CMD`
> > constant must be set to `true` in the `MiscDevice` implementation.
> >
> > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > ---
> >  rust/kernel/miscdevice.rs | 53 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> > index 6373fe183b27..fcef579218ba 100644
> > --- a/rust/kernel/miscdevice.rs
> > +++ b/rust/kernel/miscdevice.rs
> > @@ -11,9 +11,10 @@
> >  use crate::{
> >      bindings,
> >      device::Device,
> > -    error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
> > +    error::{from_result, to_result, Error, Result, VTABLE_DEFAULT_ERROR},
> >      ffi::{c_int, c_long, c_uint, c_ulong},
> >      fs::File,
> > +    io_uring::IoUringCmd,
> >      mm::virt::VmaNew,
> >      prelude::*,
> >      seq_file::SeqFile,
> > @@ -180,6 +181,21 @@ fn show_fdinfo(
> >      ) {
> >          build_error!(VTABLE_DEFAULT_ERROR)
> >      }
> > +
> > +    /// Handler for uring_cmd.
> > +    ///
> > +    /// This function is invoked when userspace process submits an uring_cmd op
> > +    /// on io-uring submission queue. The `device` is borrowed instance defined
> > +    /// by `Ptr`. The `io_uring_cmd` would be used for get arguments cmd_op, sqe,
> > +    /// cmd_data. The `issue_flags` is the flags includes options for uring_cmd.
> > +    /// The options are listed in `kernel::io_uring::cmd_flags`.
> > +    fn uring_cmd(
> > +        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
> > +        _io_uring_cmd: Pin<&mut IoUringCmd>,
> 
> Passing the IoUringCmd by reference doesn't allow the uring_cmd()
> implementation to store it beyond the function return. That precludes
> any asynchronous uring_cmd() implementation, which is kind of the
> whole point of uring_cmd. I think uring_cmd() needs to transfer
> ownership of the IoUringCmd so the implementation can complete it
> asynchronously.

I didn't know that I can take IoUringCmd ownership and calling done(). 
In C implementation, is it safe to call `io_uring_cmd_done()` in any context?

> 
> Best,
> Caleb
> 
> > +        _issue_flags: u32,
> > +    ) -> Result<i32> {
> > +        build_error!(VTABLE_DEFAULT_ERROR)
> > +    }
> >  }
> >
> >  /// A vtable for the file operations of a Rust miscdevice.
> > @@ -337,6 +353,36 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
> >          T::show_fdinfo(device, m, file);
> >      }
> >
> > +    /// # Safety
> > +    ///
> > +    /// The caller must ensure that:
> > +    /// - The pointer `ioucmd` is not null and points to a valid `bindings::io_uring_cmd`.
> > +    unsafe extern "C" fn uring_cmd(
> > +        ioucmd: *mut bindings::io_uring_cmd,
> > +        issue_flags: ffi::c_uint,
> > +    ) -> c_int {
> > +        // SAFETY: `file` referenced by `ioucmd` is valid pointer. It's assigned in
> > +        // uring cmd preparation. So dereferencing is safe.
> > +        let raw_file = unsafe { (*ioucmd).file };
> > +
> > +        // SAFETY: `private_data` is guaranteed that it has valid pointer after
> > +        // this file opened. So dereferencing is safe.
> > +        let private = unsafe { (*raw_file).private_data }.cast();
> > +
> > +        // SAFETY: `ioucmd` is not null and points to valid memory `bindings::io_uring_cmd`
> > +        // and the memory pointed by `ioucmd` is valid and will not be moved or
> > +        // freed for the lifetime of returned value `ioucmd`
> > +        let ioucmd = unsafe { IoUringCmd::from_raw(ioucmd) };
> > +
> > +        // SAFETY: This call is safe because `private` is returned by
> > +        // `into_foreign` in [`open`]. And it's guaranteed
> > +        // that `from_foreign` is called by [`release`] after the end of
> > +        // the lifetime of `device`
> > +        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> > +
> > +        from_result(|| T::uring_cmd(device, ioucmd, issue_flags))
> > +    }
> > +
> >      const VTABLE: bindings::file_operations = bindings::file_operations {
> >          open: Some(Self::open),
> >          release: Some(Self::release),
> > @@ -359,6 +405,11 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
> >          } else {
> >              None
> >          },
> > +        uring_cmd: if T::HAS_URING_CMD {
> > +            Some(Self::uring_cmd)
> > +        } else {
> > +            None
> > +        },
> >          // SAFETY: All zeros is a valid value for `bindings::file_operations`.
> >          ..unsafe { MaybeUninit::zeroed().assume_init() }
> >      };
> > --
> > 2.43.0
> >

  reply	other threads:[~2025-09-02 11:18 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22 12:55 [RFC PATCH v3 0/5] rust: miscdevice: abstraction for uring_cmd Sidong Yang
2025-08-22 12:55 ` [RFC PATCH v3 1/5] rust: bindings: add io_uring headers in bindings_helper.h Sidong Yang
2025-08-22 12:55 ` [RFC PATCH v3 2/5] io_uring/cmd: zero-init pdu in io_uring_cmd_prep() to avoid UB Sidong Yang
2025-09-02  0:34   ` Caleb Sander Mateos
2025-09-02 10:23     ` Sidong Yang
2025-09-02 15:31       ` Caleb Sander Mateos
2025-09-06 14:28         ` Sidong Yang
2025-09-08 19:45           ` Caleb Sander Mateos
2025-09-09 14:43             ` Sidong Yang
2025-09-09 16:32               ` Caleb Sander Mateos
2025-09-12 16:41                 ` Sidong Yang
2025-09-12 17:56                   ` Caleb Sander Mateos
2025-09-13 12:42                     ` Sidong Yang
2025-09-15 16:54                       ` Caleb Sander Mateos
2025-09-17 14:56                         ` Sidong Yang
2025-09-22 18:09                           ` Caleb Sander Mateos
2025-08-22 12:55 ` [RFC PATCH v3 3/5] rust: io_uring: introduce rust abstraction for io-uring cmd Sidong Yang
2025-08-27 20:41   ` Daniel Almeida
2025-08-28  7:24     ` Benno Lossin
2025-08-29 15:43     ` Sidong Yang
2025-08-29 16:11       ` Daniel Almeida
2025-08-28  0:36   ` Ming Lei
2025-08-28  7:25     ` Benno Lossin
2025-08-28 10:05       ` Ming Lei
2025-09-02  1:11   ` Caleb Sander Mateos
2025-09-02 11:11     ` Sidong Yang
2025-09-02 15:41       ` Caleb Sander Mateos
2025-08-22 12:55 ` [RFC PATCH v3 4/5] rust: miscdevice: Add `uring_cmd` support Sidong Yang
2025-09-02  1:12   ` Caleb Sander Mateos
2025-09-02 11:18     ` Sidong Yang [this message]
2025-09-02 15:53       ` Caleb Sander Mateos
2025-08-22 12:55 ` [RFC PATCH v3 5/5] samples: rust: Add `uring_cmd` example to `rust_misc_device` Sidong Yang
2025-08-28  0:48   ` Ming Lei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aLbSi5-a67i78BHl@sidongui-MacBookPro.local \
    --to=sidong.yang@furiosa.ai \
    --cc=arnd@arndb.de \
    --cc=axboe@kernel.dk \
    --cc=csander@purestorage.com \
    --cc=daniel.almeida@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=io-uring@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.