[linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[kernel test robot]

Hello,

kernel test robot noticed a 18.5% regression of lmbench3.PIPE.bandwidth.MB/sec on:


commit: f7f4665dc520fd8bbc1db20e59945773e03744a0 ("fs/pipe: unify the page pools into a single per-pipe pool") https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master

testcase: lmbench3
config: x86_64-rhel-9.4
compiler: gcc-14
test machine: 224 threads 2 sockets Intel(R) Xeon(R) (Sapphire Rapids) with 128G memory
parameters:

	test_memory_size: 50%
	nr_threads: 20%
	mode: development
	test: PIPE
	cpufreq_governor: performance

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <yi1.lai@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202611041404.befe64cd-lkp@intel.com

Details are as below:
-------------------------------------------------------------------------------------------------->

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20261104/202611041404.befe64cd-lkp@intel.com

=========================================================================================
compiler/cpufreq_governor/kconfig/mode/nr_threads/rootfs/tbox_group/test/test_memory_size/testcase:
  gcc-14/performance/x86_64-rhel-9.4/development/20%/debian-13-x86_64-20250902.cgz/lkp-spr-2sp2/PIPE/50%/lmbench3

commit: 
  c610d2d078 ("fs: annotate inode timestamp accessors")
  f7f4665dc5 ("fs/pipe: unify the page pools into a single per-pipe pool")

c610d2d0787961cd f7f4665dc520fd8bbc1db20e599
---------------- --------------------------- 
         %stddev     %change         %stddev
             \          |                \  
     29142 ±  2%     -18.5%      23737        lmbench3.PIPE.bandwidth.MB/sec
    191.80 ±  8%     +54.1%     295.65 ±  5%  lmbench3.time.elapsed_time
    191.80 ±  8%     +54.1%     295.65 ±  5%  lmbench3.time.elapsed_time.max
     23261 ± 16%     +19.3%      27743 ±  5%  lmbench3.time.involuntary_context_switches
      9853 ± 16%     +27.9%      12605 ±  5%  lmbench3.time.system_time
     51.80 ±  2%     +11.6%      57.80 ±  3%  lmbench3.time.user_time
 2.928e+08 ±  3%     +26.6%  3.706e+08 ±  4%  lmbench3.time.voluntary_context_switches


Disclaimer:
Results have been estimated based on internal Intel analysis and are provided
for informational purposes only. Any difference in system hardware or software
design or configuration may affect actual performance.


-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Christian Brauner]

On Thu, Nov 05, 2026 at 08:38:45AM +0800, kernel test robot wrote:
> 
> 
> Hello,
> 
> kernel test robot noticed a 18.5% regression of lmbench3.PIPE.bandwidth.MB/sec on:
> 
> 
> commit: f7f4665dc520fd8bbc1db20e59945773e03744a0 ("fs/pipe: unify the page pools into a single per-pipe pool") https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master

@Breno can you reproduce this?
It's something we should at least investigate.

> 
> testcase: lmbench3
> config: x86_64-rhel-9.4
> compiler: gcc-14
> test machine: 224 threads 2 sockets Intel(R) Xeon(R) (Sapphire Rapids) with 128G memory
> parameters:
> 
> 	test_memory_size: 50%
> 	nr_threads: 20%
> 	mode: development
> 	test: PIPE
> 	cpufreq_governor: performance
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <yi1.lai@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202611041404.befe64cd-lkp@intel.com
> 
> Details are as below:
> -------------------------------------------------------------------------------------------------->
> 
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20261104/202611041404.befe64cd-lkp@intel.com
> 
> =========================================================================================
> compiler/cpufreq_governor/kconfig/mode/nr_threads/rootfs/tbox_group/test/test_memory_size/testcase:
>   gcc-14/performance/x86_64-rhel-9.4/development/20%/debian-13-x86_64-20250902.cgz/lkp-spr-2sp2/PIPE/50%/lmbench3
> 
> commit: 
>   c610d2d078 ("fs: annotate inode timestamp accessors")
>   f7f4665dc5 ("fs/pipe: unify the page pools into a single per-pipe pool")
> 
> c610d2d0787961cd f7f4665dc520fd8bbc1db20e599
> ---------------- --------------------------- 
>          %stddev     %change         %stddev
>              \          |                \  
>      29142 ±  2%     -18.5%      23737        lmbench3.PIPE.bandwidth.MB/sec
>     191.80 ±  8%     +54.1%     295.65 ±  5%  lmbench3.time.elapsed_time
>     191.80 ±  8%     +54.1%     295.65 ±  5%  lmbench3.time.elapsed_time.max
>      23261 ± 16%     +19.3%      27743 ±  5%  lmbench3.time.involuntary_context_switches
>       9853 ± 16%     +27.9%      12605 ±  5%  lmbench3.time.system_time
>      51.80 ±  2%     +11.6%      57.80 ±  3%  lmbench3.time.user_time
>  2.928e+08 ±  3%     +26.6%  3.706e+08 ±  4%  lmbench3.time.voluntary_context_switches
> 
> 
> Disclaimer:
> Results have been estimated based on internal Intel analysis and are provided
> for informational purposes only. Any difference in system hardware or software
> design or configuration may affect actual performance.
> 
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 
> 
> 

