[RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Sidong Yang]

This patch series implemens an abstraction for io-uring sqe and cmd and
adds uring_cmd callback for miscdevice. Also there is an example that use
uring_cmd in rust-miscdevice sample.

Sidong Yang (4):
  rust: bindings: add io_uring headers in bindings_helper.h
  rust: io_uring: introduce rust abstraction for io-uring cmd
  rust: miscdevice: add uring_cmd() for MiscDevice trait
  samples: rust: rust_misc_device: add uring_cmd example

 rust/bindings/bindings_helper.h  |   2 +
 rust/kernel/io_uring.rs          | 114 +++++++++++++++++++++++++++++++
 rust/kernel/lib.rs               |   1 +
 rust/kernel/miscdevice.rs        |  34 +++++++++
 samples/rust/rust_misc_device.rs |  30 ++++++++
 5 files changed, 181 insertions(+)
 create mode 100644 rust/kernel/io_uring.rs

-- 
2.43.0

[PATCH 1/4] rust: bindings: add io_uring headers in bindings_helper.h

[Sidong Yang]

This patch adds two headers io_uring.h io_uring/cmd.h in bindings_helper
for implementing rust io_uring abstraction.

Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
 rust/bindings/bindings_helper.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 8cbb660e2ec2..a41205e2b8b8 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -72,6 +72,8 @@
 #include <linux/wait.h>
 #include <linux/workqueue.h>
 #include <linux/xarray.h>
+#include <linux/io_uring.h>
+#include <linux/io_uring/cmd.h>
 #include <trace/events/rust_sample.h>
 
 #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
-- 
2.43.0

[PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Sidong Yang]

This patch introduces rust abstraction for io-uring sqe, cmd. IoUringSqe
abstracts io_uring_sqe and it has cmd_data(). and IoUringCmd is
abstraction for io_uring_cmd. From this, user can get cmd_op, flags,
pdu and also sqe.

Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
 rust/kernel/io_uring.rs | 114 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs      |   1 +
 2 files changed, 115 insertions(+)
 create mode 100644 rust/kernel/io_uring.rs

diff --git a/rust/kernel/io_uring.rs b/rust/kernel/io_uring.rs
new file mode 100644
index 000000000000..7843effbedb4
--- /dev/null
+++ b/rust/kernel/io_uring.rs
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Furiosa AI.
+
+//! Files and file descriptors.
+//!
+//! C headers: [`include/linux/io_uring/cmd.h`](srctree/include/linux/io_uring/cmd.h) and
+//! [`include/linux/file.h`](srctree/include/linux/file.h)
+
+use core::mem::MaybeUninit;
+
+use crate::{fs::File, types::Opaque};
+
+pub mod flags {
+    pub const COMPLETE_DEFER: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPLETE_DEFER;
+    pub const UNLOCKED: i32 = bindings::io_uring_cmd_flags_IO_URING_F_UNLOCKED;
+
+    pub const MULTISHOT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_MULTISHOT;
+    pub const IOWQ: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOWQ;
+    pub const NONBLOCK: i32 = bindings::io_uring_cmd_flags_IO_URING_F_NONBLOCK;
+
+    pub const SQE128: i32 = bindings::io_uring_cmd_flags_IO_URING_F_SQE128;
+    pub const CQE32: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CQE32;
+    pub const IOPOLL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOPOLL;
+
+    pub const CANCEL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CANCEL;
+    pub const COMPAT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPAT;
+    pub const TASK_DEAD: i32 = bindings::io_uring_cmd_flags_IO_URING_F_TASK_DEAD;
+}
+
+#[repr(transparent)]
+pub struct IoUringCmd {
+    inner: Opaque<bindings::io_uring_cmd>,
+}
+
+impl IoUringCmd {
+    /// Returns the cmd_op with associated with the io_uring_cmd.
+    #[inline]
+    pub fn cmd_op(&self) -> u32 {
+        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
+        unsafe { (*self.inner.get()).cmd_op }
+    }
+
+    /// Returns the flags with associated with the io_uring_cmd.
+    #[inline]
+    pub fn flags(&self) -> u32 {
+        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
+        unsafe { (*self.inner.get()).flags }
+    }
+
+    /// Returns the ref pdu for free use.
+    #[inline]
+    pub fn pdu(&mut self) -> MaybeUninit<&mut [u8; 32]> {
+        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
+        unsafe { MaybeUninit::new(&mut (*self.inner.get()).pdu) }
+    }
+
+    /// Constructs a new `struct io_uring_cmd` wrapper from a file descriptor.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *const bindings::io_uring_cmd) -> &'a IoUringCmd {
+        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
+        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
+        unsafe { &*ptr.cast() }
+    }
+
+    // Returns the file that referenced by uring cmd self.
+    #[inline]
+    pub fn file<'a>(&'a self) -> &'a File {
+        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
+        let file = unsafe { (*self.inner.get()).file };
+        unsafe { File::from_raw_file(file) }
+    }
+
+    // Returns the sqe  that referenced by uring cmd self.
+    #[inline]
+    pub fn sqe(&self) -> &IoUringSqe {
+        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
+        let ptr = unsafe { (*self.inner.get()).sqe };
+        unsafe { IoUringSqe::from_raw(ptr) }
+    }
+
+    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
+    #[inline]
+    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
+        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
+        unsafe {
+            bindings::io_uring_cmd_done(self.inner.get(), ret, res2, issue_flags);
+        }
+    }
+}
+
+#[repr(transparent)]
+pub struct IoUringSqe {
+    inner: Opaque<bindings::io_uring_sqe>,
+}
+
+impl<'a> IoUringSqe {
+    pub fn cmd_data(&'a self) -> &'a [Opaque<u8>] {
+        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
+        unsafe {
+            let cmd = (*self.inner.get()).__bindgen_anon_6.cmd.as_ref();
+            core::slice::from_raw_parts(cmd.as_ptr() as *const Opaque<u8>, 8)
+        }
+    }
+
+    #[inline]
+    pub unsafe fn from_raw(ptr: *const bindings::io_uring_sqe) -> &'a IoUringSqe {
+        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
+        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
+        //
+        // INVARIANT: The caller guarantees that there are no problematic `fdget_pos` calls.
+        unsafe { &*ptr.cast() }
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c3..fb310e78d51d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -80,6 +80,7 @@
 pub mod fs;
 pub mod init;
 pub mod io;
+pub mod io_uring;
 pub mod ioctl;
 pub mod jump_label;
 #[cfg(CONFIG_KUNIT)]
-- 
2.43.0

[PATCH 3/4] rust: miscdevice: add uring_cmd() for MiscDevice trait

[Sidong Yang]

This patch adds uring_cmd() function for MiscDevice trait and its
callback implementation. It uses IoUringCmd that io_uring_cmd rust
abstraction.

Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
 rust/kernel/miscdevice.rs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 288f40e79906..5255faf27934 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -14,6 +14,7 @@
     error::{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,
@@ -175,6 +176,15 @@ fn show_fdinfo(
     ) {
         build_error!(VTABLE_DEFAULT_ERROR)
     }
+
+    fn uring_cmd(
+        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        _file: &File,
+        _io_uring_cmd: &IoUringCmd,
+        issue_flags: u32,
+    ) -> Result<isize> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
 }
 
 /// A vtable for the file operations of a Rust miscdevice.
@@ -332,6 +342,25 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
         T::show_fdinfo(device, m, file);
     }
 
+    unsafe extern "C" fn uring_cmd(
+        ioucmd: *mut bindings::io_uring_cmd,
+        issue_flags: ffi::c_uint,
+    ) -> ffi::c_int {
+        // SAFETY: The file is valid for the duration of this call.
+        let ioucmd = unsafe { IoUringCmd::from_raw(ioucmd) };
+        let file = ioucmd.file();
+
+        // SAFETY: The file is valid for the duration of this call.
+        let private = unsafe { (*file.as_ptr()).private_data }.cast();
+        // SAFETY: Ioctl calls can borrow the private data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+
+        match T::uring_cmd(device, file, ioucmd, issue_flags) {
+            Ok(ret) => ret as ffi::c_int,
+            Err(err) => err.to_errno() as ffi::c_int,
+        }
+    }
+
     const VTABLE: bindings::file_operations = bindings::file_operations {
         open: Some(Self::open),
         release: Some(Self::release),
@@ -354,6 +383,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

[PATCH 4/4] samples: rust: rust_misc_device: add uring_cmd example

[Sidong Yang]

This patch makes rust_misc_device handle uring_cmd. Command ops are like
ioctl that set or get values in simple way.

Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
---
 samples/rust/rust_misc_device.rs | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index c881fd6dbd08..cd0e578231d2 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -101,6 +101,7 @@
     c_str,
     device::Device,
     fs::File,
+    io_uring::IoUringCmd,
     ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
     miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
     new_mutex,
@@ -114,6 +115,9 @@
 const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('|' as u32, 0x81);
 const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('|' as u32, 0x82);
 
+const RUST_MISC_DEV_URING_CMD_SET_VALUE: u32 = 0x83;
+const RUST_MISC_DEV_URING_CMD_GET_VALUE: u32 = 0x84;
+
 module! {
     type: RustMiscDeviceModule,
     name: "rust_misc_device",
@@ -190,6 +194,32 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
 
         Ok(0)
     }
+
+    fn uring_cmd(
+        me: Pin<&RustMiscDevice>,
+        _file: &File,
+        io_uring_cmd: &IoUringCmd,
+        _issue_flags: u32,
+    ) -> Result<isize> {
+        dev_info!(me.dev, "UringCmd Rust Misc Device Sample\n");
+        let cmd = io_uring_cmd.cmd_op();
+        let cmd_data = io_uring_cmd.sqe().cmd_data().as_ptr() as *const usize;
+        let addr = unsafe { *cmd_data };
+
+        match cmd {
+            RUST_MISC_DEV_URING_CMD_SET_VALUE => {
+                me.set_value(UserSlice::new(addr, 8).reader())?;
+            }
+            RUST_MISC_DEV_URING_CMD_GET_VALUE => {
+                me.get_value(UserSlice::new(addr, 8).writer())?;
+            }
+            _ => {
+                dev_err!(me.dev, "-> uring_cmd not recognised: {}\n", cmd);
+                return Err(ENOTTY);
+            }
+        }
+        Ok(0)
+    }
 }
 
 #[pinned_drop]
