Re: [PATCH v20 4/8] rust: page: convert to `Ownable`'

On Mon, Sep 7, 2026 at 2:38 PM Andreas Hindborg [off-list ref] wrote:

Alice Ryhl [off-list ref] writes:

quoted

On Sun, Sep 6, 2026 at 3:02 PM Gary Guo [off-list ref] wrote:

quoted

On Tue Aug 25, 2026 at 2:20 PM BST, Alice Ryhl wrote:

quoted

On Mon, Aug 24, 2026 at 01:17:56PM +0200, Andreas Hindborg wrote:

quoted

+        // SAFETY: We just successfully allocated a page, so we now have ownership of the newly
+        // allocated page. We transfer that ownership to the new `Owned<Page>` object.
+        // Since `Page` is transparent, we can cast the pointer directly.
+        Ok(unsafe { Owned::from_raw(page.cast()) })

This doesn't satisfy the safety requirements of Owned::from_raw()
because the page may be used with vm_insert_page(), which increments its
refcount and causes it to be shared the vma system, and this occurs
before Page::release() is called.

I suppose the existing vm_insert_page() abstraction we have is already
problematic, because it uses `&Page`?

Maybe we want to change the API to use `ARef<Page>` so it already has to be
shared? Conceptually it takes a reference count from a `&Page`, which isn't
possible because `Page` is not `AlwaysRefCounted`, so it needs a `&ARef<Page>`
to be able to do that op.

Honestly, the problem is the safety requirements of Owned::from_raw().
Pages have a "special" main reference, and free_page() does more than
put_page(). It even does something when the refcount does not hit
zero.

The correct behavior for Page is to allow the user to hold one
Owned<Page> whose drop calls free_page(), *plus* any number of
ARef<Page> references that invoke put_page() on drop. This way, the
owned page controls the special drop codepath.

This does not mesh well with the model of `Owned` behaving like
`UniqueArc` to `ARef` behaving like an `Arc`.

It's a different case, I agree.

Please help me understand; with page having a main ref and an auxiliary
refcount, if we model that with a single `Owned<Page>` and a number of
`ARef<Page>`, what would happen in the case where the main ref (`Owned`)
is dropped first? Is this legal?

Yes, it's legal.

My intuition here would be to follow Garry's suggestion and have
`vm_insert_page` take an `ARef<Page>`. Can you elaborate why this is not
an option?

That would be the wrong ownership semantics. The C side increments the
refcount rather than take ownership of a passed-in refcount, so the
correct argument type is &Page.

Now, for the use-case in Binder I believe we could switch to
put_page(), as we don't need the extra stuff from free_page(), in
which case we only want ARef<Page> and do not require Owned support.
But as long as we are calling free_page(), it needs to not be
clonable.

Alice