Re: [PATCH v2 3/8] rust: uaccess: add UserSliceWriter::write_slice_partial()

Danilo Krummrich
2025-10-21T14:14:27Z

On Tue Oct 21, 2025 at 4:00 PM CEST, Alice Ryhl wrote:
> On Tue, Oct 21, 2025 at 12:26:15AM +0200, Danilo Krummrich wrote:
>> The existing write_slice() method is a wrapper around copy_to_user() and
>> expects the user buffer to be larger than the source buffer.
>>
>> However, userspace may split up reads in multiple partial operations
>> providing an offset into the source buffer and a smaller user buffer.
>>
>> In order to support this common case, provide a helper for partial
>> writes.
>>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> rust/kernel/uaccess.rs | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> index 2061a7e10c65..40d47e94b54f 100644
>> --- a/rust/kernel/uaccess.rs
>> +++ b/rust/kernel/uaccess.rs
>> @@ -463,6 +463,30 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
>> Ok(())
>> }
>>
>> + /// Writes raw data to this user pointer from a kernel buffer partially.
>> + ///
>> + /// This is the same as [`Self::write_slice`] but considers the given `offset` into `data` and
>> + /// truncates the write to the boundaries of `self` and `data`.
>> + ///
>> + /// On success, returns the number of bytes written.
>> + pub fn write_slice_partial(&mut self, data: &[u8], offset: file::Offset) -> Result<usize> {
>
> I think for the current function signature, it's kind of weird to take a
> file::Offset parameter
>
> On one hand, it is described like a generic function for writing a
> partial slice, and if that's what it is, then I would argue it should
> take usize because it's an offset into the slice.
>
> On another hand, I think what you're actually trying to do is implement
> the simple_[read_from|write_to]_buffer utilities for user slices, but
> it's only a "partial" version of those utilities. The full utility takes
> a `&mut loff_t` so that it can also perform the required modification to
> the offset.

Originally, it was intended to be the latter. And, in fact, earlier code (that
did not git the mailing list) had a &mut file::Offset argument (was &mut i64
back then).

However, for the version I sent to the list I chose the former because I
considered it to be more flexible.

Now, in v2, it's indeed a bit mixed up. I think what we should do is to have
both

fn write_slice_partial(&mut self, data: &[u8], offset: usize) -> Result<usize>

and

fn write_slice_???(&mut self, data: &[u8], offset: &mut file::Offset) -> Result<usize>

which can forward to write_slice_partial() and update the buffer.

Any name suggestions?