-- 
2.43.0

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Nikita Krasnov]

[-- Attachment #1.1: Type: text/plain, Size: 1034 bytes --]

On Sat, Jul 19, 2025 at 05:33:54PM +0300 Sidong Yang wrote:
> This patch series implemens an abstraction for io-uring sqe and cmd and
> adds uring_cmd callback for miscdevice. Also there is an example that use
> uring_cmd in rust-miscdevice sample.
> 
> Sidong Yang (4):
>   rust: bindings: add io_uring headers in bindings_helper.h
>   rust: io_uring: introduce rust abstraction for io-uring cmd
>   rust: miscdevice: add uring_cmd() for MiscDevice trait
>   samples: rust: rust_misc_device: add uring_cmd example
> 
>  rust/bindings/bindings_helper.h  |   2 +
>  rust/kernel/io_uring.rs          | 114 +++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs               |   1 +
>  rust/kernel/miscdevice.rs        |  34 +++++++++
>  samples/rust/rust_misc_device.rs |  30 ++++++++
>  5 files changed, 181 insertions(+)
>  create mode 100644 rust/kernel/io_uring.rs
> 

Is it just me or did the [PATCH 2/4] get lost? I only see 
[RFC PATCH 0/4], [PATCH 1/4], [PATCH 3/4] and [PATCH 4/4].

-- 
Nikita Krasnov

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Nikita Krasnov]

[-- Attachment #1.1: Type: text/plain, Size: 1169 bytes --]

On Sat, Jul 19, 2025 at 06:00:18PM +0300 Nikita Krasnov wrote:
> On Sat, Jul 19, 2025 at 05:33:54PM +0300 Sidong Yang wrote:
>> This patch series implemens an abstraction for io-uring sqe and cmd and
>> adds uring_cmd callback for miscdevice. Also there is an example that use
>> uring_cmd in rust-miscdevice sample.
>> 
>> Sidong Yang (4):
>>   rust: bindings: add io_uring headers in bindings_helper.h
>>   rust: io_uring: introduce rust abstraction for io-uring cmd
>>   rust: miscdevice: add uring_cmd() for MiscDevice trait
>>   samples: rust: rust_misc_device: add uring_cmd example
>> 
>>  rust/bindings/bindings_helper.h  |   2 +
>>  rust/kernel/io_uring.rs          | 114 +++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs               |   1 +
>>  rust/kernel/miscdevice.rs        |  34 +++++++++
>>  samples/rust/rust_misc_device.rs |  30 ++++++++
>>  5 files changed, 181 insertions(+)
>>  create mode 100644 rust/kernel/io_uring.rs
>> 
> 
> Is it just me or did the [PATCH 2/4] get lost? I only see 
> [RFC PATCH 0/4], [PATCH 1/4], [PATCH 3/4] and [PATCH 4/4].

Never mind, there it is. Sorry for noise.

-- 
Nikita Krasnov

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Miguel Ojeda]

On Sat, Jul 19, 2025 at 4:34 PM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> This patch series implemens an abstraction for io-uring sqe and cmd and
> adds uring_cmd callback for miscdevice. Also there is an example that use
> uring_cmd in rust-miscdevice sample.

Who will be using these?

Thanks!

Cheers,
Miguel

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Danilo Krummrich]

Hi Sidong,

On 7/19/25 4:33 PM, Sidong Yang wrote:
> This patch series implemens an abstraction for io-uring sqe and cmd and
> adds uring_cmd callback for miscdevice. Also there is an example that use
> uring_cmd in rust-miscdevice sample.

Please also add Greg, maybe get_maintainer.pl does not list him, since the
entries for miscdevice were missing for a while I think.

For new abstractions I also recommend to Cc the RUST reviewers.

Thanks,
Danilo

Re: [PATCH 1/4] rust: bindings: add io_uring headers in bindings_helper.h

[kernel test robot]

Hi Sidong,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rust/rust-next]
[also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.16-rc6 next-20250718]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sidong-Yang/rust-bindings-add-io_uring-headers-in-bindings_helper-h/20250719-223630
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/20250719143358.22363-2-sidong.yang%40furiosa.ai
patch subject: [PATCH 1/4] rust: bindings: add io_uring headers in bindings_helper.h
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250720/202507202006.8ZnBwBsW-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250720/202507202006.8ZnBwBsW-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507202006.8ZnBwBsW-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> warning: `extern` fn uses type `io_tw_state`, which is not FFI-safe
   --> rust/bindings/bindings_generated.rs:109725:28
   |
   109725 |     ::core::option::Option<unsafe extern "C" fn(req: *mut io_kiocb, tw: io_tw_token_t)>;
   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
   |
   = help: consider adding a member to this struct
   = note: this struct has no fields
   note: the type is defined here
   --> rust/bindings/bindings_generated.rs:109642:1
   |
   109642 | pub struct io_tw_state {}
   | ^^^^^^^^^^^^^^^^^^^^^^
   = note: `#[warn(improper_ctypes_definitions)]` on by default

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Sidong Yang]

On Sat, Jul 19, 2025 at 06:34:49PM +0200, Miguel Ojeda wrote:
> On Sat, Jul 19, 2025 at 4:34 PM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >
> > This patch series implemens an abstraction for io-uring sqe and cmd and
> > adds uring_cmd callback for miscdevice. Also there is an example that use
> > uring_cmd in rust-miscdevice sample.
> 
> Who will be using these?

Hi, Miguel

Although some existing kernel modules already use uring_cmd, they aren’t 
implemented in Rust. Currently, no Rust code leverages this abstraction, 
but it will enable anyone who wants to write kernel drivers in Rust using 
uring_cmd.

Thanks,
Sidong

> 
> Thanks!
> 
> Cheers,
> Miguel

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Sidong Yang]

On Sat, Jul 19, 2025 at 07:03:24PM +0200, Danilo Krummrich wrote:
> Hi Sidong,
> 
> On 7/19/25 4:33 PM, Sidong Yang wrote:
> > This patch series implemens an abstraction for io-uring sqe and cmd and
> > adds uring_cmd callback for miscdevice. Also there is an example that use
> > uring_cmd in rust-miscdevice sample.
> 
> Please also add Greg, maybe get_maintainer.pl does not list him, since the
> entries for miscdevice were missing for a while I think.
> 
> For new abstractions I also recommend to Cc the RUST reviewers.

Thanks for a advice. I've added Greg, and other RUST reviewers to Cc. 

Thanks,
Sidong

> 
> Thanks,
> Danilo

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Miguel Ojeda]

On Sun, Jul 20, 2025 at 6:07 PM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> Although some existing kernel modules already use uring_cmd, they aren’t
> implemented in Rust. Currently, no Rust code leverages this abstraction,
> but it will enable anyone who wants to write kernel drivers in Rust using
> uring_cmd.

Do you have a concrete user in mind?

i.e. I am asking because the kernel, in general, requires a user (in
mainline) for code to be merged. So maintainers normally don't merge
code unless it is clear who will use a feature upstream -- please see
the last bullet of:

    https://rust-for-linux.com/contributing#submitting-new-abstractions-and-modules

Thanks!

Cheers,
Miguel

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Sidong Yang]

On Sun, Jul 20, 2025 at 06:41:06PM +0200, Miguel Ojeda wrote:
> On Sun, Jul 20, 2025 at 6:07 PM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >
> > Although some existing kernel modules already use uring_cmd, they aren’t
> > implemented in Rust. Currently, no Rust code leverages this abstraction,
> > but it will enable anyone who wants to write kernel drivers in Rust using
> > uring_cmd.
> 
> Do you have a concrete user in mind?

Sadly, there isn’t a concrete user yet. I understand that an abstraction by itself
won’t be merged without a real in-tree user.
I’ll identify a suitable kernel module to port to Rust and follow up once I have one.

Thanks,
Sidong

> 
> i.e. I am asking because the kernel, in general, requires a user (in
> mainline) for code to be merged. So maintainers normally don't merge
> code unless it is clear who will use a feature upstream -- please see
> the last bullet of:
> 
>     https://rust-for-linux.com/contributing#submitting-new-abstractions-and-modules
> 
> Thanks!
> 
> Cheers,
> Miguel

Re: [RFC PATCH 0/4] rust: miscdevice: abstraction for uring-cmd

[Miguel Ojeda]

On Sun, Jul 20, 2025 at 6:52 PM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> Sadly, there isn’t a concrete user yet. I understand that an abstraction by itself
> won’t be merged without a real in-tree user.
> I’ll identify a suitable kernel module to port to Rust and follow up once I have one.

Sounds good, thanks!