-- 

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Breno Leitao]

On Thu, Nov 05, 2026 at 04:04:58PM +0100, Christian Brauner wrote:
> On Thu, Nov 05, 2026 at 08:38:45AM +0800, kernel test robot wrote:
> > kernel test robot noticed a 18.5% regression of lmbench3.PIPE.bandwidth.MB/sec on:
> > 
> > commit: f7f4665dc520fd8bbc1db20e59945773e03744a0 ("fs/pipe: unify the page pools into a single per-pipe pool") https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master


> @Breno can you reproduce this?
> It's something we should at least investigate.

Ack, I will check it , but not today unfortunately.

I will come back to it tomorrow, and until end of week I will report my
findings.

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Mateusz Guzik]

On Thu, Nov 5, 2026 at 4:28 PM Breno Leitao <leitao@debian.org> wrote:
>
> On Thu, Nov 05, 2026 at 04:04:58PM +0100, Christian Brauner wrote:
> > On Thu, Nov 05, 2026 at 08:38:45AM +0800, kernel test robot wrote:
> > > kernel test robot noticed a 18.5% regression of lmbench3.PIPE.bandwidth.MB/sec on:
> > >
> > > commit: f7f4665dc520fd8bbc1db20e59945773e03744a0 ("fs/pipe: unify the page pools into a single per-pipe pool") https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
>
>
> > @Breno can you reproduce this?
> > It's something we should at least investigate.
>
> Ack, I will check it , but not today unfortunately.
>
> I will come back to it tomorrow, and until end of week I will report my
> findings.

I figured it out.

The test at hand has multiple readers and writers using the same pipe,
with 64KB i/os.

Prior to the patch each writer had its own private pool to draw from,
guaranteeing no allocation will happen under the mutex.

With the patch the reader exit also drains the pool.

As a result tons of allocation ends up happening under the lock again:

-   94.88%    94.88%  bw_pipe          [kernel.kallsyms]     [k]
__alloc_frozen_pages_noprof
     94.88% __internal_syscall_cancel
        entry_SYSCALL_64_after_hwframe
        do_syscall_64
        ksys_write
        vfs_write
      - anon_pipe_write
         - 47.46% alloc_pages_noprof
              alloc_pages_mpol
              __alloc_frozen_pages_noprof
         - 47.42% anon_pipe_prefill_and_lock
              alloc_pages_noprof
              alloc_pages_mpol
              __alloc_frozen_pages_noprof

I don't think multiple high-volume writers vs multiple high
volume-readers is a realistic scenario and I would not bother fixing
it.

However, should someone want to address it, I think the patch should
be reverted and the resulting state tweaked. The patch at hand came
with some win for throughput in one reader vs one writer scenario,
which I'm pretty sure stems from postponing page deallocation until
after the mutex is released -- the kernel prior to any of this would
still put_page on the reader side under the lock. This bit would have
to get tweaked to recover the loss in that scenario (not to be
confused with the loss reported here).

tl;dr i think this is a EIRRELEVANT kind of a report

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Breno Leitao]

On Thu, Nov 05, 2026 at 04:57:33PM +0100, Mateusz Guzik wrote:
> On Thu, Nov 5, 2026 at 4:28 PM Breno Leitao <leitao@debian.org> wrote:
>
> The test at hand has multiple readers and writers using the same pipe,
> with 64KB i/os.
> 
> Prior to the patch each writer had its own private pool to draw from,
> guaranteeing no allocation will happen under the mutex.
> 
> With the patch the reader exit also drains the pool.
> 
> As a result tons of allocation ends up happening under the lock again:
> 
> -   94.88%    94.88%  bw_pipe          [kernel.kallsyms]     [k]
> __alloc_frozen_pages_noprof
>      94.88% __internal_syscall_cancel
>         entry_SYSCALL_64_after_hwframe
>         do_syscall_64
>         ksys_write
>         vfs_write
>       - anon_pipe_write
>          - 47.46% alloc_pages_noprof
>               alloc_pages_mpol
>               __alloc_frozen_pages_noprof
>          - 47.42% anon_pipe_prefill_and_lock
>               alloc_pages_noprof
>               alloc_pages_mpol
>               __alloc_frozen_pages_noprof

Ack, this matches what I found as well. Thanks for investigating it.

Because the preallocated pages are not private to the writer anymore,
the reader flushes them while the writer waits for room in the pipe,
and the writer then has to allocate them inside the lock.

