Re: [PATCH v6 3/3] mm: implement page refcount locking via dedicated bit
From: Ilya Gladyshev
Date: Fri Sep 25 2026 - 03:05:52 EST
September 25, 2026 at 5:44 AM, Zi Yan wrote:
>
> On Thu Sep 24, 2026 at 4:06 PM EDT, Linus Torvalds wrote:
>
> >
> > On Thu, 24 Sept 2026 at 11:52, David Hildenbrand (Arm) <david@xxxxxxxxxx> wrote:
> >
> > >
> > > There needs to be a common page freeing function that dipatches stuff.
> > >
> > I think this all needs to be entirely generic to all pages. Agreed.
> > Everybody needs to do it right.
> >
> It assumes all callers of page_ref_dec_and_test() are free page
> functions, but there are two exceptions:
>
> 1. compaction_free() uses folio_put_testzero(), which calls
> page_ref_dec_and_test() to put not used migration dst page back to
> isolated freelist instead of freeing them.
>
> 2. virtio_mem_fake_offline_going_offline() drops the last reference of
> fake-offline pages, so that they can be offline.
>
> They will need to be converted to not use page_ref_dec_and_test() in
> addition to a common page freeing function. And any such future use
> needs to be banned.
>
> >
> > But I think that in the scenario that Ilya mentioned:
> >
> > Refcount operations as follows (FR = FROZEN):
> >
> > A: DEC 1 -> 0 [ page_ref_dec_and_test() ]
> > B: INC 0 -> 1 [ folio_try_get() ] => success
> > B: DEC 1 -> 0 [ page_ref_dec_and_test() ]
> > B: CAS 0 -> FR [ page_ref_dec_and_test() ]
> > B: * deallocates via __folio_put() *
> >
> > C: * re-allocates page *
> > C: DEC 1 -> 0 [ page_ref_dec_and_test() ]
> > A: CAS 0 -> FR [ page_ref_dec_and_test() ] => success
> > A: * deallocates directly via free_frozen_pages() * => BUG
> >
> > the big here is that the de-allocation was done not from the state of
> > the *page*, but from the state of who finished it off.
> >
> > So that
> >
> > A: CAS 0 -> FR [ page_ref_dec_and_test() ] => success
> >
> > is not a problem per se. It's a fine case of "we had one single unique
> > final free".
> >
> > The failure is that A then goes on to do that
> >
> > "deallocates directly via free_frozen_pages"
> >
> > because A mis-judged the page as having the old stale state that it
> > had when *A* was using it.
> >
> > IOW, I think the soluition is either:
> >
> > (a) every user of put_page_testzero(page) agrees tro do exactly the
> > same thing and there is no difference between how that final
> > "0->FROZEN" is dealt with, so it doesn't matter that A is freeing a
> > page that C allocated for something else
> >
> > or
> >
> > (b) the CAS 0 -> FR state sequence always looks at the *page* to
> > decide what to do, never at the caller state (ie A will look at
> > whatever C wrote when it allocated the page to know how to do that
> > final free).
> >
> > Hmm?
> >
> Just wonder if we could make 1 -> FROZEN atomic to get rid of the gap.
Hmmm, I’m afraid that since you need CAS for a safe 1->FR transition,
it will result in a CAS loop for the decrement itself (like in
atomic_sub_unless). And this will introduce scalability issues just like
in folio_try_get(), but this time for everyone...
Unless there is a way to change the refcount encoding even further and
fix this gap.
---
Ilya Gladyshev // foxido.dev