(Just in case: maintainers may or may not want to have an equivalent
Rust module for a C one, it is up to them. In that case,
https://rust-for-linux.com/rust-reference-drivers may help.).

Cheers,
Miguel

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Caleb Sander Mateos]

On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> This patch introduces rust abstraction for io-uring sqe, cmd. IoUringSqe
> abstracts io_uring_sqe and it has cmd_data(). and IoUringCmd is
> abstraction for io_uring_cmd. From this, user can get cmd_op, flags,
> pdu and also sqe.
>
> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> ---
>  rust/kernel/io_uring.rs | 114 ++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs      |   1 +
>  2 files changed, 115 insertions(+)
>  create mode 100644 rust/kernel/io_uring.rs
>
> diff --git a/rust/kernel/io_uring.rs b/rust/kernel/io_uring.rs
> new file mode 100644
> index 000000000000..7843effbedb4
> --- /dev/null
> +++ b/rust/kernel/io_uring.rs
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2025 Furiosa AI.
> +
> +//! Files and file descriptors.
> +//!
> +//! C headers: [`include/linux/io_uring/cmd.h`](srctree/include/linux/io_uring/cmd.h) and
> +//! [`include/linux/file.h`](srctree/include/linux/file.h)
> +
> +use core::mem::MaybeUninit;
> +
> +use crate::{fs::File, types::Opaque};
> +
> +pub mod flags {
> +    pub const COMPLETE_DEFER: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPLETE_DEFER;
> +    pub const UNLOCKED: i32 = bindings::io_uring_cmd_flags_IO_URING_F_UNLOCKED;
> +
> +    pub const MULTISHOT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_MULTISHOT;
> +    pub const IOWQ: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOWQ;
> +    pub const NONBLOCK: i32 = bindings::io_uring_cmd_flags_IO_URING_F_NONBLOCK;
> +
> +    pub const SQE128: i32 = bindings::io_uring_cmd_flags_IO_URING_F_SQE128;
> +    pub const CQE32: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CQE32;
> +    pub const IOPOLL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOPOLL;
> +
> +    pub const CANCEL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CANCEL;
> +    pub const COMPAT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPAT;
> +    pub const TASK_DEAD: i32 = bindings::io_uring_cmd_flags_IO_URING_F_TASK_DEAD;
> +}
> +
> +#[repr(transparent)]
> +pub struct IoUringCmd {
> +    inner: Opaque<bindings::io_uring_cmd>,
> +}
> +
> +impl IoUringCmd {
> +    /// Returns the cmd_op with associated with the io_uring_cmd.
> +    #[inline]
> +    pub fn cmd_op(&self) -> u32 {
> +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> +        unsafe { (*self.inner.get()).cmd_op }
> +    }
> +
> +    /// Returns the flags with associated with the io_uring_cmd.
> +    #[inline]
> +    pub fn flags(&self) -> u32 {
> +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> +        unsafe { (*self.inner.get()).flags }
> +    }
> +
> +    /// Returns the ref pdu for free use.
> +    #[inline]
> +    pub fn pdu(&mut self) -> MaybeUninit<&mut [u8; 32]> {

Should be &mut MaybeUninit, right? It's the bytes that may be
uninitialized, not the reference.

> +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> +        unsafe { MaybeUninit::new(&mut (*self.inner.get()).pdu) }
> +    }
> +
> +    /// Constructs a new `struct io_uring_cmd` wrapper from a file descriptor.

Why "from a file descriptor"?

Also, missing a comment documenting the safety preconditions?

> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *const bindings::io_uring_cmd) -> &'a IoUringCmd {

Could take NonNull instead of a raw pointer.

> +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.

"File" -> "IoUringCmd"?

> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    // Returns the file that referenced by uring cmd self.

I had a hard time parsing this comment. How about "Returns a reference
to the uring cmd's file object"?

> +    #[inline]
> +    pub fn file<'a>(&'a self) -> &'a File {

Could elide the lifetime.

> +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> +        let file = unsafe { (*self.inner.get()).file };
> +        unsafe { File::from_raw_file(file) }

Missing a SAFETY comment for File::from_raw_file()? I would expect
something about io_uring_cmd's file field storing a non-null pointer
to a struct file on which a reference is held for the duration of the
uring cmd.

> +    }
> +
> +    // Returns the sqe  that referenced by uring cmd self.

"Returns a reference to the uring cmd's SQE"?

> +    #[inline]
> +    pub fn sqe(&self) -> &IoUringSqe {
> +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> +        let ptr = unsafe { (*self.inner.get()).sqe };

"ptr" isn't very descriptive. How about "sqe"?

> +        unsafe { IoUringSqe::from_raw(ptr) }

Similar, missing SAFETY comment for IoUringSqe::from_raw()?

> +    }
> +
> +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
> +    #[inline]
> +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {

I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
definitely need to be pinned in memory. For example,
io_req_normal_work_add() inserts the struct io_kiocb into a linked
list. Probably some sort of pinning is necessary for IoUringCmd.

> +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> +        unsafe {
> +            bindings::io_uring_cmd_done(self.inner.get(), ret, res2, issue_flags);
> +        }
> +    }
> +}
> +
> +#[repr(transparent)]
> +pub struct IoUringSqe {
> +    inner: Opaque<bindings::io_uring_sqe>,
> +}
> +
> +impl<'a> IoUringSqe {
> +    pub fn cmd_data(&'a self) -> &'a [Opaque<u8>] {
> +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> +        unsafe {
> +            let cmd = (*self.inner.get()).__bindgen_anon_6.cmd.as_ref();
> +            core::slice::from_raw_parts(cmd.as_ptr() as *const Opaque<u8>, 8)

Why 8? Should be 16 bytes for a 64-byte SQE and 80 bytes for a
128-byte SQE, right?

> +        }
> +    }
> +
> +    #[inline]
> +    pub unsafe fn from_raw(ptr: *const bindings::io_uring_sqe) -> &'a IoUringSqe {

Take NonNull here too?

> +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
> +        //
> +        // INVARIANT: The caller guarantees that there are no problematic `fdget_pos` calls.

Why "File" and "fdget_pos"?

Best,
Caleb

> +        unsafe { &*ptr.cast() }
> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 6b4774b2b1c3..fb310e78d51d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -80,6 +80,7 @@
>  pub mod fs;
>  pub mod init;
>  pub mod io;
> +pub mod io_uring;
>  pub mod ioctl;
>  pub mod jump_label;
>  #[cfg(CONFIG_KUNIT)]
> --
> 2.43.0
>
>

Re: [PATCH 3/4] rust: miscdevice: add uring_cmd() for MiscDevice trait

[kernel test robot]

Hi Sidong,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rust/rust-next]
[also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.16-rc6 next-20250718]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sidong-Yang/rust-bindings-add-io_uring-headers-in-bindings_helper-h/20250719-223630
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/20250719143358.22363-4-sidong.yang%40furiosa.ai
patch subject: [PATCH 3/4] rust: miscdevice: add uring_cmd() for MiscDevice trait
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250721/202507210306.zCOB3QtO-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250721/202507210306.zCOB3QtO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507210306.zCOB3QtO-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> warning: unused variable: `issue_flags`
   --> rust/kernel/miscdevice.rs:184:9
   |
   184 |         issue_flags: u32,
   |         ^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_issue_flags`
   |
   = note: `#[warn(unused_variables)]` on by default

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/4] rust: miscdevice: add uring_cmd() for MiscDevice trait

[Caleb Sander Mateos]