This basically loses the wins[0] from commit 212ed884a1aeef ("fs/pipe:
pre-allocate pages outside pipe->mutex in anon_pipe_write") for this
specific scenario.

> I don't think multiple high-volume writers vs multiple high
> volume-readers is a realistic scenario and I would not bother fixing
> it.

Agree, but I would like to spend some cycles on it.

> However, should someone want to address it, I think the patch should
> be reverted and the resulting state tweaked. The patch at hand came
> after the mutex is released -- the kernel prior to any of this would
> still put_page on the reader side under the lock. This bit would have
> to get tweaked to recover the loss in that scenario (not to be
> confused with the loss reported here).

We can revert the patch, but that would bring back the two page pools
(the on-stack prealloc and tmp_page[]), which is probably worse than
this regression itself (IIRC it would also lose the gains for smaller
writes (about 20% for 8-16 KiB writes IIRC ))

That said, these are the items I am planning to spend some time one:

1) Reclaiming the pooled pages from a shrinker, as suggested in [1]

2) Trimming the pool gradually on the reader side, instead of cutting it
   down to two pages in one go. Or not trimming it while a writer is
   in the waiting room (!?)

3) Keeping the writer's preallocated pages private again, while keeping
   the unified pool (i.e. without bringing tmp_page[] back).

4) Freeing the pages the reader drops after releasing pipe->mutex,
   instead of under it, as you suggested. This is independent of the
   options above.

5) Longer term: copying the data into the preallocated pages before
   taking pipe->mutex, so the writer only links them into the pipe
   under the lock. This doesn't seem trivial, but it would make this
   whole pre-allocated buffers go away

6) Any other idea?

[0] https://lore.kernel.org/all/202607091015.c3936260-lkp@intel.com/
[1] https://lore.kernel.org/all/CAGudoHGKyN6k+eSAUm35_WDSZcxxtm4QL6fO+4rYJTvtObMqzA@mail.gmail.com/

Thanks for looking at it,
--breno

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Mateusz Guzik]

On Fri, Nov 6, 2026 at 12:11 PM Breno Leitao <leitao@debian.org> wrote:
>
> On Thu, Nov 05, 2026 at 04:57:33PM +0100, Mateusz Guzik wrote:
> > On Thu, Nov 5, 2026 at 4:28 PM Breno Leitao <leitao@debian.org> wrote:
> >
> > The test at hand has multiple readers and writers using the same pipe,
> > with 64KB i/os.
> >
> > Prior to the patch each writer had its own private pool to draw from,
> > guaranteeing no allocation will happen under the mutex.
> >
> > With the patch the reader exit also drains the pool.
> >
> > As a result tons of allocation ends up happening under the lock again:
> >
> > -   94.88%    94.88%  bw_pipe          [kernel.kallsyms]     [k]
> > __alloc_frozen_pages_noprof
> >      94.88% __internal_syscall_cancel
> >         entry_SYSCALL_64_after_hwframe
> >         do_syscall_64
> >         ksys_write
> >         vfs_write
> >       - anon_pipe_write
> >          - 47.46% alloc_pages_noprof
> >               alloc_pages_mpol
> >               __alloc_frozen_pages_noprof
> >          - 47.42% anon_pipe_prefill_and_lock
> >               alloc_pages_noprof
> >               alloc_pages_mpol
> >               __alloc_frozen_pages_noprof
>
> Ack, this matches what I found as well. Thanks for investigating it.
>
> Because the preallocated pages are not private to the writer anymore,
> the reader flushes them while the writer waits for room in the pipe,
> and the writer then has to allocate them inside the lock.
>
> This basically loses the wins[0] from commit 212ed884a1aeef ("fs/pipe:
> pre-allocate pages outside pipe->mutex in anon_pipe_write") for this
> specific scenario.
>
> > I don't think multiple high-volume writers vs multiple high
> > volume-readers is a realistic scenario and I would not bother fixing
> > it.
>
> Agree, but I would like to spend some cycles on it.
>
> > However, should someone want to address it, I think the patch should
> > be reverted and the resulting state tweaked. The patch at hand came
> > after the mutex is released -- the kernel prior to any of this would
> > still put_page on the reader side under the lock. This bit would have
> > to get tweaked to recover the loss in that scenario (not to be
> > confused with the loss reported here).
>
> We can revert the patch, but that would bring back the two page pools
> (the on-stack prealloc and tmp_page[]), which is probably worse than
> this regression itself (IIRC it would also lose the gains for smaller
> writes (about 20% for 8-16 KiB writes IIRC ))
>

Like I said above, per my understanding the win is not specific to the
unified pool thing, but merely to the fact that the reader would put
pages *after* releasing the lock. this can be done in either approach.

> That said, these are the items I am planning to spend some time one:
>
> 1) Reclaiming the pooled pages from a shrinker, as suggested in [1]
>

The more I look at this the more the extra memory usage bothers me --
even with shrinker support there is just plain waste until it runs and
it is going to cost elsewhere. If this was "free" performance I would
have no comments.

Makes me wonder if there is any practical win to pursuing it in the first place.

But here is an idea just to ponder: what if the threads using the pipe
were guaranteed to trim it at some point, even if it remains open and
unused?

Currently there is the notion of task_work, which allows for arbitrary
callbacks to execute on the user<->kernel boundary. As implemented it
is rather expensive (atomics) to use and always flushes the entire
work queue.

