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@xxxxxxxxxxxxx> 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.
