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

[David Hildenbrand (Arm)]

On 9/25/26 09:03, Ilya Gladyshev wrote:
> September 25, 2026 at 5:44 AM, Zi Yan wrote:
>>
>> On Thu Sep 24, 2026 at 4:06 PM EDT, Linus Torvalds wrote:
>>
>>>
>>>
>>>  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).

I was playing with exact this idea when I was reviewing one of the earlier
versions; didn't manage to make it fly in a nice way :)

-- 
Cheers,

David