But imagine the mechanism got extended such that there is a separate
queue where only current can add stuff (so it is cheap). Getting to
the code could be gated with the same branch which tells the kernel
about task_work, so no extra overhead in syscall handling as long as
you don't have anything queued up.

With this in place pipe code seeing a big buffer could add a callback,
which would do some magic to decide whether to trim the pool and by
what amount. Just for the sake of argument, here is an example: 1/16
chance it will do anything to begin with. if it chooses to do
something, it checks if there is a "trimmer was here" bit. If no, sets
the bit and leaves. If yes, shrinks the pool in half. The pipe write
code would clear the bit, so a pipe with continuous usage would mostly
remain intact and an unused pipe would eventually drain to nothing
(including the currently held 2 pages).

> 2) Trimming the pool gradually on the reader side, instead of cutting it
>    down to two pages in one go. Or not trimming it while a writer is
>    in the waiting room (!?)
>

There is no indicator there is a writer pending, at best you can check
if the lock is contested. This seems like gaming the benchmark to me
though, as a real program who happened to generate data a little later
would suddenly find itself with the pool trimmed.

Granular trimming definitely makes sense, provided shrinker support is
implemented.

> 3) Keeping the writer's preallocated pages private again, while keeping
>    the unified pool (i.e. without bringing tmp_page[] back).
>
> 4) Freeing the pages the reader drops after releasing pipe->mutex,
>    instead of under it, as you suggested. This is independent of the
>    options above.
>
> 5) Longer term: copying the data into the preallocated pages before
>    taking pipe->mutex, so the writer only links them into the pipe
>    under the lock. This doesn't seem trivial, but it would make this
>    whole pre-allocated buffers go away
>

I think this is sounds awfully complicated in face of multiple writers.

> 6) Any other idea?

The SMAP stuff is the primary problem right now. To reiterate: every
single buffer read/write takes a SMAP trip, which is very expensive
even on modern uarchs. Refactoring this into i/o under one SMAP round
would provide a visible speed up. It also happens to not pose a
tradeoff in terms of memory usage and would be my first avenue of
attack if I was working on this.

All that said, one reader vs one writer scenario definitely makes
sense to optimize.

Apart from that, highly parallel i/o on one pipe is imo the wrong
thing to look at in the first place. What is more likely to occur in
the real world is the user having separate processes operating on
distinct pipes, but this was not benchmarked at all. Another case to
look at is one reader vs one writer, but all the tuples have unique
uids.

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Breno Leitao]

On Fri, Nov 06, 2026 at 02:44:54PM +0100, Mateusz Guzik wrote:
> > 6) Any other idea?
> 
> The SMAP stuff is the primary problem right now. To reiterate: every
> single buffer read/write takes a SMAP trip, which is very expensive
> even on modern uarchs. Refactoring this into i/o under one SMAP round
> would provide a visible speed up. It also happens to not pose a
> tradeoff in terms of memory usage and would be my first avenue of
> attack if I was working on this.
> 
> All that said, one reader vs one writer scenario definitely makes
> sense to optimize.

I've spent some tokens on it, and it seems to give ~5% improvement on
the pipe bench. Is this what you had in mind (in a single patch format)?

commit 724fa7def2700d51a7f0427d0de06c123b3bf8e9
Author: Breno Leitao <leitao@debian.org>
Date:   Mon Nov 9 05:04:45 2026 -0800

    x86/uaccess, fs/pipe: copy a run of pipe buffers in one user access section
    
    Mateusz has been constantly suggesting that The SMAP stuff is the
    primary problem right now in the pipe code, which summarizes to
    anon_pipe_read() copying one pipe_buffer per copy_page_to_iter() call.
    
    On x86 every call opens and closes its own user access section, so a 64
    KiB read that drains a full default pipe executes 16 STAC/CLAC pairs.
    
    The pipe's pages are not contiguous, so the copies stay separate.
    
    Move the body of copy_user_generic() into unsafe_copy_user_generic(),
    which leaves STAC/CLAC to the caller's user access section, and turn
    copy_user_generic() into a STAC/CLAC wrapper around it. The existing
    callers get the same code. (This will eventually be a second patch if/once
    this becomes an non-PoC)
    
    When the destination is a plain user buffer (ITER_UBUF, which is what
    read(2) passes), copy up to PIPE_DEF_BUFFERS consecutive anonymous
    buffers that the read consumes whole inside one scoped user write access
    with unsafe_copy_user_generic(), then release them. The buffers are
    looked up before the section starts, so that only the copies run with
    user access enabled.
    
    What this buys depends on what SMAP costs the CPU. With the buffer by
    buffer loop, booting with clearcpuid=smap speeds pipe_bench up by 4-14%
    on a Sapphire Rapids Xeon Gold 6450C.
    
    pipe_bench from the pipe selftests runs 1, 2 or 5 writers against 1, 5
    or 10 readers on one 1 MiB pipe, with 64 KiB writes and reads.
    Throughput change from this patch, median of 2 boots x 5 runs, SMAP on:
    
                                   Sapphire Rapids
      1 writer, 1 reader                  +5.2%
      5 writers, 10 readers               +8.0%
      all 9 mixes                    +5% to +15%
    
    Suggested-by: Mateusz Guzik <mjguzik@gmail.com>
    Link: https://lore.kernel.org/all/CAGudoHF3de_55YvdztPs7QiYs25u6nPEV=uMOzfr=bWmwof69g@mail.gmail.com/
    Signed-off-by: Breno Leitao <leitao@debian.org>

diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index 20de34cc9aa6e7..28208db6132c27 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -115,10 +115,13 @@ static inline bool __access_ok(const void __user *ptr, unsigned long size)
 __must_check unsigned long
 rep_movs_alternative(void *to, const void *from, unsigned len);
 
+/*
+ * copy_user_generic() without the STAC/CLAC, for several copies inside one
+ * user access section. Returns the number of bytes not copied.
+ */
 static __always_inline __must_check unsigned long
-copy_user_generic(void *to, const void *from, unsigned long len)
+unsafe_copy_user_generic(void *to, const void *from, unsigned long len)
 {
-	stac();
 	/*
 	 * If CPU has FSRM feature, use 'rep movs'.
 	 * Otherwise, use rep_movs_alternative.
@@ -131,6 +134,16 @@ copy_user_generic(void *to, const void *from, unsigned long len)
 		_ASM_EXTABLE_UA(1b, 2b)
 		:"+c" (len), "+D" (to), "+S" (from), ASM_CALL_CONSTRAINT
 		: : "memory", "rax");
+	return len;
+}
+
+#define unsafe_copy_user_generic unsafe_copy_user_generic
+
+static __always_inline __must_check unsigned long
+copy_user_generic(void *to, const void *from, unsigned long len)
+{
+	stac();
+	len = unsafe_copy_user_generic(to, from, len);
 	clac();
 	return len;
 }

diff --git a/fs/pipe.c b/fs/pipe.c
index 4fa69192e6fba1..6f6041c94abefa 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -340,6 +340,115 @@ static inline unsigned int pipe_update_tail(struct pipe_inode_info *pipe,
 	return tail;
 }
 
+#ifdef unsafe_copy_user_generic
+/* One buffer of a batched read, looked up before the user access section. */
+struct pipe_copy_src {
+	void *kaddr;
+	size_t len;
+};
+
+/*
+ * Fill @run with the anonymous buffers from @tail on that the read consumes
+ * whole. Returns how many, with their total length in @lenp.
+ */
+static unsigned int anon_pipe_batch_lookup(struct pipe_inode_info *pipe,
+					   unsigned int head, unsigned int tail,
+					   size_t total_len,
+					   struct pipe_copy_src *run,
+					   size_t *lenp)
+{
+	struct pipe_buffer *buf;
+	size_t len = 0;
+	unsigned int n;
+
+	for (n = 0; n < PIPE_DEF_BUFFERS && tail + n != head; n++) {
+		buf = pipe_buf(pipe, tail + n);
+		if (buf->ops != &anon_pipe_buf_ops ||
+		    (buf->flags & PIPE_BUF_FLAG_PACKET) ||
+		    buf->len > total_len - len)
+			break;
+		run[n].kaddr = page_address(buf->page) + buf->offset;
+		run[n].len = buf->len;
+		len += buf->len;
+	}
+	*lenp = len;
+	return n;
+}
+
+/*
+ * Copy the @n buffers in @run, @len bytes in all, to @to inside one user
+ * access section. All or nothing: @to only advances if no copy faults.
+ */
+static bool anon_pipe_batch_copy(struct iov_iter *to,
+				 const struct pipe_copy_src *run,
+				 unsigned int n, size_t len)
+{
+	const struct pipe_copy_src *r, *end = run + n;
+	char __user *dst = to->ubuf + to->iov_offset;
+	size_t off;
+
+	if (should_fail_usercopy())
+		return false;
+	might_fault();
+	for (r = run, off = 0; r < end; off += r->len, r++)
+		instrument_copy_to_user(dst + off, r->kaddr, r->len);
+	scoped_user_write_access_size(dst, len, fault) {
+		for (r = run, off = 0; r < end; off += r->len, r++)
+			if (unsafe_copy_user_generic((__force void *)dst + off,
+						     r->kaddr, r->len))
+				goto fault;
+	}
+	iov_iter_advance(to, len);
+	return true;
+
+fault:
+	return false;
+}
+
+/* Release the @n buffers from @tail on, which a read consumed whole. */
+static unsigned int anon_pipe_batch_release(struct pipe_inode_info *pipe,
+					    unsigned int head,
+					    unsigned int tail, unsigned int n,
+					    bool *wake_writer)
+{
+	*wake_writer |= pipe_full(head, tail, pipe->max_usage);
+	while (n--)
+		tail = pipe_update_tail(pipe, pipe_buf(pipe, tail), tail);
+	return tail;
+}
+
+/*
+ * Copy a run of anonymous buffers that the read consumes whole to a plain
+ * user buffer inside one user access section, instead of one STAC/CLAC pair
+ * per buffer. Returns the bytes consumed, or 0 to go buffer by buffer.
+ */
+static ssize_t anon_pipe_read_batch(struct pipe_inode_info *pipe,
+				    struct iov_iter *to, size_t total_len,
+				    unsigned int head, unsigned int *tailp,
+				    bool *wake_writer)
+{
+	struct pipe_copy_src run[PIPE_DEF_BUFFERS];
+	unsigned int n;
+	size_t len;
+
+	if (!iter_is_ubuf(to))
+		return 0;
+	n = anon_pipe_batch_lookup(pipe, head, *tailp, total_len, run, &len);
+	if (n < 2 || !anon_pipe_batch_copy(to, run, n, len))
+		return 0;
+	*tailp = anon_pipe_batch_release(pipe, head, *tailp, n, wake_writer);
+	return len;
+}
+#else
+static ssize_t anon_pipe_read_batch(struct pipe_inode_info *pipe,
+				    struct iov_iter *to, size_t total_len,
+				    unsigned int head, unsigned int *tailp,
+				    bool *wake_writer)
+{
+	return 0;
+}
+#endif
+
 static ssize_t
 anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
 {
@@ -347,7 +456,7 @@ anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
 	struct file *filp = iocb->ki_filp;
 	struct pipe_inode_info *pipe = filp->private_data;
 	bool wake_writer = false, wake_next_reader = false;
-	ssize_t ret;
+	ssize_t ret, batched;
 
 	/* Null read succeeds. */
 	if (unlikely(total_len == 0))
@@ -393,7 +502,16 @@ anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
 		}
 #endif
 
-		if (!pipe_empty(head, tail)) {
+		batched = anon_pipe_read_batch(pipe, to, total_len, head,
+					       &tail, &wake_writer);
+		if (batched) {
+			ret += batched;
+			total_len -= batched;
+			if (!total_len)
+				break;	/* common path: read succeeded */
+			if (!pipe_empty(head, tail))	/* More to do? */
+				continue;
+		} else if (!pipe_empty(head, tail)) {
 			struct pipe_buffer *buf = pipe_buf(pipe, tail);
 			size_t chars = buf->len;
 			size_t written;

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Mateusz Guzik]

On Tue, Nov 10, 2026 at 1:46 PM Breno Leitao <leitao@debian.org> wrote:
>
> On Fri, Nov 06, 2026 at 02:44:54PM +0100, Mateusz Guzik wrote:
> > > 6) Any other idea?
> >
> > The SMAP stuff is the primary problem right now. To reiterate: every
> > single buffer read/write takes a SMAP trip, which is very expensive
> > even on modern uarchs. Refactoring this into i/o under one SMAP round
> > would provide a visible speed up. It also happens to not pose a
> > tradeoff in terms of memory usage and would be my first avenue of
> > attack if I was working on this.
> >
> > All that said, one reader vs one writer scenario definitely makes
> > sense to optimize.
>
> I've spent some tokens on it, and it seems to give ~5% improvement on
> the pipe bench. Is this what you had in mind (in a single patch format)?
>

The resulting behavior matches.

A clean implementation escapes me and that's part of a reason I did
not do the work myself.

The proposed patch is on the uglier side, but AFAICS that was never a
blocker in Linux.

Or to put it differently, I'm not happy with the shape of the patch,
but don't have a better suggestion. The win is probably enough of a
justification to get this in.

> commit 724fa7def2700d51a7f0427d0de06c123b3bf8e9
> Author: Breno Leitao <leitao@debian.org>
> Date:   Mon Nov 9 05:04:45 2026 -0800
>
>     x86/uaccess, fs/pipe: copy a run of pipe buffers in one user access section
>
>     Mateusz has been constantly suggesting that The SMAP stuff is the
>     primary problem right now in the pipe code, which summarizes to
>     anon_pipe_read() copying one pipe_buffer per copy_page_to_iter() call.
>
>     On x86 every call opens and closes its own user access section, so a 64
>     KiB read that drains a full default pipe executes 16 STAC/CLAC pairs.
>
>     The pipe's pages are not contiguous, so the copies stay separate.
>
>     Move the body of copy_user_generic() into unsafe_copy_user_generic(),
>     which leaves STAC/CLAC to the caller's user access section, and turn
>     copy_user_generic() into a STAC/CLAC wrapper around it. The existing
>     callers get the same code. (This will eventually be a second patch if/once
>     this becomes an non-PoC)
>
>     When the destination is a plain user buffer (ITER_UBUF, which is what
>     read(2) passes), copy up to PIPE_DEF_BUFFERS consecutive anonymous
>     buffers that the read consumes whole inside one scoped user write access
>     with unsafe_copy_user_generic(), then release them. The buffers are
>     looked up before the section starts, so that only the copies run with
>     user access enabled.
>
>     What this buys depends on what SMAP costs the CPU. With the buffer by
>     buffer loop, booting with clearcpuid=smap speeds pipe_bench up by 4-14%
>     on a Sapphire Rapids Xeon Gold 6450C.
>
>     pipe_bench from the pipe selftests runs 1, 2 or 5 writers against 1, 5
>     or 10 readers on one 1 MiB pipe, with 64 KiB writes and reads.
>     Throughput change from this patch, median of 2 boots x 5 runs, SMAP on:
>
>                                    Sapphire Rapids
>       1 writer, 1 reader                  +5.2%
>       5 writers, 10 readers               +8.0%
>       all 9 mixes                    +5% to +15%
>
>     Suggested-by: Mateusz Guzik <mjguzik@gmail.com>
>     Link: https://lore.kernel.org/all/CAGudoHF3de_55YvdztPs7QiYs25u6nPEV=uMOzfr=bWmwof69g@mail.gmail.com/
>     Signed-off-by: Breno Leitao <leitao@debian.org>
>
> diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
> index 20de34cc9aa6e7..28208db6132c27 100644
> --- a/arch/x86/include/asm/uaccess_64.h
> +++ b/arch/x86/include/asm/uaccess_64.h
> @@ -115,10 +115,13 @@ static inline bool __access_ok(const void __user *ptr, unsigned long size)
>  __must_check unsigned long
>  rep_movs_alternative(void *to, const void *from, unsigned len);
>
> +/*
> + * copy_user_generic() without the STAC/CLAC, for several copies inside one
> + * user access section. Returns the number of bytes not copied.
> + */
>  static __always_inline __must_check unsigned long
> -copy_user_generic(void *to, const void *from, unsigned long len)
> +unsafe_copy_user_generic(void *to, const void *from, unsigned long len)
>  {
> -       stac();
>         /*
>          * If CPU has FSRM feature, use 'rep movs'.
>          * Otherwise, use rep_movs_alternative.
> @@ -131,6 +134,16 @@ copy_user_generic(void *to, const void *from, unsigned long len)
>                 _ASM_EXTABLE_UA(1b, 2b)
>                 :"+c" (len), "+D" (to), "+S" (from), ASM_CALL_CONSTRAINT
>                 : : "memory", "rax");
> +       return len;
> +}
> +
> +#define unsafe_copy_user_generic unsafe_copy_user_generic
> +
> +static __always_inline __must_check unsigned long
> +copy_user_generic(void *to, const void *from, unsigned long len)
> +{
> +       stac();
> +       len = unsafe_copy_user_generic(to, from, len);
>         clac();
>         return len;
>  }
>
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 4fa69192e6fba1..6f6041c94abefa 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -340,6 +340,115 @@ static inline unsigned int pipe_update_tail(struct pipe_inode_info *pipe,
>         return tail;
>  }
>
> +#ifdef unsafe_copy_user_generic
> +/* One buffer of a batched read, looked up before the user access section. */
> +struct pipe_copy_src {
> +       void *kaddr;
> +       size_t len;
> +};
> +
> +/*
> + * Fill @run with the anonymous buffers from @tail on that the read consumes
> + * whole. Returns how many, with their total length in @lenp.
> + */
> +static unsigned int anon_pipe_batch_lookup(struct pipe_inode_info *pipe,
> +                                          unsigned int head, unsigned int tail,
> +                                          size_t total_len,
> +                                          struct pipe_copy_src *run,
> +                                          size_t *lenp)
> +{
> +       struct pipe_buffer *buf;
> +       size_t len = 0;
> +       unsigned int n;
> +
> +       for (n = 0; n < PIPE_DEF_BUFFERS && tail + n != head; n++) {
> +               buf = pipe_buf(pipe, tail + n);
> +               if (buf->ops != &anon_pipe_buf_ops ||
> +                   (buf->flags & PIPE_BUF_FLAG_PACKET) ||
> +                   buf->len > total_len - len)
> +                       break;
> +               run[n].kaddr = page_address(buf->page) + buf->offset;
> +               run[n].len = buf->len;
> +               len += buf->len;
> +       }
> +       *lenp = len;
> +       return n;
> +}
> +
> +/*
> + * Copy the @n buffers in @run, @len bytes in all, to @to inside one user
> + * access section. All or nothing: @to only advances if no copy faults.
> + */
> +static bool anon_pipe_batch_copy(struct iov_iter *to,
> +                                const struct pipe_copy_src *run,
> +                                unsigned int n, size_t len)
> +{
> +       const struct pipe_copy_src *r, *end = run + n;
> +       char __user *dst = to->ubuf + to->iov_offset;
> +       size_t off;
> +
> +       if (should_fail_usercopy())
> +               return false;
> +       might_fault();
> +       for (r = run, off = 0; r < end; off += r->len, r++)
> +               instrument_copy_to_user(dst + off, r->kaddr, r->len);
> +       scoped_user_write_access_size(dst, len, fault) {
> +               for (r = run, off = 0; r < end; off += r->len, r++)
> +                       if (unsafe_copy_user_generic((__force void *)dst + off,
> +                                                    r->kaddr, r->len))
> +                               goto fault;
> +       }
> +       iov_iter_advance(to, len);
> +       return true;
> +
> +fault:
> +       return false;
> +}
> +
> +/* Release the @n buffers from @tail on, which a read consumed whole. */
> +static unsigned int anon_pipe_batch_release(struct pipe_inode_info *pipe,
> +                                           unsigned int head,
> +                                           unsigned int tail, unsigned int n,
> +                                           bool *wake_writer)
> +{
> +       *wake_writer |= pipe_full(head, tail, pipe->max_usage);
> +       while (n--)
> +               tail = pipe_update_tail(pipe, pipe_buf(pipe, tail), tail);
> +       return tail;
> +}
> +
> +/*
> + * Copy a run of anonymous buffers that the read consumes whole to a plain
> + * user buffer inside one user access section, instead of one STAC/CLAC pair
> + * per buffer. Returns the bytes consumed, or 0 to go buffer by buffer.
> + */
> +static ssize_t anon_pipe_read_batch(struct pipe_inode_info *pipe,
> +                                   struct iov_iter *to, size_t total_len,
> +                                   unsigned int head, unsigned int *tailp,
> +                                   bool *wake_writer)
> +{
> +       struct pipe_copy_src run[PIPE_DEF_BUFFERS];
> +       unsigned int n;
> +       size_t len;
> +
> +       if (!iter_is_ubuf(to))
> +               return 0;
> +       n = anon_pipe_batch_lookup(pipe, head, *tailp, total_len, run, &len);
> +       if (n < 2 || !anon_pipe_batch_copy(to, run, n, len))
> +               return 0;
> +       *tailp = anon_pipe_batch_release(pipe, head, *tailp, n, wake_writer);
> +       return len;
> +}
> +#else
> +static ssize_t anon_pipe_read_batch(struct pipe_inode_info *pipe,
> +                                   struct iov_iter *to, size_t total_len,
> +                                   unsigned int head, unsigned int *tailp,
> +                                   bool *wake_writer)
> +{
> +       return 0;
> +}
> +#endif
> +
>  static ssize_t
>  anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
>  {
> @@ -347,7 +456,7 @@ anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
>         struct file *filp = iocb->ki_filp;
>         struct pipe_inode_info *pipe = filp->private_data;
>         bool wake_writer = false, wake_next_reader = false;
> -       ssize_t ret;
> +       ssize_t ret, batched;
>
>         /* Null read succeeds. */
>         if (unlikely(total_len == 0))
> @@ -393,7 +502,16 @@ anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
>                 }
>  #endif
>
> -               if (!pipe_empty(head, tail)) {
> +               batched = anon_pipe_read_batch(pipe, to, total_len, head,
> +                                              &tail, &wake_writer);
> +               if (batched) {
> +                       ret += batched;
> +                       total_len -= batched;
> +                       if (!total_len)
> +                               break;  /* common path: read succeeded */
> +                       if (!pipe_empty(head, tail))    /* More to do? */
> +                               continue;
> +               } else if (!pipe_empty(head, tail)) {
>                         struct pipe_buffer *buf = pipe_buf(pipe, tail);
>                         size_t chars = buf->len;
>                         size_t written;
>
>
>

Re: [linus:master] [fs/pipe] f7f4665dc5: lmbench3.PIPE.bandwidth.MB/sec 18.5% regression

[Breno Leitao]

On Tue, Nov 10, 2026 at 03:44:36PM +0100, Mateusz Guzik wrote:
> On Tue, Nov 10, 2026 at 1:46 PM Breno Leitao <leitao@debian.org> wrote:
> >
> > On Fri, Nov 06, 2026 at 02:44:54PM +0100, Mateusz Guzik wrote:
> > > > 6) Any other idea?
> > >
> > > The SMAP stuff is the primary problem right now. To reiterate: every
> > > single buffer read/write takes a SMAP trip, which is very expensive
> > > even on modern uarchs. Refactoring this into i/o under one SMAP round
> > > would provide a visible speed up. It also happens to not pose a
> > > tradeoff in terms of memory usage and would be my first avenue of
> > > attack if I was working on this.
> > >
> > > All that said, one reader vs one writer scenario definitely makes
> > > sense to optimize.
> >
> > I've spent some tokens on it, and it seems to give ~5% improvement on
> > the pipe bench. Is this what you had in mind (in a single patch format)?
> >
> 
> The resulting behavior matches.
> 
> A clean implementation escapes me and that's part of a reason I did
> not do the work myself.
> 
> The proposed patch is on the uglier side, but AFAICS that was never a
> blocker in Linux.

Agree, the implementation is not beatiful. Another approach is to open
code and have a single function to do the whole copying, and simplify it
a bit, but, that makes the code even harder to digest.

Let me try to come up with something a bit more digestible and see where
I will end up.

thanks,
--breno