From: Breno Leitao <leitao@debian.org>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Christian Brauner <brauner@kernel.org>,
oleg@redhat.com, josh@joshtriplett.org, Jan Kara <jack@suse.cz>,
jlayton@kernel.org, axboe@kernel.dk, shakeel.butt@linux.dev,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-team@meta.com
Subject: Re: [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
Date: Wed, 8 Jul 2026 08:11:03 -0700 [thread overview]
Message-ID: <ak5nXRnSSXfA2B75@gmail.com> (raw)
In-Reply-To: <rac56njg4yorvhc5oyrme6bt7ej2vs64umbqkz2np32wpjbcjj@wohli2ntub4s>
On Wed, Jul 08, 2026 at 03:18:58PM +0200, Mateusz Guzik wrote:
> On Wed, Jul 08, 2026 at 05:09:19AM -0700, Breno Leitao wrote:
> > On Tue, Jul 07, 2026 at 05:29:20PM +0200, Mateusz Guzik wrote:
> > > On Tue, Jul 7, 2026 at 5:05 PM Breno Leitao <leitao@debian.org> wrote:
> > > >
> > > > TL;DR: This simplifies the pipe code, unify the page pools, reduce the
> > > > code by 9 lines (not counting comments), and no regressions are seen in
> > > > terms of performance.
> > > >
> > >
> > > This adds an additional acquire + release cycle on the mutex for every
> > > write which preallocates, so I don't see how that's supposed to *not*
> > > slow things down in some capacity.
> >
> > Thanks a lot for the benchmark and the numbers -- a write-heavy pipe
> > workload with the reader and writer on separate CPUs is exactly the case
> > I wanted to make sure doesn't regress.
> >
> > I double-checked it again with your test on different setups and page sizes I
> > don't see the regression you are seeing: the bare-metal numbers are
> > flat-to-positive at your size.
> >
> > I couldn't get my hands on a Sapphire Rapids box easily, so the bare-metal runs
> > are on a Cooper Lake Xeon (the same class I used for the cover letter) and
> > NVIDIA Grace (arm64). Both hosts are completely idle.
> >
> > 1) X86 test
> >
> > CPU: Intel(R) Xeon(R) Platinum 8321HC @ 1.40GHz (Cooper Lake)
> > 1 socket / 26 cores / 2 threads = 52 CPUs
> > L3 35.8 MiB (1 instance), single NUMA node (0-51)
> > max freq == base 1.40GHz (no turbo), so the clock is steady
> >
> > Bare metal, Intel(R) Xeon(R) Platinum 8321HC, your pipen.c
> > (writer on CPU0, reader on CPU1 -- separate physical cores, same
> > socket, shared L3), blocking pipe, 12 x 8s per point, median MB/s:
> >
> > PIPEN_SIZE baseline patched delta
> > 4096 2658 2531 -4.8% (noise?)
> > 32678 3517 3527 +0.3% (your size -- flat)
> > 65536 3072 3358 +9.3%
> >
> > 4096 is a single-page write that barely touches the pool; the swing
> > there is inside the run-to-run variance (sd ~100 MB/s on a ~2600
> > median), so I read it as neutral, not a regression. At 32678/65536
> > the variance is small (sd 18-58 MB/s), so those deltas are real.
> >
> >
> >
> > 2) Arm64 test:
> >
> > Machine / build:
> > - NVIDIA Grace (Neoverse-V2), 72 cores, 1 socket, no SMT,
> > single NUMA node, ~256 GB RAM
> > - Kernel using 64k pages.
> >
> > Results (baseline vs patched):
> >
> > PIPEN_SIZE pages baseline patched delta regime
> > 65536 1 17759 17683 -0.4% want=1 → pool covers it, NO extra lock (no-op)
> > 131072 2 17583 19734 +12.2% prefill + extra lock taken
> > 262144 4 18781 21017 +11.9% prefill + extra lock taken
> > 524288 8 19061 20886 +9.6% = pool max; == Similar to Guzik's 32678 (8 pages)
> > 1048576 16 16842 17110 +1.6% pool overflows; == Guzik's 65536 (16 pages)
>
> I verified the extra lock acquires *do* show up on the profile for me
> (with bpftrace -e 'kprobe:osq_lock { @[kstack()] = count(); }'), so this
> has to be leaving perf on the table.
>
> However, looking at the diff it seems the extra acquires can be
> trivially avoided? See below (untested, consider it an illustration of
> what I mean -- if it works as is I'm fine if it gets folded into your
> patchset without credit).
That is a very nice improvement, thanks for thinking about it and proposing the
patch.
In my quick check on arm64, I haven't seen a big improvement in my Grace arm64
host, These are the numbers I got now:
3-way comparison — arm64 Grace, 64K pages
SIZE pg base mypatch guzik v2/base guzik/base guzik/mypatch
65536 1 17759 17683 17598 -0.4% -0.9% -0.5%
131072 2 17583 19734 19953 +12.2% +13.5% +1.1%
262144 4 18781 21017 20998 +11.9% +11.8% -0.1%
524288 8 19061 20886 20569 +9.6% +7.9% -1.5%
1048576 16 16842 17110 16920 +1.6% +0.5% -1.1%
So, basically just noise compared to this patch in here for that given
CPU/platform.
That said, your approach is better than what I had, and I would like to
integrate it to this patch set. Os it OK if I give a Co-developed-with:
tag?
Thanks for your help here,
--breno
next prev parent reply other threads:[~2026-07-08 15:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-07 15:04 [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool Breno Leitao
2026-07-07 15:04 ` [PATCH v2 1/4] fs/pipe: move the prealloc pool to per-pipe infrastructure Breno Leitao
2026-07-07 15:04 ` [PATCH v2 2/4] fs/pipe: add per-pipe pool push, prefill and trim helpers Breno Leitao
2026-07-07 15:05 ` [PATCH v2 3/4] fs/pipe: switch the read and write paths to the per-pipe pool Breno Leitao
2026-07-07 15:05 ` [PATCH v2 4/4] fs/pipe: remove the old on-stack prealloc helpers and tmp_page[2] Breno Leitao
2026-07-07 15:29 ` [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool Mateusz Guzik
2026-07-08 12:09 ` Breno Leitao
2026-07-08 13:18 ` Mateusz Guzik
2026-07-08 15:11 ` Breno Leitao [this message]
2026-07-08 15:24 ` Mateusz Guzik
2026-07-08 15:54 ` Breno Leitao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ak5nXRnSSXfA2B75@gmail.com \
--to=leitao@debian.org \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=jlayton@kernel.org \
--cc=josh@joshtriplett.org \
--cc=kernel-team@meta.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mjguzik@gmail.com \
--cc=oleg@redhat.com \
--cc=shakeel.butt@linux.dev \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox