Re: [PATCH v3 0/2] rust: page: Add support for existing struct page mappings

[Matthew Wilcox <willy@infradead.org>]

On Wed, Nov 20, 2024 at 08:20:16AM -0800, Boqun Feng wrote:
> On Wed, Nov 20, 2024 at 10:10:44AM +0100, Alice Ryhl wrote:
> > On Wed, Nov 20, 2024 at 5:57 AM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Tue, Nov 19, 2024 at 01:24:01PM +0200, Abdiel Janulgue wrote:
> > > > This series aims to add support for pages that are not constructed by an
> > > > instance of the rust Page abstraction, for example those returned by
> > > > vmalloc_to_page() or virt_to_page().
> > > >
> > > > Changes sinve v3:
> > > > - Use the struct page's reference count to decide when to free the
> > > >   allocation (Alice Ryhl, Boqun Feng).
> > >
> > > Bleh, this is going to be "exciting".  We're in the middle of a multi-year
> > > project to remove refcounts from struct page.  The lifetime of a page
> > > will be controlled by the memdesc that it belongs to.  Some of those
> > > memdescs will have refcounts, but others will not.
> > >
> 
> One question: will the page that doesn't have refcounts has an exclusive
> owner? I.e. there is one owner that's responsible to free the page and
> make sure other references to the page get properly invalidated (maybe
> via RCU?)

It's up to the owner of the page how they want to manage freeing it.
They can use a refcount (folios will still have a refcount, for example),
or they can know when there are no more users of the page (eg slab knows
when all objects in a slab are freed).  RCU is a possibility, but would
be quite unusual I would think.  The model I'm looking for here is that
'page' is too low-level an object to have its own lifecycle; it's always
defined by a higher level object.

> > > We don't have a fully formed destination yet, so I can't give you a
> > > definite answer to a lot of questions.  Obviously I don't want to hold
> > > up the Rust project in any way, but I need to know that what we're trying
> > > to do will be expressible in Rust.
> > >
> > > Can we avoid referring to a page's refcount?
> > 
> > I don't think this patch needs the refcount at all, and the previous
> > version did not expose it. This came out of the advice to use put_page
> > over free_page. Does this mean that we should switch to put_page but
> > not use get_page?

Did I advise using put_page() over free_page()?  I hope I didn't say
that.  I don't see a reason why binder needs to refcount its pages (nor
use a mapcount on them), but I don't fully understand binder so maybe
it does need a refcount.

> I think the point is finding the exact lifetime model for pages, if it's
> not a simple refcounting, then what it is? Besides, we can still
> represent refcounting pages with `struct Page` and other pages with a
> different type name. So as far as I can see, this patch is OK for now.

I don't want Page to have a refcount.  If you need something with a
refcount, it needs to be called something else.