[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aLbSi5-a67i78BHl@sidongui-MacBookPro.local>
Date: Tue, 2 Sep 2025 20:18:35 +0900
From: Sidong Yang <sidong.yang@...iosa.ai>
To: Caleb Sander Mateos <csander@...estorage.com>
Cc: Jens Axboe <axboe@...nel.dk>,
Daniel Almeida <daniel.almeida@...labora.com>,
Benno Lossin <lossin@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
io-uring@...r.kernel.org
Subject: Re: [RFC PATCH v3 4/5] rust: miscdevice: Add `uring_cmd` support
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@...iosa.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@...iosa.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
> >
Powered by blists - more mailing lists