On Sat, Jul 19, 2025 at 10:35 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> This patch adds uring_cmd() function for MiscDevice trait and its
> callback implementation. It uses IoUringCmd that io_uring_cmd rust
> abstraction.
>
> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> ---
>  rust/kernel/miscdevice.rs | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index 288f40e79906..5255faf27934 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -14,6 +14,7 @@
>      error::{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,
> @@ -175,6 +176,15 @@ fn show_fdinfo(
>      ) {
>          build_error!(VTABLE_DEFAULT_ERROR)
>      }
> +
> +    fn uring_cmd(
> +        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
> +        _file: &File,
> +        _io_uring_cmd: &IoUringCmd,
> +        issue_flags: u32,
> +    ) -> Result<isize> {

Would i32 make more sense as the return value, since that's what
io_uring_cqe actually stores?

> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
>  }
>
>  /// A vtable for the file operations of a Rust miscdevice.
> @@ -332,6 +342,25 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
>          T::show_fdinfo(device, m, file);
>      }
>
> +    unsafe extern "C" fn uring_cmd(
> +        ioucmd: *mut bindings::io_uring_cmd,
> +        issue_flags: ffi::c_uint,
> +    ) -> ffi::c_int {
> +        // SAFETY: The file is valid for the duration of this call.
> +        let ioucmd = unsafe { IoUringCmd::from_raw(ioucmd) };
> +        let file = ioucmd.file();
> +
> +        // SAFETY: The file is valid for the duration of this call.
> +        let private = unsafe { (*file.as_ptr()).private_data }.cast();
> +        // SAFETY: Ioctl calls can borrow the private data of the file.

"Ioctl" -> "uring cmd"?

Best,
Caleb

> +        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> +
> +        match T::uring_cmd(device, file, ioucmd, issue_flags) {
> +            Ok(ret) => ret as ffi::c_int,
> +            Err(err) => err.to_errno() as ffi::c_int,
> +        }
> +    }
> +
>      const VTABLE: bindings::file_operations = bindings::file_operations {
>          open: Some(Self::open),
>          release: Some(Self::release),
> @@ -354,6 +383,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
>
>

Re: [PATCH 4/4] samples: rust: rust_misc_device: add uring_cmd example

[Caleb Sander Mateos]

On Sat, Jul 19, 2025 at 10:35 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> This patch makes rust_misc_device handle uring_cmd. Command ops are like
> ioctl that set or get values in simple way.
>
> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> ---
>  samples/rust/rust_misc_device.rs | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> index c881fd6dbd08..cd0e578231d2 100644
> --- a/samples/rust/rust_misc_device.rs
> +++ b/samples/rust/rust_misc_device.rs
> @@ -101,6 +101,7 @@
>      c_str,
>      device::Device,
>      fs::File,
> +    io_uring::IoUringCmd,
>      ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
>      miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
>      new_mutex,
> @@ -114,6 +115,9 @@
>  const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('|' as u32, 0x81);
>  const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('|' as u32, 0x82);
>
> +const RUST_MISC_DEV_URING_CMD_SET_VALUE: u32 = 0x83;
> +const RUST_MISC_DEV_URING_CMD_GET_VALUE: u32 = 0x84;

In real uring_cmd() implementations, the cmd_op values are assigned
using the _IO* macros, same as for ioctls. But I suppose that's not
strictly required for the sample driver.

> +
>  module! {
>      type: RustMiscDeviceModule,
>      name: "rust_misc_device",
> @@ -190,6 +194,32 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
>
>          Ok(0)
>      }
> +
> +    fn uring_cmd(
> +        me: Pin<&RustMiscDevice>,
> +        _file: &File,
> +        io_uring_cmd: &IoUringCmd,
> +        _issue_flags: u32,
> +    ) -> Result<isize> {
> +        dev_info!(me.dev, "UringCmd Rust Misc Device Sample\n");
> +        let cmd = io_uring_cmd.cmd_op();
> +        let cmd_data = io_uring_cmd.sqe().cmd_data().as_ptr() as *const usize;
> +        let addr = unsafe { *cmd_data };

The io_uring_sqe is user-mapped memory, so this load needs to be
atomic. In C, the uring_cmd() implementation would use READ_ONCE().
Sounds like Rust code is currently using read_volatile() (with a FIXME
comment to switch to read_once() once that's available).

Best,
Caleb

> +
> +        match cmd {
> +            RUST_MISC_DEV_URING_CMD_SET_VALUE => {
> +                me.set_value(UserSlice::new(addr, 8).reader())?;
> +            }
> +            RUST_MISC_DEV_URING_CMD_GET_VALUE => {
> +                me.get_value(UserSlice::new(addr, 8).writer())?;
> +            }
> +            _ => {
> +                dev_err!(me.dev, "-> uring_cmd not recognised: {}\n", cmd);
> +                return Err(ENOTTY);
> +            }
> +        }
> +        Ok(0)
> +    }
>  }
>
>  #[pinned_drop]
> --
> 2.43.0
>
>

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Sidong Yang]

On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
> On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >
> > This patch introduces rust abstraction for io-uring sqe, cmd. IoUringSqe
> > abstracts io_uring_sqe and it has cmd_data(). and IoUringCmd is
> > abstraction for io_uring_cmd. From this, user can get cmd_op, flags,
> > pdu and also sqe.
> >
> > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > ---
> >  rust/kernel/io_uring.rs | 114 ++++++++++++++++++++++++++++++++++++++++
> >  rust/kernel/lib.rs      |   1 +
> >  2 files changed, 115 insertions(+)
> >  create mode 100644 rust/kernel/io_uring.rs
> >
> > diff --git a/rust/kernel/io_uring.rs b/rust/kernel/io_uring.rs
> > new file mode 100644
> > index 000000000000..7843effbedb4
> > --- /dev/null
> > +++ b/rust/kernel/io_uring.rs
> > @@ -0,0 +1,114 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2025 Furiosa AI.
> > +
> > +//! Files and file descriptors.
> > +//!
> > +//! C headers: [`include/linux/io_uring/cmd.h`](srctree/include/linux/io_uring/cmd.h) and
> > +//! [`include/linux/file.h`](srctree/include/linux/file.h)
> > +
> > +use core::mem::MaybeUninit;
> > +
> > +use crate::{fs::File, types::Opaque};
> > +
> > +pub mod flags {
> > +    pub const COMPLETE_DEFER: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPLETE_DEFER;
> > +    pub const UNLOCKED: i32 = bindings::io_uring_cmd_flags_IO_URING_F_UNLOCKED;
> > +
> > +    pub const MULTISHOT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_MULTISHOT;
> > +    pub const IOWQ: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOWQ;
> > +    pub const NONBLOCK: i32 = bindings::io_uring_cmd_flags_IO_URING_F_NONBLOCK;
> > +
> > +    pub const SQE128: i32 = bindings::io_uring_cmd_flags_IO_URING_F_SQE128;
> > +    pub const CQE32: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CQE32;
> > +    pub const IOPOLL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOPOLL;
> > +
> > +    pub const CANCEL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CANCEL;
> > +    pub const COMPAT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPAT;
> > +    pub const TASK_DEAD: i32 = bindings::io_uring_cmd_flags_IO_URING_F_TASK_DEAD;
> > +}
> > +
> > +#[repr(transparent)]
> > +pub struct IoUringCmd {
> > +    inner: Opaque<bindings::io_uring_cmd>,
> > +}
> > +
> > +impl IoUringCmd {
> > +    /// Returns the cmd_op with associated with the io_uring_cmd.
> > +    #[inline]
> > +    pub fn cmd_op(&self) -> u32 {
> > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > +        unsafe { (*self.inner.get()).cmd_op }
> > +    }
> > +
> > +    /// Returns the flags with associated with the io_uring_cmd.
> > +    #[inline]
> > +    pub fn flags(&self) -> u32 {
> > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > +        unsafe { (*self.inner.get()).flags }
> > +    }
> > +
> > +    /// Returns the ref pdu for free use.
> > +    #[inline]
> > +    pub fn pdu(&mut self) -> MaybeUninit<&mut [u8; 32]> {
> 
> Should be &mut MaybeUninit, right? It's the bytes that may be
> uninitialized, not the reference.

Yes, it should be &mut MaybeUninit.
> 
> > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > +        unsafe { MaybeUninit::new(&mut (*self.inner.get()).pdu) }
> > +    }
> > +
> > +    /// Constructs a new `struct io_uring_cmd` wrapper from a file descriptor.
> 
> Why "from a file descriptor"?
> 
> Also, missing a comment documenting the safety preconditions?

Yes, it's a mistake in comment and also ptr should be NonNull.
> 
> > +    #[inline]
> > +    pub unsafe fn from_raw<'a>(ptr: *const bindings::io_uring_cmd) -> &'a IoUringCmd {
> 
> Could take NonNull instead of a raw pointer.
> 
> > +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> > +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
> 
> "File" -> "IoUringCmd"?
> 
> > +        unsafe { &*ptr.cast() }
> > +    }
> > +
> > +    // Returns the file that referenced by uring cmd self.
> 
> I had a hard time parsing this comment. How about "Returns a reference
> to the uring cmd's file object"?

Agreed. thanks.
> 
> > +    #[inline]
> > +    pub fn file<'a>(&'a self) -> &'a File {
> 
> Could elide the lifetime.

Thanks, I didn't know about this.
> 
> > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > +        let file = unsafe { (*self.inner.get()).file };
> > +        unsafe { File::from_raw_file(file) }
> 
> Missing a SAFETY comment for File::from_raw_file()? I would expect
> something about io_uring_cmd's file field storing a non-null pointer
> to a struct file on which a reference is held for the duration of the
> uring cmd.

Yes, I missed. thanks.
> 
> > +    }
> > +
> > +    // Returns the sqe  that referenced by uring cmd self.
> 
> "Returns a reference to the uring cmd's SQE"?

Agreed, thanks.
> 
> > +    #[inline]
> > +    pub fn sqe(&self) -> &IoUringSqe {
> > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > +        let ptr = unsafe { (*self.inner.get()).sqe };
> 
> "ptr" isn't very descriptive. How about "sqe"?

Sounds good.
> 
> > +        unsafe { IoUringSqe::from_raw(ptr) }
> 
> Similar, missing SAFETY comment for IoUringSqe::from_raw()?

Thanks. I missed.
> 
> > +    }
> > +
> > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
> > +    #[inline]
> > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
> 
> I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
> example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
> into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
> definitely need to be pinned in memory. For example,
> io_req_normal_work_add() inserts the struct io_kiocb into a linked
> list. Probably some sort of pinning is necessary for IoUringCmd.

Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
&mut IoUringCmd for arg.

> 
> > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > +        unsafe {
> > +            bindings::io_uring_cmd_done(self.inner.get(), ret, res2, issue_flags);
> > +        }
> > +    }
> > +}
> > +
> > +#[repr(transparent)]
> > +pub struct IoUringSqe {
> > +    inner: Opaque<bindings::io_uring_sqe>,
> > +}
> > +
> > +impl<'a> IoUringSqe {
> > +    pub fn cmd_data(&'a self) -> &'a [Opaque<u8>] {
> > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > +        unsafe {
> > +            let cmd = (*self.inner.get()).__bindgen_anon_6.cmd.as_ref();
> > +            core::slice::from_raw_parts(cmd.as_ptr() as *const Opaque<u8>, 8)
> 
> Why 8? Should be 16 bytes for a 64-byte SQE and 80 bytes for a
> 128-byte SQE, right?

Yes, it should be 16 bytes or 80 bytes. I'll fix this.

> 
> > +        }
> > +    }
> > +
> > +    #[inline]
> > +    pub unsafe fn from_raw(ptr: *const bindings::io_uring_sqe) -> &'a IoUringSqe {
> 
> Take NonNull here too?

Yes, Thanks.
> 
> > +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> > +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
> > +        //
> > +        // INVARIANT: The caller guarantees that there are no problematic `fdget_pos` calls.
> 
> Why "File" and "fdget_pos"?

It's a bad mistake. thanks!

Thank you so much for deatiled review!

Thanks,
Sidong
> 
> Best,
> Caleb
> 
> > +        unsafe { &*ptr.cast() }
> > +    }
> > +}
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 6b4774b2b1c3..fb310e78d51d 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -80,6 +80,7 @@
> >  pub mod fs;
> >  pub mod init;
> >  pub mod io;
> > +pub mod io_uring;
> >  pub mod ioctl;
> >  pub mod jump_label;
> >  #[cfg(CONFIG_KUNIT)]
> > --
> > 2.43.0
> >
> >

