Re: [PATCH 3/3] rust: add abstraction for `struct page`

From: Andreas Hindborg (Samsung)
Date: Tue Jan 30 2024 - 04:29:57 EST



Boqun Feng <boqun.feng@xxxxxxxxx> writes:

> On Wed, Jan 24, 2024 at 11:20:23AM +0000, Alice Ryhl wrote:

>> +
>> + /// Maps the page and writes into it from the given buffer.
>> + ///
>> + /// # Safety
>> + ///
>> + /// Callers must ensure that `src` is valid for reading `len` bytes.
>> + pub unsafe fn write(&self, src: *const u8, offset: usize, len: usize) -> Result {
>
> Use a slice like type as `src` maybe? Then the function can be safe:
>
> pub fn write<S: AsRef<[u8]>>(&self, src: S, offset: usize) -> Result
>
> Besides, since `Page` impl `Sync`, shouldn't this `write` and the
> `fill_zero` be a `&mut self` function? Or make them both `unsafe`
> because of potential race and add some safety requirement?

We can add a safe version that takes a slice later, as here [1]. Same
for the with_* that take a closure.

It would be nice to model ownership of pages that are only mapped in
kernel with `&mut`.

BR Andreas

[1] https://github.com/metaspace/linux/blob/702026e6645193fc89b7d55e00dac75fd492bfb8/rust/kernel/pages.rs#L178