Re: [PATCH v6 3/3] mm: implement page refcount locking via dedicated bit

[Zi Yan]

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.

-- 
Best Regards,
Yan, Zi