Re: [PATCH v2 1/2] fs/pipe: pre-allocate pages outside pipe->mutex in anon_pipe_write

[Breno Leitao <leitao@debian.org>]

On Fri, May 22, 2026 at 12:51:40PM -0400, Jeff Layton wrote:
> On Fri, 2026-05-22 at 09:44 -0700, Breno Leitao wrote:
> > +/*
> > + * Pre-allocate pages outside pipe->mutex for multi-page writes.
> > + * alloc_page() with GFP_HIGHUSER can sleep in reclaim and runs memcg
> > + * charging; doing it under the mutex stalls a concurrent reader.
> > + *
> > + * Loop alloc_page() instead of alloc_pages_bulk_*(): the bulk path refuses
> > + * __GFP_ACCOUNT under memcg (see commit 8dcb3060d81d "memcg: page_alloc:
> > + * skip bulk allocator for __GFP_ACCOUNT") and silently degrades to a single
> > + * page. A per-page loop keeps memcg accounting and the task NUMA mempolicy
> > + * honoured for every page; the per-call overhead is small compared to the
> > + * pipe->mutex hold-time being shrunk. Any shortfall is covered by the
> > + * in-lock alloc_page() fallback in anon_pipe_get_page().
> > + */
> > +static void anon_pipe_get_page_prealloc(struct anon_pipe_prealloc *prealloc,
> > +					size_t total_len)
> > +{
> > +	unsigned int want, i;
> > +	struct page *page;
> > +
> > +	prealloc->count = 0;
> > +	if (total_len <= PAGE_SIZE)
> > +		return;
> > +
> > +	want = min_t(unsigned int, DIV_ROUND_UP(total_len, PAGE_SIZE),
> > +		     PIPE_PREALLOC_MAX);
> > +
> > +	for (i = 0; i < want; i++) {
> > +		page = alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
> > +		if (!page)
> > +			break;
> > +		prealloc->pages[prealloc->count++] = page;
> > +	}
> 
> I believe alloc_pages_bulk() is supposed to be more efficient when
> allocating several pages at once like this.

Thanks Jeff. I understand bulk allocators refuses __GFP_ACCOUNT under memcg.
(That is the reason I've CCed Shakeel in this patchset).

Anyway, reading the code it shows me:

  unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
                          nodemask_t *nodemask, int nr_pages,
                          struct page **page_array)
  {

	...
        /* Bulk allocator does not support memcg accounting. */
        if (memcg_kmem_online() && (gfp & __GFP_ACCOUNT))
                goto failed;


Thanks for the review,
--breno