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

[Ilya Gladyshev]

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)