[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87v7s4g7ev.fsf@kernel.org>
Date: Wed, 19 Mar 2025 20:14:00 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Christian Schrefl" <chrisi.schrefl@...il.com>
Cc: "Alice Ryhl" <aliceryhl@...gle.com>, "Greg Kroah-Hartman"
<gregkh@...uxfoundation.org>, "Alexander Viro" <viro@...iv.linux.org.uk>,
"Arnd Bergmann" <arnd@...db.de>, "Miguel Ojeda" <ojeda@...nel.org>,
"Boqun Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno
Lossin"
<benno.lossin@...ton.me>, "Trevor Gross" <tmgross@...ch.edu>, "Danilo
Krummrich" <dakr@...nel.org>, "Matthew Maurer" <mmaurer@...gle.com>,
"Lee Jones" <lee@...nel.org>, <linux-kernel@...r.kernel.org>,
<rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST
"Christian Schrefl" <chrisi.schrefl@...il.com> writes:
> Hi Alice
>
> On 11.03.25 3:25 PM, Alice Ryhl wrote:
>> This adds abstractions for the iov_iter type in the case where
>> data_source is ITER_DEST. This will make Rust implementations of
>> fops->read_iter possible.
>>
>> This series only has support for using existing IO vectors created by C
>> code. Additional abstractions will be needed to support the creation of
>> IO vectors in Rust code.
>>
>> These abstractions make the assumption that `struct iov_iter` does not
>> have internal self-references, which implies that it is valid to move it
>> between different local variables, and that you can make a copy of it to
>> get two IO vectors into the same buffers.
>>
>> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
>> ---
>> rust/kernel/iov.rs | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 139 insertions(+), 1 deletion(-)
>>
>> diff --git a/rust/kernel/iov.rs b/rust/kernel/iov.rs
>> index 4498f65e1f65bd964909810c020db3a9f8fae389..dc32c27c5c76d059562fd7c6b9d4b178a8ea7c81 100644
>> --- a/rust/kernel/iov.rs
>> +++ b/rust/kernel/iov.rs
>> @@ -7,7 +7,12 @@
>> //! C headers: [`include/linux/iov_iter.h`](srctree/include/linux/iov_iter.h),
>> //! [`include/linux/uio.h`](srctree/include/linux/uio.h)
>>
>> -use crate::{bindings, prelude::*, types::Opaque};
>> +use crate::{
>> + alloc::{Allocator, Flags},
>> + bindings,
>> + prelude::*,
>> + types::Opaque,
>> +};
>> use core::{marker::PhantomData, mem::MaybeUninit, slice};
>>
>> const ITER_SOURCE: bool = bindings::ITER_SOURCE != 0;
>> @@ -168,3 +173,136 @@ fn clone(&self) -> IovIterSource<'data> {
>> unsafe { core::ptr::read(self) }
>> }
>> }
>> +
>> +/// An IO vector that acts as a destination for data.
>> +///
>> +/// # Invariants
>> +///
>> +/// Must hold a valid `struct iov_iter` with `data_source` set to `ITER_DEST`. The buffers
>> +/// referenced by the IO vector must be valid for writing for the duration of `'data`.
>> +///
>> +/// Note that if the IO vector is backed by a userspace pointer, it is always considered valid for
>> +/// writing.
>> +#[repr(transparent)]
>> +pub struct IovIterDest<'data> {
>> + iov: Opaque<bindings::iov_iter>,
>> + /// Represent to the type system that this value contains a pointer to writable data it does
>> + /// not own.
>> + _source: PhantomData<&'data mut [u8]>,
>> +}
>
> It might be a bit nicer to add a (private) struct 'IovIter' that implements the common operations.
> Then 'IovIterDest' and 'IovIterSource' could store that struct and forward the implementations to
> it.
> But I'm not sure if that's really much better.
Yea, I was thinking the same. Maybe we could have an `IovInner` and a
local `AsIovInner` trait that would give all the shared methods?
I am sure Alice knows the idiomatic way to achieve code sharing here.
Best regards,
Andreas Hindborg
Powered by blists - more mailing lists