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

From: Alice Ryhl
Date: Thu Feb 08 2024 - 09:14:03 EST


On Thu, Feb 8, 2024 at 3:02 PM Andreas Hindborg <a.hindborg@samsungcom> wrote:
>
>
> Alice Ryhl <aliceryhl@xxxxxxxxxx> writes:
>
> > On Thu, Feb 1, 2024 at 7:02 AM Trevor Gross <tmgross@xxxxxxxxx> wrote:
> >>
> >> On Wed, Jan 24, 2024 at 6:22 AM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> >> > +/// A pointer to a page that owns the page allocation.
> >> > +///
> >> > +/// # Invariants
> >> > +///
> >> > +/// The pointer points at a page, and has ownership over the page.
> >> > +pub struct Page {
> >> > + page: NonNull<bindings::page>,
> >> > +}
> >>
> >> Shouldn't this be UnsafeCell / Opaque? Since `struct page` contains locks.
> >
> > That only matters when we use a reference. Here, it's behind a raw pointer.
>
> Why is it behind a pointer rather than being transparent over
> `Opaque<bindings::page>` and using a `&Page` instead?

Because `&Page` would not have ownership of the page, but I need
ownership. We also can't use `ARef<Page>` because that has a `clone`
method.

One could introduce an Owned smart pointer and use `Owned<Page>`, but
I think that is out of scope for this patchset.

Alice