Re: [PATCH 04/25] mm/fbatch: lru bit set, no extra ref, while folio on per-cpu fbatch

[Hugh Dickins]

On Fri, 28 Aug 2026, Matthew Wilcox wrote:
> On Fri, Aug 28, 2026 at 01:04:34AM -0700, Hugh Dickins wrote:
> > On Thu, 27 Aug 2026, Kiryl Shutsemau wrote:
> > > On Mon, Aug 24, 2026 at 07:01:20AM -0700, Hugh Dickins wrote:
> > > > Treat folios on a per-cpu fbatch as if they were already on the lruvec:
> > > > with PG_lru set, without holding an extra reference. This will enable
> > > > the removal of most lru_add_drain() and lru_add_drain_all() calls soon.
> > > > 
> > > > Recognize such a folio by 0x02 set in the folio->lru.next pointer by
> > > > folio_add_lru().
> > > 
> > > Hm. pfmemalloc (__GFP_MEMALLOC) thingy already claims the bit. Is it
> > > safe because such memory is never on LRU?
> > > 
> > > Are pfmemalloc and PG_lru mutually exclusive?
> > > 
> > > Do we want to be explicit about this? Like, folio/page_is_pfmemalloc()
> > > shouldn't return true for PG_lru folios/pages or something.
> > 
> > Gosh, thanks so much for pointing that out: I was completely ignorant
> > of the the pfmemalloc use, and a bit (bit 1!) shocked to learn of it
> > (why wouldn't they just reuse a pageflag, I wonder? but doesn't matter).
> 
> The idea is that it is used temporarily to communicate from the
> page allocator to the caller "This only succeeded because of
> PF_MEMALLOC".  Making it a page flag would require all callers be
> aware of it -- most simply do not care.  PF_MEMALLOC was set on
> their behalf and there is nothing they can usefully do with this
> information.
> 
> So we want to communicate it in a way that allows the unaware caller to
> discard the information, and I chose bit 1 of page->lru.next.  We
> used to use page->index == -1UL (which was also naturally overwritten by
> the unaware caller), but we needed to have it be part of PP_SIGNATURE
> and that needed to be not part of page->index ...
> 
> Commit dc8cf7550a70 if you want to read more about it.

Thanks for explaining, I had looked at some of the other commits
implicated, but not that one.

So, it's a special snowflake which can melt away before it interferes
with anyone not interested (which a page flag would not): yes, that
makes some sense.

> 
> We do use a page flag in slab; we check the pfmalloc bit and move it
> into a page flag (SL_pfmemalloc).  But we don't use it for folios.
> 
> While there is a folio_is_pfmemalloc(), I think that was a mistake
> and it should now be deleted.  It hasn't been used since slab was
> converted away from folios last November.
> 
> > Anyway, as you've rightly guessed, it's not a problem at all: these
> > mm/folio.c and mm/mlock.c per-cpu fbatches are entirely for folios;
> > and if any pfmemalloced page ever get used for a folio (dunno) and
> > put on an fbatch for LRU, then of course its use of lru.next is
> > immediately overwritten (first by what this patch writes in lru_next,
> > then later by the lru.next pointer for whatever LRU it goes on to -
> > just as before this patch).
> > 
> > If you were to tell me that some subsystem uses PG_lru for some
> > other purpose, then I would have to get more worried; but we can
> > be fairly sure that's not so, since mm/compaction.c for one relies
> > on konwing it's free to  play with PG_lru folios.
> > 
> > Whether a folio is ever allocated with __GFP_MEMALLOC, I'm not
> > certain (haven't looked), but there is no need to exclude that:
> > it simply would not retain that page_is_pfmemalloc() info across
> > folio_add_lru().
> 
> That's the correct thinking.
> 
> How about this patch?

Oh, when I said "This certainly deserves a comment somewhere",
I meant an acknowledgement somewhere in my series, not a complaint
that it had not been already commented.

Once Kiryl pointed me, I found the comment in page_is_pfmemalloc()
good enough: the problem is not the wording of the comment, but
knowing where and when to look for such a comment. We have many
header files, and understandably no registry of low bits in pointers.
Wherever you put a good comment, I'd have missed it.

So, I was okay with the original, but your longer explanation below
looks fine, and the removal of folio_is_pfmemalloc() good, but best of
all is the comment line (nit: wants a full stop) you add in mm_types.h.

Thanks,
Hugh

> 
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index dd09c438fa23..fcefffb52d3e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3122,36 +3122,25 @@ static inline void *folio_address(const struct folio *folio)
>  	return page_address(&folio->page);
>  }
>  
> -/*
> - * Return true only if the page has been allocated with
> - * ALLOC_NO_WATERMARKS and the low watermark was not
> - * met implying that the system is under some pressure.
> +/**
> + * page_is_pfmemalloc - Page allocation should have failed
> + * @page: The just-allocated page
> + *
> + * Usually the page allocator keeps some memory in reserve.  If
> + * __GFP_MEMALLOC is used, the page allocator can dip into those
> + * reserves.  The caller can find out if the allocation came from
> + * the reservers by calling this function.
> + *
> + * If the caller does not care, it can simply use the page as
> + * normal.  The field that the information is stored in is usually
> + * overwritten by most uses of a page.  Nobody should call this for
> + * a page they did not allocate as it can easily have false positives.
>   */
>  static inline bool page_is_pfmemalloc(const struct page *page)
>  {
> -	/*
> -	 * lru.next has bit 1 set if the page is allocated from the
> -	 * pfmemalloc reserves.  Callers may simply overwrite it if
> -	 * they do not need to preserve that information.
> -	 */
>  	return (uintptr_t)page->lru.next & BIT(1);
>  }
>  
> -/*
> - * Return true only if the folio has been allocated with
> - * ALLOC_NO_WATERMARKS and the low watermark was not
> - * met implying that the system is under some pressure.
> - */
> -static inline bool folio_is_pfmemalloc(const struct folio *folio)
> -{
> -	/*
> -	 * lru.next has bit 1 set if the page is allocated from the
> -	 * pfmemalloc reserves.  Callers may simply overwrite it if
> -	 * they do not need to preserve that information.
> -	 */
> -	return (uintptr_t)folio->lru.next & BIT(1);
> -}
> -
>  /*
>   * Only to be called by the page allocator on a freshly allocated
>   * page.
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 09cb1e18bc16..9bcfc4213b91 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -85,6 +85,7 @@ struct page {
>  	 * WARNING: bit 0 of the first word is used for PageTail(). That
>  	 * means the other users of this union MUST NOT use the bit to
>  	 * avoid collision and false-positive PageTail().
> +	 * Bit 1 of the first word is used by page_is_pfmemalloc()
>  	 */
>  	union {
>  		struct {	/* Page cache and anonymous pages */