Re: [PATCH 3/4] rust: miscdevice: add uring_cmd() for MiscDevice trait

[Sidong Yang]

On Sun, Jul 20, 2025 at 04:08:19PM -0400, Caleb Sander Mateos wrote:
> On Sat, Jul 19, 2025 at 10:35 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >
> > This patch adds uring_cmd() function for MiscDevice trait and its
> > callback implementation. It uses IoUringCmd that io_uring_cmd rust
> > abstraction.
> >
> > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > ---
> >  rust/kernel/miscdevice.rs | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> > index 288f40e79906..5255faf27934 100644
> > --- a/rust/kernel/miscdevice.rs
> > +++ b/rust/kernel/miscdevice.rs
> > @@ -14,6 +14,7 @@
> >      error::{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,
> > @@ -175,6 +176,15 @@ fn show_fdinfo(
> >      ) {
> >          build_error!(VTABLE_DEFAULT_ERROR)
> >      }
> > +
> > +    fn uring_cmd(
> > +        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
> > +        _file: &File,
> > +        _io_uring_cmd: &IoUringCmd,
> > +        issue_flags: u32,
> > +    ) -> Result<isize> {
> 
> Would i32 make more sense as the return value, since that's what
> io_uring_cqe actually stores?

Agreed. thanks.
> 
> > +        build_error!(VTABLE_DEFAULT_ERROR)
> > +    }
> >  }
> >
> >  /// A vtable for the file operations of a Rust miscdevice.
> > @@ -332,6 +342,25 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
> >          T::show_fdinfo(device, m, file);
> >      }
> >
> > +    unsafe extern "C" fn uring_cmd(
> > +        ioucmd: *mut bindings::io_uring_cmd,
> > +        issue_flags: ffi::c_uint,
> > +    ) -> ffi::c_int {
> > +        // SAFETY: The file is valid for the duration of this call.
> > +        let ioucmd = unsafe { IoUringCmd::from_raw(ioucmd) };
> > +        let file = ioucmd.file();
> > +
> > +        // SAFETY: The file is valid for the duration of this call.
> > +        let private = unsafe { (*file.as_ptr()).private_data }.cast();
> > +        // SAFETY: Ioctl calls can borrow the private data of the file.
> 
> "Ioctl" -> "uring cmd"?

Thanks it's a bad mistake.

Thanks,
Sidong
> 
> Best,
> Caleb
> 
> > +        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> > +
> > +        match T::uring_cmd(device, file, ioucmd, issue_flags) {
> > +            Ok(ret) => ret as ffi::c_int,
> > +            Err(err) => err.to_errno() as ffi::c_int,
> > +        }
> > +    }
> > +
> >      const VTABLE: bindings::file_operations = bindings::file_operations {
> >          open: Some(Self::open),
> >          release: Some(Self::release),
> > @@ -354,6 +383,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
> >
> >

Re: [PATCH 4/4] samples: rust: rust_misc_device: add uring_cmd example

[Sidong Yang]

On Sun, Jul 20, 2025 at 04:21:00PM -0400, Caleb Sander Mateos wrote:
> On Sat, Jul 19, 2025 at 10:35 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >
> > This patch makes rust_misc_device handle uring_cmd. Command ops are like
> > ioctl that set or get values in simple way.
> >
> > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > ---
> >  samples/rust/rust_misc_device.rs | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> > index c881fd6dbd08..cd0e578231d2 100644
> > --- a/samples/rust/rust_misc_device.rs
> > +++ b/samples/rust/rust_misc_device.rs
> > @@ -101,6 +101,7 @@
> >      c_str,
> >      device::Device,
> >      fs::File,
> > +    io_uring::IoUringCmd,
> >      ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
> >      miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
> >      new_mutex,
> > @@ -114,6 +115,9 @@
> >  const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('|' as u32, 0x81);
> >  const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('|' as u32, 0x82);
> >
> > +const RUST_MISC_DEV_URING_CMD_SET_VALUE: u32 = 0x83;
> > +const RUST_MISC_DEV_URING_CMD_GET_VALUE: u32 = 0x84;
> 
> In real uring_cmd() implementations, the cmd_op values are assigned
> using the _IO* macros, same as for ioctls. But I suppose that's not
> strictly required for the sample driver.

Okay, I'll use same _IO* pattern with ioctl.
> 
> > +
> >  module! {
> >      type: RustMiscDeviceModule,
> >      name: "rust_misc_device",
> > @@ -190,6 +194,32 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
> >
> >          Ok(0)
> >      }
> > +
> > +    fn uring_cmd(
> > +        me: Pin<&RustMiscDevice>,
> > +        _file: &File,
> > +        io_uring_cmd: &IoUringCmd,
> > +        _issue_flags: u32,
> > +    ) -> Result<isize> {
> > +        dev_info!(me.dev, "UringCmd Rust Misc Device Sample\n");
> > +        let cmd = io_uring_cmd.cmd_op();
> > +        let cmd_data = io_uring_cmd.sqe().cmd_data().as_ptr() as *const usize;
> > +        let addr = unsafe { *cmd_data };
> 
> The io_uring_sqe is user-mapped memory, so this load needs to be
> atomic. In C, the uring_cmd() implementation would use READ_ONCE().
> Sounds like Rust code is currently using read_volatile() (with a FIXME
> comment to switch to read_once() once that's available).

Yes, I've missed this. read_volatile with comment would be good.

Thanks,
Sidong
> 
> Best,
> Caleb
> 
> > +
> > +        match cmd {
> > +            RUST_MISC_DEV_URING_CMD_SET_VALUE => {
> > +                me.set_value(UserSlice::new(addr, 8).reader())?;
> > +            }
> > +            RUST_MISC_DEV_URING_CMD_GET_VALUE => {
> > +                me.get_value(UserSlice::new(addr, 8).writer())?;
> > +            }
> > +            _ => {
> > +                dev_err!(me.dev, "-> uring_cmd not recognised: {}\n", cmd);
> > +                return Err(ENOTTY);
> > +            }
> > +        }
> > +        Ok(0)
> > +    }
> >  }
> >
> >  #[pinned_drop]
> > --
> > 2.43.0
> >
> >

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Caleb Sander Mateos]

