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

[Ilya Gladyshev <ilya.gladyshev@linux.dev>]

On 9/25/26 22:38, Matthew Wilcox wrote:
> On Sat, Sep 12, 2026 at 10:50:10PM +0300, Ilya Gladyshev wrote:
>> This patch reallocates the refcount value range:
>>
>> (1) refcount < 0 means dead refcount (uninit / frozen)
>> (2) refcount = 0 allowed only as a temporary state (see below)
>> (3) refcount > 0 is a regular reference count
>>
>> In other words, refcount is now split into "dead bit" and a 31-bit
>> counter.
> 
> It seems to me that refcount overflow is now a problem.
> 
> We can deliberately increase the refcount on a page by stuffing it into
> a pipe.  Over and over again.  See merge commit 6b3a70773630 and the four
> commits on that branch: f958d7b528b1 88b1a17dfc3e 8fde12ca79af 15fab63e1e57

Thanks for pointing that out!

> Unfortunately, I think the discussion that led to those commits was
> conducted off-list because security.  I wish we had a way to declassify
> thse emails after the fact.
> 
>> +/* Most significant bit in page refcount */
>> +#define PAGEREF_FROZEN_BIT BIT(31)
>> +
>> +/* Page reference counter can be in 3 logical states,
>> + * which are described below with their value representation
>> + *        state              |         value
>> + * (1)  safe with  owners    |   1...INT_MAX
>> + * (2)  safe with no owners  |         0
>> + * (3)  frozen               |  INT_MIN....-1
> 
> I think we need four states.  The first two are the same.
> 
> (3) frozen: 			0xc000'0000 - 0xffff'ffff
> (4) temporarily overflown:	0x8000'0000 - 0xbfff'ffff

Agree.

> (we don't really need that much space for temporary overflow; we could
> have something like 0x8100'0000 as the boundary if that works out better)

Frozen state doesn't really need that much space either, so I guess the 
boundary with the simplest check in page_ref_count wins.
>>   static inline bool __page_count_is_frozen(int count)
>>   {
>> -	return count == 0;
>> +	return count & PAGEREF_FROZEN_BIT;
> 
> This probably becomes '(unsigned)count >> 30 == 3'.  If we choose a
> different boundary then something like (unsigned)count >> 24 >= 0x81.
> 
>>   static inline int page_ref_count(const struct page *page)
>>   {
>> -	return atomic_read(&page->_refcount);
>> +	int val = atomic_read(&page->_refcount);
>> +
>> +	if (unlikely(val & PAGEREF_FROZEN_BIT))
>> +		return 0;
> 
> ... use __page_count_is_frozen() here?

Missed that, thanks!

> and you need to adjust folio_ref_zero_or_close_to_overflow().
> try_get_page() can stay as it is.

Agree.

> (Thanks to Pedro for asking me annoying questions about mapcount
> overflow which prompted me to look at this again)