Re: [PATCH 1/1] mm: implement page refcount locking via dedicated bit
From: Gladyshev Ilya
Date: Fri Mar 06 2026 - 09:33:14 EST
Okay, I agree. My reasoning was more about "it's not _that_ bad if we miss something outside the allocator" than "everything is fine as it is", but maybe it is really that bad...For example, all pages we add to the page allocator through
__free_pages_core().
You are right that refcount = 0 is tricky. However, for a bad outcome
you will need:
1. Some external reference to this page, through which you try to
increment the refcount;
2. set_page_count(0) somewhere between freeing and "it is safe to alloc"
state.
That is way, way, way too dodgy.
What you should likely do is
(a) Make set_page_count(0) set it to PAGEREF_LOCKED_BIT
(b) Make any places that might skip set_page_count(0) to use it
(c) Document this all extremely thoroughly.
Alternatively, disallow set_page_count(0) ccompletely and add something
like "initialize_page_as_frozen()" or sth. like that.
(Below are my thoughts, not active proposal)
Actually, we can keep "zero is locked" property if we change scheme a little:
If we invert locked bit so that 0 means locked, then we don't care about memset() / GFP_ZERO / set_page(0). But then there is extra bit on each active refcount, and (un)masking will be required on each refcount read/set/etc operation.
We can place locked bit in the highest bit like now, or move it to the lowest. This doesn't make any difference except for how masking is performed (and or shift). I can't really tell which approach is faster (and more optimizable after inline).
This approach doesn't look appealing to me, mostly because of redundant masking. So for now I will try to add proper initialization / documentation in all risky places.
>So adding new pages with zeroed refcount to allocator is safe, because
there are no external references. Zeroing tail page's refcount is safe,
unless someone actually tries to increment its refcount (and this is bug).
Generally, the only unsafe set_page_count() (or any other zeroing) will
be in allocator itself between freeing and allocating. Or maybe I missed
something, and this approach is indeed incorrect
Probably we can think of some debug checks to prevent bugs in "safe"
scenarious
Just take a look at do_migrate_range(), where we do a
page = pfn_to_page(pfn);
folio = page_folio(page);
if (!folio_try_get(folio))
continue;
If that is a page that was added to the buddy (set_page_count(0)) but
either (a) not allocated yet or (b) allocated as frozen that's a problem.
We really rely on
(1) Frozen pages
(2) Buddy pages
(3) Tail pages (compound or non-compoud)
To have a refcount of 0 that *reliably* makes folio_try_get() etc. fail.
Anything else is just playing with fire.