All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sidong Yang <sidong.yang@furiosa.ai>
To: Benno Lossin <lossin@kernel.org>
Cc: Daniel Almeida <daniel.almeida@collabora.com>,
	Caleb Sander Mateos <csander@purestorage.com>,
	Miguel Ojeda <ojeda@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Jens Axboe <axboe@kernel.dk>,
	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 v2 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd
Date: Tue, 12 Aug 2025 21:19:30 +0900	[thread overview]
Message-ID: <aJsxUpWXu6phEMLR@sidongui-MacBookPro.local> (raw)
In-Reply-To: <DC0B7TRVRFMY.29LDRJOU3WJY2@kernel.org>

On Tue, Aug 12, 2025 at 10:34:56AM +0200, Benno Lossin wrote:
> On Mon Aug 11, 2025 at 4:50 PM CEST, Sidong Yang wrote:
> > On Mon, Aug 11, 2025 at 09:44:22AM -0300, Daniel Almeida wrote:
> >> > There is `uring_cmd` callback in `file_operation` at c side. `Pin<&mut IoUringCmd>`
> >> > would be create in the callback function. But the callback function could be
> >> > called repeatedly with same `io_uring_cmd` instance as far as I know.
> >> > 
> >> > But in c side, there is initialization step `io_uring_cmd_prep()`.
> >> > How about fill zero pdu in `io_uring_cmd_prep()`? And we could assign a byte
> >> > as flag in pdu for checking initialized also we should provide 31 bytes except
> >> > a byte for the flag.
> >> > 
> >> 
> >> That was a follow-up question of mine. Can´t we enforce zero-initialization
> >> in C to get rid of this MaybeUninit? Uninitialized data is just bad in general.
> >> 
> >> Hopefully this can be done as you've described above, but I don't want to over
> >> extend my opinion on something I know nothing about.
> >
> > I need to add a commit that initialize pdu in prep step in next version. 
> > I'd like to get a comment from io_uring maintainer Jens. Thanks.
> >
> > If we could initialize (filling zero) in prep step, How about casting issue?
> > Driver still needs to cast array to its private struct in unsafe?
> 
> We still would have the casting issue.
> 
> Can't we do the following:
> 
> * Add a new associated type to `MiscDevice` called `IoUringPdu` that
>   has to implement `Default` and have a size of at most 32 bytes.
> * make `IoUringCmd` generic
> * make `MiscDevice::uring_cmd` take `Pin<&mut IoUringCmd<Self::IoUringPdu>>`
> * initialize the private data to be `IoUringPdu::default()` when we
>   create the `IoUringCmd` object.

`uring_cmd` could be called multiple times. So we can't initialize
in that time. I don't understand that how can we cast [u8; 32] to
`IoUringPdu` safely. It seems that casting can't help to use unsafe.
I think best way is that just return zerod `&mut [u8; 32]` and
each driver implements safe serde logic for its private data. 

> * provide a `fn pdu(&mut self) -> &mut Pdu` on `IoUringPdu<Pdu>`.
> 
> Any thoughts? If we don't want to add a new associated type to
> `MiscDevice` (because not everyone has to declare the `IoUringCmd`
> data), I have a small trait dance that we can do to avoid that:
> 
>     pub trait IoUringMiscDevice: MiscDevice {
>         type IoUringPdu: Default; // missing the 32 byte constraint
>     }
> 
> and then in MiscDevice we still add this function:
> 
>         fn uring_cmd(
>             _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
>             _io_uring_cmd: Pin<&mut IoUringCmd<Self::IoUringPdu>>,
>             _issue_flags: u32,
>         ) -> Result<i32>
>         where
>             Self: IoUringMiscDevice,
>         {
>             build_error!(VTABLE_DEFAULT_ERROR)
>         }
> 
> It can only be called when the user also implements `IoUringMiscDevice`.
> 
> ---
> Cheers,
> Benno

  reply	other threads:[~2025-08-12 12:19 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-27 15:03 [RFC PATCH v2 0/4] rust: miscdevice: abstraction for uring-cmd Sidong Yang
2025-07-27 15:03 ` [RFC PATCH v2 1/4] rust: bindings: add io_uring headers in bindings_helper.h Sidong Yang
2025-07-27 15:03 ` [RFC PATCH v2 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd Sidong Yang
2025-08-01 13:48   ` Daniel Almeida
2025-08-02 10:52     ` Benno Lossin
2025-08-06 12:38       ` Daniel Almeida
2025-08-06 13:38         ` Benno Lossin
2025-08-08  6:56           ` Sidong Yang
2025-08-08  8:49             ` Benno Lossin
2025-08-08  9:43               ` Sidong Yang
2025-08-09 10:18                 ` Benno Lossin
2025-08-09 12:51                   ` Sidong Yang
2025-08-09 20:22                     ` Benno Lossin
2025-08-10 13:50                       ` Sidong Yang
2025-08-10 14:27                         ` Daniel Almeida
2025-08-10 14:46                           ` Sidong Yang
2025-08-10 20:06                             ` Benno Lossin
2025-08-11 12:34                               ` Sidong Yang
2025-08-11 12:44                                 ` Daniel Almeida
2025-08-11 14:50                                   ` Sidong Yang
2025-08-12  8:34                                     ` Benno Lossin
2025-08-12 12:19                                       ` Sidong Yang [this message]
2025-08-12 12:43                                         ` Daniel Almeida
2025-08-12 13:56                                           ` Sidong Yang
2025-08-12 13:59                                             ` Daniel Almeida
2025-08-12 14:38                                       ` Daniel Almeida
2025-08-13  0:54                                         ` Sidong Yang
2025-08-08 13:55             ` Caleb Sander Mateos
2025-08-09 12:53               ` Sidong Yang
2025-08-05  3:39     ` Sidong Yang
2025-08-05 13:02       ` Daniel Almeida
2025-08-06  9:11         ` Sidong Yang
2025-07-27 15:03 ` [RFC PATCH v2 3/4] rust: miscdevice: add uring_cmd() for MiscDevice trait Sidong Yang
2025-08-01 14:04   ` Daniel Almeida
2025-08-07  7:46     ` Sidong Yang
2025-07-27 15:03 ` [RFC PATCH v2 4/4] samples: rust: rust_misc_device: add uring_cmd example Sidong Yang
2025-08-01 14:11   ` Daniel Almeida
2025-08-07  6:30     ` Sidong Yang
2025-07-27 17:17 ` [RFC PATCH v2 0/4] rust: miscdevice: abstraction for uring-cmd Daniel Almeida
2025-08-01 14:13 ` Daniel Almeida
2025-08-07  6:17   ` Sidong Yang

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=aJsxUpWXu6phEMLR@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.