Thread (27 messages) flat view 27 messages, 6 authors, 8d ago

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

From: Andreas Hindborg <a.hindborg@kernel.org>
Date: 2026-09-07 12:38:44
Also in: dri-devel, driver-core, linux-fsdevel, linux-mm, linux-pci, linux-pm, linux-pwm, linux-security-module, linux-usb, lkml, rust-for-linux

Alice Ryhl [off-list ref] writes:
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`.

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?

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?


Best regards,
Andreas Hindborg
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help