On Mon, Jul 21, 2025 at 1:23 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>
> On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
> > On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> > >
> > > This patch introduces rust abstraction for io-uring sqe, cmd. IoUringSqe
> > > abstracts io_uring_sqe and it has cmd_data(). and IoUringCmd is
> > > abstraction for io_uring_cmd. From this, user can get cmd_op, flags,
> > > pdu and also sqe.
> > >
> > > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > > ---
> > >  rust/kernel/io_uring.rs | 114 ++++++++++++++++++++++++++++++++++++++++
> > >  rust/kernel/lib.rs      |   1 +
> > >  2 files changed, 115 insertions(+)
> > >  create mode 100644 rust/kernel/io_uring.rs
> > >
> > > diff --git a/rust/kernel/io_uring.rs b/rust/kernel/io_uring.rs
> > > new file mode 100644
> > > index 000000000000..7843effbedb4
> > > --- /dev/null
> > > +++ b/rust/kernel/io_uring.rs
> > > @@ -0,0 +1,114 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +// Copyright (C) 2025 Furiosa AI.
> > > +
> > > +//! Files and file descriptors.
> > > +//!
> > > +//! C headers: [`include/linux/io_uring/cmd.h`](srctree/include/linux/io_uring/cmd.h) and
> > > +//! [`include/linux/file.h`](srctree/include/linux/file.h)
> > > +
> > > +use core::mem::MaybeUninit;
> > > +
> > > +use crate::{fs::File, types::Opaque};
> > > +
> > > +pub mod flags {
> > > +    pub const COMPLETE_DEFER: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPLETE_DEFER;
> > > +    pub const UNLOCKED: i32 = bindings::io_uring_cmd_flags_IO_URING_F_UNLOCKED;
> > > +
> > > +    pub const MULTISHOT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_MULTISHOT;
> > > +    pub const IOWQ: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOWQ;
> > > +    pub const NONBLOCK: i32 = bindings::io_uring_cmd_flags_IO_URING_F_NONBLOCK;
> > > +
> > > +    pub const SQE128: i32 = bindings::io_uring_cmd_flags_IO_URING_F_SQE128;
> > > +    pub const CQE32: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CQE32;
> > > +    pub const IOPOLL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOPOLL;
> > > +
> > > +    pub const CANCEL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CANCEL;
> > > +    pub const COMPAT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPAT;
> > > +    pub const TASK_DEAD: i32 = bindings::io_uring_cmd_flags_IO_URING_F_TASK_DEAD;
> > > +}
> > > +
> > > +#[repr(transparent)]
> > > +pub struct IoUringCmd {
> > > +    inner: Opaque<bindings::io_uring_cmd>,
> > > +}
> > > +
> > > +impl IoUringCmd {
> > > +    /// Returns the cmd_op with associated with the io_uring_cmd.
> > > +    #[inline]
> > > +    pub fn cmd_op(&self) -> u32 {
> > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > +        unsafe { (*self.inner.get()).cmd_op }
> > > +    }
> > > +
> > > +    /// Returns the flags with associated with the io_uring_cmd.
> > > +    #[inline]
> > > +    pub fn flags(&self) -> u32 {
> > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > +        unsafe { (*self.inner.get()).flags }
> > > +    }
> > > +
> > > +    /// Returns the ref pdu for free use.
> > > +    #[inline]
> > > +    pub fn pdu(&mut self) -> MaybeUninit<&mut [u8; 32]> {
> >
> > Should be &mut MaybeUninit, right? It's the bytes that may be
> > uninitialized, not the reference.
>
> Yes, it should be &mut MaybeUninit.
> >
> > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > +        unsafe { MaybeUninit::new(&mut (*self.inner.get()).pdu) }
> > > +    }
> > > +
> > > +    /// Constructs a new `struct io_uring_cmd` wrapper from a file descriptor.
> >
> > Why "from a file descriptor"?
> >
> > Also, missing a comment documenting the safety preconditions?
>
> Yes, it's a mistake in comment and also ptr should be NonNull.
> >
> > > +    #[inline]
> > > +    pub unsafe fn from_raw<'a>(ptr: *const bindings::io_uring_cmd) -> &'a IoUringCmd {
> >
> > Could take NonNull instead of a raw pointer.
> >
> > > +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> > > +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
> >
> > "File" -> "IoUringCmd"?
> >
> > > +        unsafe { &*ptr.cast() }
> > > +    }
> > > +
> > > +    // Returns the file that referenced by uring cmd self.
> >
> > I had a hard time parsing this comment. How about "Returns a reference
> > to the uring cmd's file object"?
>
> Agreed. thanks.
> >
> > > +    #[inline]
> > > +    pub fn file<'a>(&'a self) -> &'a File {
> >
> > Could elide the lifetime.
>
> Thanks, I didn't know about this.
> >
> > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > +        let file = unsafe { (*self.inner.get()).file };
> > > +        unsafe { File::from_raw_file(file) }
> >
> > Missing a SAFETY comment for File::from_raw_file()? I would expect
> > something about io_uring_cmd's file field storing a non-null pointer
> > to a struct file on which a reference is held for the duration of the
> > uring cmd.
>
> Yes, I missed. thanks.
> >
> > > +    }
> > > +
> > > +    // Returns the sqe  that referenced by uring cmd self.
> >
> > "Returns a reference to the uring cmd's SQE"?
>
> Agreed, thanks.
> >
> > > +    #[inline]
> > > +    pub fn sqe(&self) -> &IoUringSqe {
> > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > +        let ptr = unsafe { (*self.inner.get()).sqe };
> >
> > "ptr" isn't very descriptive. How about "sqe"?
>
> Sounds good.
> >
> > > +        unsafe { IoUringSqe::from_raw(ptr) }
> >
> > Similar, missing SAFETY comment for IoUringSqe::from_raw()?
>
> Thanks. I missed.
> >
> > > +    }
> > > +
> > > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
> > > +    #[inline]
> > > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
> >
> > I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
> > example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
> > into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
> > definitely need to be pinned in memory. For example,
> > io_req_normal_work_add() inserts the struct io_kiocb into a linked
> > list. Probably some sort of pinning is necessary for IoUringCmd.
>
> Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
> in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
> &mut IoUringCmd for arg.

I'm still a little worried about exposing &mut IoUringCmd without
pinning. It would allow swapping the fields of two IoUringCmd's (and
therefore struct io_uring_cmd's), for example. If a struct
io_uring_cmd belongs to a struct io_kiocb linked into task_list,
swapping it with another struct io_uring_cmd would result in
io_uring_cmd_work() being invoked on the wrong struct io_uring_cmd.
Maybe it would be okay if IoUringCmd had an invariant that the struct
io_uring_cmd is not on the task work list. But I would feel safer with
using Pin<&mut IoUringCmd>. I don't have much experience with Rust in
the kernel, though, so I would welcome other opinions.

Best,
Caleb

>
> >
> > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > +        unsafe {
> > > +            bindings::io_uring_cmd_done(self.inner.get(), ret, res2, issue_flags);
> > > +        }
> > > +    }
> > > +}
> > > +
> > > +#[repr(transparent)]
> > > +pub struct IoUringSqe {
> > > +    inner: Opaque<bindings::io_uring_sqe>,
> > > +}
> > > +
> > > +impl<'a> IoUringSqe {
> > > +    pub fn cmd_data(&'a self) -> &'a [Opaque<u8>] {
> > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > +        unsafe {
> > > +            let cmd = (*self.inner.get()).__bindgen_anon_6.cmd.as_ref();
> > > +            core::slice::from_raw_parts(cmd.as_ptr() as *const Opaque<u8>, 8)
> >
> > Why 8? Should be 16 bytes for a 64-byte SQE and 80 bytes for a
> > 128-byte SQE, right?
>
> Yes, it should be 16 bytes or 80 bytes. I'll fix this.
>
> >
> > > +        }
> > > +    }
> > > +
> > > +    #[inline]
> > > +    pub unsafe fn from_raw(ptr: *const bindings::io_uring_sqe) -> &'a IoUringSqe {
> >
> > Take NonNull here too?
>
> Yes, Thanks.
> >
> > > +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> > > +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
> > > +        //
> > > +        // INVARIANT: The caller guarantees that there are no problematic `fdget_pos` calls.
> >
> > Why "File" and "fdget_pos"?
>
> It's a bad mistake. thanks!
>
> Thank you so much for deatiled review!
>
> Thanks,
> Sidong
> >
> > Best,
> > Caleb
> >
> > > +        unsafe { &*ptr.cast() }
> > > +    }
> > > +}
> > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > > index 6b4774b2b1c3..fb310e78d51d 100644
> > > --- a/rust/kernel/lib.rs
> > > +++ b/rust/kernel/lib.rs
> > > @@ -80,6 +80,7 @@
> > >  pub mod fs;
> > >  pub mod init;
> > >  pub mod io;
> > > +pub mod io_uring;
> > >  pub mod ioctl;
> > >  pub mod jump_label;
> > >  #[cfg(CONFIG_KUNIT)]
> > > --
> > > 2.43.0
> > >
> > >

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Sidong Yang]

