Re: [PATCH v2 1/2] mm: drop page refcount zero state semantics

[Gorbunov Ivan]

On 4/23/2026 10:32 PM, Zi Yan wrote:
> On 20 Apr 2026, at 4:01, Gorbunov Ivan wrote:
>
> > This patch proposes 2 changes
> > 1) Deprecate invariant that the value stored in reference count of frozen page is 0
> > (Getter functions folio_ref_count/page_ref_count must still return 0 for frozen pages)
> > 2) Allow modification operations like page_ref_add to be used only with
> >     pages with owners
>
> Should we also ban calling set_page_count() except init_page_count() and
> set_page_count_as_frozen()? So that no one can manipulate page refcount
> randomly. The same applies to folio_set_count().
>
> All set_page_count(..., 0) are converted to set_page_count_as_frozen().
> All set_page_count(..., 1) are converted to init_page_count().
>
> We might need an init_folio_count() for folio_set_count(..., 1) users
> and folio_set_count() has no other use.
>
> I see set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1); from
> mm/page_frag_cache.c and it could be converted to page_ref_unfreeze(),
> since above it there is free_frozen_page(), which makes me think the
> page is frozen.

Overall I agree, but this is out of scope for this patch main change. 
This could be handled as an independent patch later.

> >   static inline int page_ref_dec_return(struct page *page)
> >   {
> >   	int ret = atomic_dec_return(&page->_refcount);
> > +	VM_BUG_ON(__page_count_is_frozen(ret + 1));
> >
> >   	if (page_ref_tracepoint_active(page_ref_mod_and_return))
> >   		__page_ref_mod_and_return(page, -1, ret);
>
> VM_WARN_ON_ONCE() might be better?

With "locking via dedicated bit" patch modification, any function which 
drop reference counter to zero changes the behavior of 
refcount(page_add_unless would succeed and etc.). This could lead to 
hazardous results. Therefore, we decided to prohibit that completely and 
make zero value only possible as middle state during dec_and_test.


> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 65e702fade61..27734cf795da 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1639,14 +1639,14 @@ void __meminit __free_pages_core(struct page *page, unsigned int order,
> >   		for (loop = 0; loop < nr_pages; loop++, p++) {
> >   			VM_WARN_ON_ONCE(PageReserved(p));
> >   			__ClearPageOffline(p);
> > -			set_page_count(p, 0);
> > +			set_page_count_as_frozen(p);
> >   		}
> >
> >   		adjust_managed_page_count(page, nr_pages);
> >   	} else {
> >   		for (loop = 0; loop < nr_pages; loop++, p++) {
> >   			__ClearPageReserved(p);
> > -			set_page_count(p, 0);
> > +			set_page_count_as_frozen(p);
> >   		}
> >
> >   		/* memblock adjusts totalram_pages() manually. */
> > --
>
> Not sure about these two, they freeze p and p goes into buddy without
> unfreeze to refcount==0. But at the beginning, you said there are two
> states:
> 1) Unfrozen page which right now has no explicit owner
> 2) Frozen page
>
> 1) means free pages in buddy. But the above change brings frozen pages
> into buddy, mixing the two states into one. Is that intended?


Since state 1 only appears during dec_and_test, pages in the buddy 
allocator are considered frozen. So there is no mixing of states.