On Thu, May 21, 2026 at 07:26:48PM +0200, Mateusz Guzik wrote:
> On Thu, May 21, 2026 at 5:38 PM Breno Leitao <[email protected]> wrote:
>
> I do suspect the best way forward in the long run is repopulate the
> tmp_page array if you have pages to do so.
Ack!
> > + struct anon_pipe_prealloc prealloc = {};
> >
>
> I forgot this bit is going to be a problem with gcc. I verified it
> emits rep stosq in place, which is going to completely unnecessarily
> slow things down especially on older uarchs. This is a known bug with
> gcc doing terrible job optimizing this.
>
> The problem will be avoided by merely initializing the count to 0.
> which looks kind of ugly if done here, but see below.
Ack, I can do it inside anon_pipe_get_page_prealloc()
static void anon_pipe_get_page_prealloc(struct anon_pipe_prealloc
*prealloc,
size_t total_len)
{
prealloc->count = 0;
if (total_len <= PAGE_SIZE)
return;
...
}
> > + /*
> > + * Bulk pre-allocate pages outside pipe->mutex for writes that span
> > more
> > + * than one full page. alloc_page() with GFP_HIGHUSER may sleep
> > doing
> > + * reclaim and runs memcg charging, so doing it under the mutex
> > + * extends the critical section and stalls the reader. The merge
> > path
> > + * handles sub-PAGE_SIZE writes without a fresh page; single-page
> > and
> > + * >PIPE_PREALLOC_MAX-page writes fall back to alloc_page() under
> > the
> > + * mutex for the remainder.
> > + */
> > + if (total_len > PAGE_SIZE)
> > + anon_pipe_get_page_prealloc(&prealloc, total_len);
> > +
>
> I don't think this comment belongs here, it should move above the
> prealloc routine.
>
> How about this: anon_pipe_get_page_prealloc gets called
> unconditionally and expects an uninitialized prealloc struct. For
> total_len > PAGE_SIZE you roll with the current code. Otherwise you
> just set ->count to 0, which prevents the ->pages array from being
> looked at. You can even pre-set to 0 on entry, just don't memset the
> entire obj.
Thanks, let me respin a v2.
--breno