On Mon, Jul 21, 2025 at 11:04:31AM -0400, Caleb Sander Mateos wrote:
> On Mon, Jul 21, 2025 at 1:23 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >
> > On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
> > > On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> > > >
> > > > This patch introduces rust abstraction for io-uring sqe, cmd. IoUringSqe
> > > > abstracts io_uring_sqe and it has cmd_data(). and IoUringCmd is
> > > > abstraction for io_uring_cmd. From this, user can get cmd_op, flags,
> > > > pdu and also sqe.
> > > >
> > > > Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
> > > > ---
> > > >  rust/kernel/io_uring.rs | 114 ++++++++++++++++++++++++++++++++++++++++
> > > >  rust/kernel/lib.rs      |   1 +
> > > >  2 files changed, 115 insertions(+)
> > > >  create mode 100644 rust/kernel/io_uring.rs
> > > >
> > > > diff --git a/rust/kernel/io_uring.rs b/rust/kernel/io_uring.rs
> > > > new file mode 100644
> > > > index 000000000000..7843effbedb4
> > > > --- /dev/null
> > > > +++ b/rust/kernel/io_uring.rs
> > > > @@ -0,0 +1,114 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +// Copyright (C) 2025 Furiosa AI.
> > > > +
> > > > +//! Files and file descriptors.
> > > > +//!
> > > > +//! C headers: [`include/linux/io_uring/cmd.h`](srctree/include/linux/io_uring/cmd.h) and
> > > > +//! [`include/linux/file.h`](srctree/include/linux/file.h)
> > > > +
> > > > +use core::mem::MaybeUninit;
> > > > +
> > > > +use crate::{fs::File, types::Opaque};
> > > > +
> > > > +pub mod flags {
> > > > +    pub const COMPLETE_DEFER: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPLETE_DEFER;
> > > > +    pub const UNLOCKED: i32 = bindings::io_uring_cmd_flags_IO_URING_F_UNLOCKED;
> > > > +
> > > > +    pub const MULTISHOT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_MULTISHOT;
> > > > +    pub const IOWQ: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOWQ;
> > > > +    pub const NONBLOCK: i32 = bindings::io_uring_cmd_flags_IO_URING_F_NONBLOCK;
> > > > +
> > > > +    pub const SQE128: i32 = bindings::io_uring_cmd_flags_IO_URING_F_SQE128;
> > > > +    pub const CQE32: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CQE32;
> > > > +    pub const IOPOLL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_IOPOLL;
> > > > +
> > > > +    pub const CANCEL: i32 = bindings::io_uring_cmd_flags_IO_URING_F_CANCEL;
> > > > +    pub const COMPAT: i32 = bindings::io_uring_cmd_flags_IO_URING_F_COMPAT;
> > > > +    pub const TASK_DEAD: i32 = bindings::io_uring_cmd_flags_IO_URING_F_TASK_DEAD;
> > > > +}
> > > > +
> > > > +#[repr(transparent)]
> > > > +pub struct IoUringCmd {
> > > > +    inner: Opaque<bindings::io_uring_cmd>,
> > > > +}
> > > > +
> > > > +impl IoUringCmd {
> > > > +    /// Returns the cmd_op with associated with the io_uring_cmd.
> > > > +    #[inline]
> > > > +    pub fn cmd_op(&self) -> u32 {
> > > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > > +        unsafe { (*self.inner.get()).cmd_op }
> > > > +    }
> > > > +
> > > > +    /// Returns the flags with associated with the io_uring_cmd.
> > > > +    #[inline]
> > > > +    pub fn flags(&self) -> u32 {
> > > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > > +        unsafe { (*self.inner.get()).flags }
> > > > +    }
> > > > +
> > > > +    /// Returns the ref pdu for free use.
> > > > +    #[inline]
> > > > +    pub fn pdu(&mut self) -> MaybeUninit<&mut [u8; 32]> {
> > >
> > > Should be &mut MaybeUninit, right? It's the bytes that may be
> > > uninitialized, not the reference.
> >
> > Yes, it should be &mut MaybeUninit.
> > >
> > > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > > +        unsafe { MaybeUninit::new(&mut (*self.inner.get()).pdu) }
> > > > +    }
> > > > +
> > > > +    /// Constructs a new `struct io_uring_cmd` wrapper from a file descriptor.
> > >
> > > Why "from a file descriptor"?
> > >
> > > Also, missing a comment documenting the safety preconditions?
> >
> > Yes, it's a mistake in comment and also ptr should be NonNull.
> > >
> > > > +    #[inline]
> > > > +    pub unsafe fn from_raw<'a>(ptr: *const bindings::io_uring_cmd) -> &'a IoUringCmd {
> > >
> > > Could take NonNull instead of a raw pointer.
> > >
> > > > +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> > > > +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
> > >
> > > "File" -> "IoUringCmd"?
> > >
> > > > +        unsafe { &*ptr.cast() }
> > > > +    }
> > > > +
> > > > +    // Returns the file that referenced by uring cmd self.
> > >
> > > I had a hard time parsing this comment. How about "Returns a reference
> > > to the uring cmd's file object"?
> >
> > Agreed. thanks.
> > >
> > > > +    #[inline]
> > > > +    pub fn file<'a>(&'a self) -> &'a File {
> > >
> > > Could elide the lifetime.
> >
> > Thanks, I didn't know about this.
> > >
> > > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > > +        let file = unsafe { (*self.inner.get()).file };
> > > > +        unsafe { File::from_raw_file(file) }
> > >
> > > Missing a SAFETY comment for File::from_raw_file()? I would expect
> > > something about io_uring_cmd's file field storing a non-null pointer
> > > to a struct file on which a reference is held for the duration of the
> > > uring cmd.
> >
> > Yes, I missed. thanks.
> > >
> > > > +    }
> > > > +
> > > > +    // Returns the sqe  that referenced by uring cmd self.
> > >
> > > "Returns a reference to the uring cmd's SQE"?
> >
> > Agreed, thanks.
> > >
> > > > +    #[inline]
> > > > +    pub fn sqe(&self) -> &IoUringSqe {
> > > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > > +        let ptr = unsafe { (*self.inner.get()).sqe };
> > >
> > > "ptr" isn't very descriptive. How about "sqe"?
> >
> > Sounds good.
> > >
> > > > +        unsafe { IoUringSqe::from_raw(ptr) }
> > >
> > > Similar, missing SAFETY comment for IoUringSqe::from_raw()?
> >
> > Thanks. I missed.
A> > >
> > > > +    }
> > > > +
> > > > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
> > > > +    #[inline]
> > > > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
> > >
> > > I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
> > > example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
> > > into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
> > > definitely need to be pinned in memory. For example,
> > > io_req_normal_work_add() inserts the struct io_kiocb into a linked
> > > list. Probably some sort of pinning is necessary for IoUringCmd.
> >
> > Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
> > in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
> > &mut IoUringCmd for arg.
> 
> I'm still a little worried about exposing &mut IoUringCmd without
> pinning. It would allow swapping the fields of two IoUringCmd's (and
> therefore struct io_uring_cmd's), for example. If a struct
> io_uring_cmd belongs to a struct io_kiocb linked into task_list,
> swapping it with another struct io_uring_cmd would result in
> io_uring_cmd_work() being invoked on the wrong struct io_uring_cmd.
> Maybe it would be okay if IoUringCmd had an invariant that the struct
> io_uring_cmd is not on the task work list. But I would feel safer with
> using Pin<&mut IoUringCmd>. I don't have much experience with Rust in
> the kernel, though, so I would welcome other opinions.

I've thought about this deeply. You're right. exposing &mut without
pinning make it unsafe. User also can make *mut and memmove to anywhere
without unsafe block. It's safest to get NonNull from from_raw and it
returns Pin<&mut IoUringCmd>. from_raw() name is weird. it should be
from_nonnnull()? Also, done() would get Pin<&mut Self>.

Thanks,
Sidong
> 
> Best,
> Caleb
> 
> >
> > >
> > > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > > +        unsafe {
> > > > +            bindings::io_uring_cmd_done(self.inner.get(), ret, res2, issue_flags);
> > > > +        }
> > > > +    }
> > > > +}
> > > > +
> > > > +#[repr(transparent)]
> > > > +pub struct IoUringSqe {
> > > > +    inner: Opaque<bindings::io_uring_sqe>,
> > > > +}
> > > > +
> > > > +impl<'a> IoUringSqe {
> > > > +    pub fn cmd_data(&'a self) -> &'a [Opaque<u8>] {
> > > > +        // SAFETY: The call guarantees that the pointer is not dangling and stays valid
> > > > +        unsafe {
> > > > +            let cmd = (*self.inner.get()).__bindgen_anon_6.cmd.as_ref();
> > > > +            core::slice::from_raw_parts(cmd.as_ptr() as *const Opaque<u8>, 8)
> > >
> > > Why 8? Should be 16 bytes for a 64-byte SQE and 80 bytes for a
> > > 128-byte SQE, right?
> >
> > Yes, it should be 16 bytes or 80 bytes. I'll fix this.
> >
> > >
> > > > +        }
> > > > +    }
> > > > +
> > > > +    #[inline]
> > > > +    pub unsafe fn from_raw(ptr: *const bindings::io_uring_sqe) -> &'a IoUringSqe {
> > >
> > > Take NonNull here too?
> >
> > Yes, Thanks.
> > >
> > > > +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> > > > +        // duration of 'a. The cast is okay because `File` is `repr(transparent)`.
> > > > +        //
> > > > +        // INVARIANT: The caller guarantees that there are no problematic `fdget_pos` calls.
> > >
> > > Why "File" and "fdget_pos"?
> >
> > It's a bad mistake. thanks!
> >
> > Thank you so much for deatiled review!
> >
> > Thanks,
> > Sidong
> > >
> > > Best,
> > > Caleb
> > >
> > > > +        unsafe { &*ptr.cast() }
> > > > +    }
> > > > +}
> > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > > > index 6b4774b2b1c3..fb310e78d51d 100644
> > > > --- a/rust/kernel/lib.rs
> > > > +++ b/rust/kernel/lib.rs
> > > > @@ -80,6 +80,7 @@
> > > >  pub mod fs;
> > > >  pub mod init;
> > > >  pub mod io;
> > > > +pub mod io_uring;
> > > >  pub mod ioctl;
> > > >  pub mod jump_label;
> > > >  #[cfg(CONFIG_KUNIT)]
> > > > --
> > > > 2.43.0
> > > >
> > > >

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Benno Lossin]

On Mon Jul 21, 2025 at 5:04 PM CEST, Caleb Sander Mateos wrote:
> On Mon, Jul 21, 2025 at 1:23 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>> On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
>> > On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>> > > +    }
>> > > +
>> > > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
>> > > +    #[inline]
>> > > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
>> >
>> > I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
>> > example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
>> > into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
>> > definitely need to be pinned in memory. For example,
>> > io_req_normal_work_add() inserts the struct io_kiocb into a linked
>> > list. Probably some sort of pinning is necessary for IoUringCmd.
>>
>> Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
>> in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
>> &mut IoUringCmd for arg.
>
> I'm still a little worried about exposing &mut IoUringCmd without
> pinning. It would allow swapping the fields of two IoUringCmd's (and
> therefore struct io_uring_cmd's), for example. If a struct
> io_uring_cmd belongs to a struct io_kiocb linked into task_list,
> swapping it with another struct io_uring_cmd would result in
> io_uring_cmd_work() being invoked on the wrong struct io_uring_cmd.
> Maybe it would be okay if IoUringCmd had an invariant that the struct
> io_uring_cmd is not on the task work list. But I would feel safer with
> using Pin<&mut IoUringCmd>. I don't have much experience with Rust in
> the kernel, though, so I would welcome other opinions.

Pinning in the kernel isn't much different from userspace. From your
description of what normally happens with `struct io_uring_cmd`, it
definitely must be pinned.

From a quick glance at the patch series, I don't see a way to create a
`IoUringCmd` by-value, which also means that the `done` function won't
be callable (also the `fn pdu(&mut self)` function won't be callable,
since you only ever create a `&IoUringCmd`). I'm not sure if I'm missing
something, do you plan on further patches in the future?

How (aside from `from_raw`) are `IoUringCmd` values going to be created
or exposed to the user?

---
Cheers,
Benno

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Benno Lossin]

On Mon Jul 21, 2025 at 5:47 PM CEST, Sidong Yang wrote:
> On Mon, Jul 21, 2025 at 11:04:31AM -0400, Caleb Sander Mateos wrote:
>> On Mon, Jul 21, 2025 at 1:23 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>> > On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
>> > > On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
>> > > > +    }
>> > > > +
>> > > > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
>> > > > +    #[inline]
>> > > > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
>> > >
>> > > I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
>> > > example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
>> > > into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
>> > > definitely need to be pinned in memory. For example,
>> > > io_req_normal_work_add() inserts the struct io_kiocb into a linked
>> > > list. Probably some sort of pinning is necessary for IoUringCmd.
>> >
>> > Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
>> > in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
>> > &mut IoUringCmd for arg.
>> 
>> I'm still a little worried about exposing &mut IoUringCmd without
>> pinning. It would allow swapping the fields of two IoUringCmd's (and
>> therefore struct io_uring_cmd's), for example. If a struct
>> io_uring_cmd belongs to a struct io_kiocb linked into task_list,
>> swapping it with another struct io_uring_cmd would result in
>> io_uring_cmd_work() being invoked on the wrong struct io_uring_cmd.
>> Maybe it would be okay if IoUringCmd had an invariant that the struct
>> io_uring_cmd is not on the task work list. But I would feel safer with
>> using Pin<&mut IoUringCmd>. I don't have much experience with Rust in
>> the kernel, though, so I would welcome other opinions.
>
> I've thought about this deeply. You're right. exposing &mut without
> pinning make it unsafe.

> User also can make *mut and memmove to anywhere without unsafe block.

How so? Using `*mut T` always needs unsafe.

> It's safest to get NonNull from from_raw and it returns
> Pin<&mut IoUringCmd>.

I don't think you need `NonNull<T>`.

> from_raw() name is weird. it should be from_nonnnull()? Also, done()
> would get Pin<&mut Self>.

That sounds reasonable.

Are you certain that it's an exclusive reference?

---
Cheers,
Benno

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Sidong Yang]

On Mon, Jul 21, 2025 at 06:28:09PM +0200, Benno Lossin wrote:
> On Mon Jul 21, 2025 at 5:47 PM CEST, Sidong Yang wrote:
> > On Mon, Jul 21, 2025 at 11:04:31AM -0400, Caleb Sander Mateos wrote:
> >> On Mon, Jul 21, 2025 at 1:23 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >> > On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
> >> > > On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >> > > > +    }
> >> > > > +
> >> > > > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
> >> > > > +    #[inline]
> >> > > > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
> >> > >
> >> > > I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
> >> > > example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
> >> > > into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
> >> > > definitely need to be pinned in memory. For example,
> >> > > io_req_normal_work_add() inserts the struct io_kiocb into a linked
> >> > > list. Probably some sort of pinning is necessary for IoUringCmd.
> >> >
> >> > Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
> >> > in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
> >> > &mut IoUringCmd for arg.
> >> 
> >> I'm still a little worried about exposing &mut IoUringCmd without
> >> pinning. It would allow swapping the fields of two IoUringCmd's (and
> >> therefore struct io_uring_cmd's), for example. If a struct
> >> io_uring_cmd belongs to a struct io_kiocb linked into task_list,
> >> swapping it with another struct io_uring_cmd would result in
> >> io_uring_cmd_work() being invoked on the wrong struct io_uring_cmd.
> >> Maybe it would be okay if IoUringCmd had an invariant that the struct
> >> io_uring_cmd is not on the task work list. But I would feel safer with
> >> using Pin<&mut IoUringCmd>. I don't have much experience with Rust in
> >> the kernel, though, so I would welcome other opinions.
> >
> > I've thought about this deeply. You're right. exposing &mut without
> > pinning make it unsafe.
> 
> > User also can make *mut and memmove to anywhere without unsafe block.
> 
> How so? Using `*mut T` always needs unsafe.

You're right. Please forget about this.
> 
> > It's safest to get NonNull from from_raw and it returns
> > Pin<&mut IoUringCmd>.
> 
> I don't think you need `NonNull<T>`.

NonNull<T> gurantees that it's not null. It could be also dangling but it's
safer than *mut T. Could you tell me why I don't need it?

> 
> > from_raw() name is weird. it should be from_nonnnull()? Also, done()
> > would get Pin<&mut Self>.
> 
> That sounds reasonable.
> 
> Are you certain that it's an exclusive reference?

As far as I know, yes.

Thanks,
Sidong
> 
> ---
> Cheers,
> Benno

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Sidong Yang]

On Mon, Jul 21, 2025 at 05:52:41PM +0200, Benno Lossin wrote:
> On Mon Jul 21, 2025 at 5:04 PM CEST, Caleb Sander Mateos wrote:
> > On Mon, Jul 21, 2025 at 1:23 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >> On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
> >> > On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@furiosa.ai> wrote:
> >> > > +    }
> >> > > +
> >> > > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
> >> > > +    #[inline]
> >> > > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
> >> >
> >> > I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
> >> > example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
> >> > into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
> >> > definitely need to be pinned in memory. For example,
> >> > io_req_normal_work_add() inserts the struct io_kiocb into a linked
> >> > list. Probably some sort of pinning is necessary for IoUringCmd.
> >>
> >> Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
> >> in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
> >> &mut IoUringCmd for arg.
> >
> > I'm still a little worried about exposing &mut IoUringCmd without
> > pinning. It would allow swapping the fields of two IoUringCmd's (and
> > therefore struct io_uring_cmd's), for example. If a struct
> > io_uring_cmd belongs to a struct io_kiocb linked into task_list,
> > swapping it with another struct io_uring_cmd would result in
> > io_uring_cmd_work() being invoked on the wrong struct io_uring_cmd.
> > Maybe it would be okay if IoUringCmd had an invariant that the struct
> > io_uring_cmd is not on the task work list. But I would feel safer with
> > using Pin<&mut IoUringCmd>. I don't have much experience with Rust in
> > the kernel, though, so I would welcome other opinions.
> 
> Pinning in the kernel isn't much different from userspace. From your
> description of what normally happens with `struct io_uring_cmd`, it
> definitely must be pinned.
> 
> From a quick glance at the patch series, I don't see a way to create a
> `IoUringCmd` by-value, which also means that the `done` function won't
> be callable (also the `fn pdu(&mut self)` function won't be callable,
> since you only ever create a `&IoUringCmd`). I'm not sure if I'm missing
> something, do you plan on further patches in the future?

Sure, this version is full of nonsence. v2 will be better than this.

> 
> How (aside from `from_raw`) are `IoUringCmd` values going to be created
> or exposed to the user?

Nomrally user would gets Pin<&mut IoUringCmd> from MiscDevice::uring_cmd().

Thanks,
Sidong

> 
> ---
> Cheers,
> Benno

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Benno Lossin]

On Tue Jul 22, 2025 at 4:30 PM CEST, Sidong Yang wrote:
> On Mon, Jul 21, 2025 at 06:28:09PM +0200, Benno Lossin wrote:
>> On Mon Jul 21, 2025 at 5:47 PM CEST, Sidong Yang wrote:
>> > It's safest to get NonNull from from_raw and it returns
>> > Pin<&mut IoUringCmd>.
>> 
>> I don't think you need `NonNull<T>`.
>
> NonNull<T> gurantees that it's not null. It could be also dangling but it's
> safer than *mut T. Could you tell me why I don't need it?

Raw pointers have better ergonomics and if you're just passing it back
into ffi, I don't see the point of using `NonNull`...

>> > from_raw() name is weird. it should be from_nonnnull()? Also, done()
>> > would get Pin<&mut Self>.
>> 
>> That sounds reasonable.
>> 
>> Are you certain that it's an exclusive reference?
>
> As far as I know, yes.

So the `IoUringCmd` is not refcounted and it is also not owned by the
`done` callee?

---
Cheers,
Benno

Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

[Sidong Yang]

On Tue, Jul 22, 2025 at 08:52:05PM +0200, Benno Lossin wrote:
> On Tue Jul 22, 2025 at 4:30 PM CEST, Sidong Yang wrote:
> > On Mon, Jul 21, 2025 at 06:28:09PM +0200, Benno Lossin wrote:
> >> On Mon Jul 21, 2025 at 5:47 PM CEST, Sidong Yang wrote:
> >> > It's safest to get NonNull from from_raw and it returns
> >> > Pin<&mut IoUringCmd>.
> >> 
> >> I don't think you need `NonNull<T>`.
> >
> > NonNull<T> gurantees that it's not null. It could be also dangling but it's
> > safer than *mut T. Could you tell me why I don't need it?
> 
> Raw pointers have better ergonomics and if you're just passing it back
> into ffi, I don't see the point of using `NonNull`...

Agreed, from_raw() would be called in rust kernel unsafe condition not in safe
driver. Thinking it over, using a raw pointer would be convenient.

> 
> >> > from_raw() name is weird. it should be from_nonnnull()? Also, done()
> >> > would get Pin<&mut Self>.
> >> 
> >> That sounds reasonable.
> >> 
> >> Are you certain that it's an exclusive reference?
> >
> > As far as I know, yes.
> 
> So the `IoUringCmd` is not refcounted and it is also not owned by the
> `done` callee?

Yes, io_uring_cmd would be allocated in io_submit_sqes() and it's not accessed
concurrently. io_uring_cmd_done() sets its result to io_uring_cmd and prepare
some field to completion.

Thanks,
Sidong

> 
> ---
> Cheers,
> Benno