* [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
@ 2026-07-07 15:04 Breno Leitao
2026-07-07 15:04 ` [PATCH v2 1/4] fs/pipe: move the prealloc pool to per-pipe infrastructure Breno Leitao
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Breno Leitao @ 2026-07-07 15:04 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, oleg, mjguzik, josh, Jan Kara,
jlayton
Cc: axboe, shakeel.butt, linux-fsdevel, linux-kernel, Breno Leitao,
kernel-team
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.
Summary:
=======
I've spent some time converging tmp_page[] and the on-stack
anon_pipe_prealloc pool of pages into a single per-pipe pool, as
discussed previously in a few places, most recently at:
https://lore.kernel.org/all/ajLA_zxsYyKISkwp@redhat.com/
Problem:
========
1) We have two types of page caches in the pipe mechanism today
* tmp_page[]
* anon_pipe_prealloc
2) they operate in different ways:
* tmp_page[] is protected by the pipe lock
* per-pipe, persistent, 2 pages
* anon_pipe_prealloc is an on-stack pool, not lock protected
* burst, up to 8 pages
Proposal/Design:
================
1) Keep the same page budget as today
a) up to two per-pipe persistent pages
b) burst of up to 8 pages
2) no pages are allocated unless necessary
* Pages are _ONLY_ allocated based on the length of the write,
minus the pages already available in the pool.
* No page is allocated but left unused
3) keep allocation and freeing outside of the lock
* only the assignment of pages stays lock-protected
* Currently, tmp_page[] pages are allocated in the lock, so
this patch will improve it (thus the performance numbers)
How:
====
1) replace tmp_page[] with anon_pipe_prealloc in pipe_inode_info
2) at write (anon_pipe_write), allocate the pages outside the lock in a helper
called anon_pipe_prefill()
a) the assignment into the pool must be lock protected
* anon_pipe_prefill() does it
b) anon_pipe_prefill() can populate up to PIPE_PREALLOC_MAX pages in the
pool
3) once anon_pipe_write is done, the pool is trimmed back to at most
PIPE_PREALLOC_KEEP (2) pages by anon_pipe_trim_pool()
Future:
=======
Once this lands, we could keep all allocated pages in the pool and rely
on a shrinker to trim it under memory pressure.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- User READ_ONCE to read prealloc.count
- Trim the pool at the reader side
- Link to v1: https://lore.kernel.org/r/20260626-b4-pipe-unification-v1-0-d23fa6b1ee27@debian.org
---
Breno Leitao (4):
fs/pipe: move the prealloc pool to per-pipe infrastructure
fs/pipe: add per-pipe pool push, prefill and trim helpers
fs/pipe: switch the read and write paths to the per-pipe pool
fs/pipe: remove the old on-stack prealloc helpers and tmp_page[2]
fs/pipe.c | 169 ++++++++++++++++++++--------------------------
include/linux/pipe_fs_i.h | 21 +++++-
2 files changed, 94 insertions(+), 96 deletions(-)
---
base-commit: 4e5dfb7c84012007c3c7061126491bbc92d71bf1
change-id: 20260625-b4-pipe-unification-aba7b8525de7
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/4] fs/pipe: move the prealloc pool to per-pipe infrastructure
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 ` Breno Leitao
2026-07-07 15:04 ` [PATCH v2 2/4] fs/pipe: add per-pipe pool push, prefill and trim helpers Breno Leitao
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Breno Leitao @ 2026-07-07 15:04 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, oleg, mjguzik, josh, Jan Kara,
jlayton
Cc: axboe, shakeel.butt, linux-fsdevel, linux-kernel, Breno Leitao,
kernel-team
Move struct anon_pipe_prealloc and PIPE_PREALLOC_MAX to pipe_fs_i.h and
embed the pool in each pipe via a new prealloc field, next to the existing
tmp_page[2] cache (which will be removed by the end of this patchset).
Add PIPE_PREALLOC_KEEP for the post-write trim target.
PIPE_PREALLOC_KEEP (2) is the number of pages that will be buffered in
the pipe (similar to struct page *tmp_page[2]), which will be removed
soon.
The on-stack prealloc pool used by anon_pipe_write() is unchanged; this
only adds the per-pipe storage that later patches switch the read/write
paths over to.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
fs/pipe.c | 7 -------
include/linux/pipe_fs_i.h | 19 +++++++++++++++++++
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/fs/pipe.c b/fs/pipe.c
index 429b0714ec575..325fd9757dbdd 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -111,13 +111,6 @@ void pipe_double_lock(struct pipe_inode_info *pipe1,
pipe_lock(pipe2);
}
-#define PIPE_PREALLOC_MAX 8
-
-struct anon_pipe_prealloc {
- struct page *pages[PIPE_PREALLOC_MAX];
- unsigned int count;
-};
-
/*
* Pre-allocate pages outside pipe->mutex for multi-page writes.
* alloc_page() with GFP_HIGHUSER can sleep in reclaim and runs memcg
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index a1eeed8006694..796860cbddf30 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -13,6 +13,9 @@
#define PIPE_BUF_FLAG_LOSS 0x40 /* Message loss happened after this buffer */
#endif
+#define PIPE_PREALLOC_MAX 8 /* max pages in prealloc pool */
+#define PIPE_PREALLOC_KEEP 2 /* keep at least this many after trim */
+
/**
* struct pipe_buffer - a linux kernel pipe buffer
* @page: the page containing the data for the pipe buffer
@@ -57,6 +60,20 @@ union pipe_index {
};
};
+/**
+ * struct anon_pipe_prealloc - per-pipe page preallocation pool
+ * @pages: array of cached pages (pool)
+ * @count: number of pages currently in the pool
+ *
+ * Each pipe keeps a small bounded pool of preallocated pages to reduce
+ * allocation overhead during writes. The pool is bounded at PIPE_PREALLOC_MAX
+ * and trimmed down to PIPE_PREALLOC_KEEP after a write completes.
+ */
+struct anon_pipe_prealloc {
+ struct page *pages[PIPE_PREALLOC_MAX];
+ unsigned int count;
+};
+
/**
* struct pipe_inode_info - a linux kernel pipe
* @mutex: mutex protecting the whole thing
@@ -68,6 +85,7 @@ union pipe_index {
* @ring_size: total number of buffers (should be a power of 2)
* @nr_accounted: The amount this pipe accounts for in user->pipe_bufs
* @tmp_page: cached released page
+ * @prealloc: per-pipe page preallocation pool
* @readers: number of current readers of this pipe
* @writers: number of current writers of this pipe
* @files: number of struct file referring this pipe (protected by ->i_lock)
@@ -99,6 +117,7 @@ struct pipe_inode_info {
bool note_loss;
#endif
struct page *tmp_page[2];
+ struct anon_pipe_prealloc prealloc;
struct fasync_struct *fasync_readers;
struct fasync_struct *fasync_writers;
struct pipe_buffer *bufs;
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] fs/pipe: add per-pipe pool push, prefill and trim helpers
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 ` 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
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Breno Leitao @ 2026-07-07 15:04 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, oleg, mjguzik, josh, Jan Kara,
jlayton
Cc: axboe, shakeel.butt, linux-fsdevel, linux-kernel, Breno Leitao,
kernel-team
Add the helpers the per-pipe pool needs: anon_pipe_prealloc_push() to
return a page to the pool, anon_pipe_prefill() to top the pipe's pool up
before the lock, and anon_pipe_trim_pool() to drop it back to
PIPE_PREALLOC_KEEP after a write. anon_pipe_prealloc_pop() already exists
and is reused.
prefill and trim_pool have no callers yet and are marked __maybe_unused;
the next patch wires them into anon_pipe_write() and removes the
annotation along with the old on-stack pool helpers.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
fs/pipe.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/fs/pipe.c b/fs/pipe.c
index 325fd9757dbdd..240fede27542d 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -155,6 +155,78 @@ static struct page *anon_pipe_prealloc_pop(struct anon_pipe_prealloc *prealloc)
return prealloc->pages[prealloc->count];
}
+/* Push a page to the prealloc pool. Returns true if added, false if full. */
+static bool anon_pipe_prealloc_push(struct anon_pipe_prealloc *prealloc,
+ struct page *page)
+{
+ if (prealloc->count >= PIPE_PREALLOC_MAX)
+ return false;
+ prealloc->pages[prealloc->count++] = page;
+ return true;
+}
+
+/*
+ * Top up the pipe's own pool before taking pipe->mutex, allocating only the
+ * shortfall outside the lock, then briefly take the lock to push the pages in.
+ * anon_pipe_get_page() then drains the pool instead of allocating under the lock.
+ */
+static void __maybe_unused anon_pipe_prefill(struct pipe_inode_info *pipe,
+ size_t total_len)
+{
+ struct page *pages[PIPE_PREALLOC_MAX];
+ unsigned int want, have, need, n = 0;
+
+ want = min_t(unsigned int, DIV_ROUND_UP(total_len, PAGE_SIZE),
+ PIPE_PREALLOC_MAX);
+ /* Unlocked read; the pool is refilled under the lock below. */
+ have = min_t(unsigned int, READ_ONCE(pipe->prealloc.count), want);
+ need = want - have;
+
+ if (!need)
+ return;
+
+ while (n < need) {
+ struct page *page = alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
+
+ if (!page)
+ break;
+ pages[n++] = page;
+ }
+ if (!n)
+ return;
+
+ mutex_lock(&pipe->mutex);
+ while (n && anon_pipe_prealloc_push(&pipe->prealloc, pages[n - 1]))
+ n--;
+ mutex_unlock(&pipe->mutex);
+
+ /*
+ * Just flush any extra page that got affected by the TOCTOU
+ * effect
+ */
+ while (n)
+ put_page(pages[--n]);
+}
+
+/* Trim the pool down to PIPE_PREALLOC_KEEP, freeing the excess unlocked. */
+static void __maybe_unused anon_pipe_trim_pool(struct pipe_inode_info *pipe)
+{
+ struct page *excess[PIPE_PREALLOC_MAX];
+ unsigned int nexcess = 0;
+
+ /* Unlocked fast path; the count is re-checked under the lock below. */
+ if (READ_ONCE(pipe->prealloc.count) <= PIPE_PREALLOC_KEEP)
+ return;
+
+ mutex_lock(&pipe->mutex);
+ while (pipe->prealloc.count > PIPE_PREALLOC_KEEP)
+ excess[nexcess++] = anon_pipe_prealloc_pop(&pipe->prealloc);
+ mutex_unlock(&pipe->mutex);
+
+ while (nexcess)
+ put_page(excess[--nexcess]);
+}
+
static struct page *anon_pipe_get_page(struct pipe_inode_info *pipe,
struct anon_pipe_prealloc *prealloc)
{
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] fs/pipe: switch the read and write paths to the per-pipe pool
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 ` 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
4 siblings, 0 replies; 11+ messages in thread
From: Breno Leitao @ 2026-07-07 15:05 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, oleg, mjguzik, josh, Jan Kara,
jlayton
Cc: axboe, shakeel.butt, linux-fsdevel, linux-kernel, Breno Leitao,
kernel-team
Replace the per-write on-stack prealloc pool with the pipe's persistent
pool: anon_pipe_write() now tops up pipe->prealloc before the lock via
anon_pipe_prefill() and trims it after the write via anon_pipe_trim_pool(),
and anon_pipe_get_page()/anon_pipe_put_page() drain and refill that pool
directly. Free the pool, instead of tmp_page[2], on teardown.
This leaves the old on-stack helpers (anon_pipe_get_page_prealloc,
anon_pipe_refill_tmp_pages, anon_pipe_free_pages) and tmp_page[2] without
callers; they are marked __maybe_unused here and removed in the next
patch (aiming to make the patch more reviewable)
Signed-off-by: Breno Leitao <leitao@debian.org>
---
fs/pipe.c | 58 ++++++++++++++++++++--------------------------------------
1 file changed, 20 insertions(+), 38 deletions(-)
diff --git a/fs/pipe.c b/fs/pipe.c
index 240fede27542d..3288a16f8a40a 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -124,8 +124,8 @@ void pipe_double_lock(struct pipe_inode_info *pipe1,
* 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)
+static void __maybe_unused anon_pipe_get_page_prealloc(struct anon_pipe_prealloc *prealloc,
+ size_t total_len)
{
unsigned int want, i;
struct page *page;
@@ -170,8 +170,7 @@ static bool anon_pipe_prealloc_push(struct anon_pipe_prealloc *prealloc,
* shortfall outside the lock, then briefly take the lock to push the pages in.
* anon_pipe_get_page() then drains the pool instead of allocating under the lock.
*/
-static void __maybe_unused anon_pipe_prefill(struct pipe_inode_info *pipe,
- size_t total_len)
+static void anon_pipe_prefill(struct pipe_inode_info *pipe, size_t total_len)
{
struct page *pages[PIPE_PREALLOC_MAX];
unsigned int want, have, need, n = 0;
@@ -209,7 +208,7 @@ static void __maybe_unused anon_pipe_prefill(struct pipe_inode_info *pipe,
}
/* Trim the pool down to PIPE_PREALLOC_KEEP, freeing the excess unlocked. */
-static void __maybe_unused anon_pipe_trim_pool(struct pipe_inode_info *pipe)
+static void anon_pipe_trim_pool(struct pipe_inode_info *pipe)
{
struct page *excess[PIPE_PREALLOC_MAX];
unsigned int nexcess = 0;
@@ -227,24 +226,15 @@ static void __maybe_unused anon_pipe_trim_pool(struct pipe_inode_info *pipe)
put_page(excess[--nexcess]);
}
-static struct page *anon_pipe_get_page(struct pipe_inode_info *pipe,
- struct anon_pipe_prealloc *prealloc)
+static struct page *anon_pipe_get_page(struct pipe_inode_info *pipe)
{
struct page *page;
- /* Drain prealloc first to keep tmp_page[] hot for later small writes. */
- page = anon_pipe_prealloc_pop(prealloc);
+ /* Drain the prealloc pool before allocating. Called with mutex held. */
+ page = anon_pipe_prealloc_pop(&pipe->prealloc);
if (page)
return page;
- for (int i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
- if (pipe->tmp_page[i]) {
- page = pipe->tmp_page[i];
- pipe->tmp_page[i] = NULL;
- return page;
- }
- }
-
/* FWIW: This is called with pipe->mutex held */
return alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
}
@@ -252,14 +242,9 @@ static struct page *anon_pipe_get_page(struct pipe_inode_info *pipe,
static void anon_pipe_put_page(struct pipe_inode_info *pipe,
struct page *page)
{
- if (page_count(page) == 1) {
- for (int i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
- if (!pipe->tmp_page[i]) {
- pipe->tmp_page[i] = page;
- return;
- }
- }
- }
+ if (page_count(page) == 1 &&
+ anon_pipe_prealloc_push(&pipe->prealloc, page))
+ return;
put_page(page);
}
@@ -268,8 +253,8 @@ static void anon_pipe_put_page(struct pipe_inode_info *pipe,
* Stash leftover prealloc pages in tmp_page[] so the next write to this
* pipe gets a hot page without entering the allocator.
*/
-static void anon_pipe_refill_tmp_pages(struct pipe_inode_info *pipe,
- struct anon_pipe_prealloc *prealloc)
+static void __maybe_unused anon_pipe_refill_tmp_pages(struct pipe_inode_info *pipe,
+ struct anon_pipe_prealloc *prealloc)
{
int i, idx;
@@ -288,7 +273,7 @@ static void anon_pipe_refill_tmp_pages(struct pipe_inode_info *pipe,
}
/* Runs after mutex_unlock() to keep put_page() out of the critical section. */
-static void anon_pipe_free_pages(struct anon_pipe_prealloc *prealloc)
+static void __maybe_unused anon_pipe_free_pages(struct anon_pipe_prealloc *prealloc)
{
while (prealloc->count) {
prealloc->count--;
@@ -551,6 +536,8 @@ anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
if (pipe_is_empty(pipe))
wake_next_reader = false;
mutex_unlock(&pipe->mutex);
+ /* Consumed buffers may have refilled the pool; trim it back. */
+ anon_pipe_trim_pool(pipe);
if (wake_writer)
wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM);
@@ -589,7 +576,6 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
{
struct file *filp = iocb->ki_filp;
struct pipe_inode_info *pipe = filp->private_data;
- struct anon_pipe_prealloc prealloc;
unsigned int head;
ssize_t ret = 0;
size_t total_len = iov_iter_count(from);
@@ -613,8 +599,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
if (unlikely(total_len == 0))
return 0;
- anon_pipe_get_page_prealloc(&prealloc, total_len);
-
+ anon_pipe_prefill(pipe, total_len);
mutex_lock(&pipe->mutex);
if (!pipe->readers) {
@@ -672,7 +657,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
struct page *page;
int copied;
- page = anon_pipe_get_page(pipe, &prealloc);
+ page = anon_pipe_get_page(pipe);
if (unlikely(!page)) {
if (!ret)
ret = -ENOMEM;
@@ -736,11 +721,10 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
wake_next_writer = true;
}
out:
- anon_pipe_refill_tmp_pages(pipe, &prealloc);
if (pipe_is_full(pipe))
wake_next_writer = false;
mutex_unlock(&pipe->mutex);
- anon_pipe_free_pages(&prealloc);
+ anon_pipe_trim_pool(pipe);
/*
* If we do do a wakeup event, we do a 'sync' wakeup, because we
@@ -1021,10 +1005,8 @@ void free_pipe_info(struct pipe_inode_info *pipe)
if (pipe->watch_queue)
put_watch_queue(pipe->watch_queue);
#endif
- for (i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
- if (pipe->tmp_page[i])
- __free_page(pipe->tmp_page[i]);
- }
+ for (i = 0; i < pipe->prealloc.count; i++)
+ __free_page(pipe->prealloc.pages[i]);
kfree(pipe->bufs);
kfree(pipe);
}
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] fs/pipe: remove the old on-stack prealloc helpers and tmp_page[2]
2026-07-07 15:04 [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool Breno Leitao
` (2 preceding siblings ...)
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 ` 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
4 siblings, 0 replies; 11+ messages in thread
From: Breno Leitao @ 2026-07-07 15:05 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, oleg, mjguzik, josh, Jan Kara,
jlayton
Cc: axboe, shakeel.butt, linux-fsdevel, linux-kernel, Breno Leitao,
kernel-team
With the write path converted to the per-pipe pool, the old on-stack
prealloc helpers (anon_pipe_get_page_prealloc, anon_pipe_refill_tmp_pages,
anon_pipe_free_pages) and the tmp_page[2] cache have no remaining users.
Remove them.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
fs/pipe.c | 66 -----------------------------------------------
include/linux/pipe_fs_i.h | 2 --
2 files changed, 68 deletions(-)
diff --git a/fs/pipe.c b/fs/pipe.c
index 3288a16f8a40a..c163b05ef9708 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -111,40 +111,6 @@ void pipe_double_lock(struct pipe_inode_info *pipe1,
pipe_lock(pipe2);
}
-/*
- * 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 __maybe_unused 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;
- }
-}
-
static struct page *anon_pipe_prealloc_pop(struct anon_pipe_prealloc *prealloc)
{
if (!prealloc->count)
@@ -249,38 +215,6 @@ static void anon_pipe_put_page(struct pipe_inode_info *pipe,
put_page(page);
}
-/*
- * Stash leftover prealloc pages in tmp_page[] so the next write to this
- * pipe gets a hot page without entering the allocator.
- */
-static void __maybe_unused anon_pipe_refill_tmp_pages(struct pipe_inode_info *pipe,
- struct anon_pipe_prealloc *prealloc)
-{
- int i, idx;
-
- if (!prealloc->count)
- return;
-
- for (i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
- if (pipe->tmp_page[i])
- continue;
- if (!prealloc->count)
- return;
- idx = --prealloc->count;
- pipe->tmp_page[i] = prealloc->pages[idx];
- prealloc->pages[idx] = NULL;
- }
-}
-
-/* Runs after mutex_unlock() to keep put_page() out of the critical section. */
-static void __maybe_unused anon_pipe_free_pages(struct anon_pipe_prealloc *prealloc)
-{
- while (prealloc->count) {
- prealloc->count--;
- put_page(prealloc->pages[prealloc->count]);
- }
-}
-
static void anon_pipe_buf_release(struct pipe_inode_info *pipe,
struct pipe_buffer *buf)
{
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 796860cbddf30..6bd0d956691cb 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -84,7 +84,6 @@ struct anon_pipe_prealloc {
* @max_usage: The maximum number of slots that may be used in the ring
* @ring_size: total number of buffers (should be a power of 2)
* @nr_accounted: The amount this pipe accounts for in user->pipe_bufs
- * @tmp_page: cached released page
* @prealloc: per-pipe page preallocation pool
* @readers: number of current readers of this pipe
* @writers: number of current writers of this pipe
@@ -116,7 +115,6 @@ struct pipe_inode_info {
#ifdef CONFIG_WATCH_QUEUE
bool note_loss;
#endif
- struct page *tmp_page[2];
struct anon_pipe_prealloc prealloc;
struct fasync_struct *fasync_readers;
struct fasync_struct *fasync_writers;
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
2026-07-07 15:04 [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool Breno Leitao
` (3 preceding siblings ...)
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 ` Mateusz Guzik
2026-07-08 12:09 ` Breno Leitao
4 siblings, 1 reply; 11+ messages in thread
From: Mateusz Guzik @ 2026-07-07 15:29 UTC (permalink / raw)
To: Breno Leitao
Cc: Alexander Viro, Christian Brauner, oleg, josh, Jan Kara, jlayton,
axboe, shakeel.butt, linux-fsdevel, linux-kernel, kernel-team
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.
I have a trivial benchmark for pipe throughput, usable with
will-it-scale (e.g., plop into tests/pipen.c):
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <errno.h>
#include <assert.h>
#define READ 0
#define WRITE 1
char *testcase_description = "pipe read/write";
int fd[2];
void testcase_prepare(unsigned long nr_tasks)
{
if (nr_tasks < 2)
err(1, "bad worker count");
assert(pipe(fd) == 0);
}
void testcase(unsigned long long *iterations, unsigned long nr)
{
char buf[1024 * 128] __attribute__((aligned(64)));
int ret;
int size = atoi(getenv("PIPEN_SIZE"));
if (nr == 0) {
for (;;) {
ret = write(fd[WRITE], buf, size);
if (ret >= 0)
(*iterations) += ret;
else
break;
}
} else {
do {
ret = read(fd[READ], buf, size);
} while (ret >= 0);
}
err(1, "bailing ret %d");
}
Then for example: PIPEN_SIZE=32678 ./pipen_processes -t 2
This gives one writer and one reader, both on separate CPUs.
With this size of write I'm seeing a 7% drop in throughput with the
patchset when benchmarking on Sapphire Rapids.
> Summary:
> =======
>
> I've spent some time converging tmp_page[] and the on-stack
> anon_pipe_prealloc pool of pages into a single per-pipe pool, as
> discussed previously in a few places, most recently at:
>
> https://lore.kernel.org/all/ajLA_zxsYyKISkwp@redhat.com/
>
> Problem:
> ========
>
> 1) We have two types of page caches in the pipe mechanism today
> * tmp_page[]
> * anon_pipe_prealloc
>
> 2) they operate in different ways:
> * tmp_page[] is protected by the pipe lock
> * per-pipe, persistent, 2 pages
> * anon_pipe_prealloc is an on-stack pool, not lock protected
> * burst, up to 8 pages
>
> Proposal/Design:
> ================
>
> 1) Keep the same page budget as today
> a) up to two per-pipe persistent pages
> b) burst of up to 8 pages
>
> 2) no pages are allocated unless necessary
> * Pages are _ONLY_ allocated based on the length of the write,
> minus the pages already available in the pool.
> * No page is allocated but left unused
>
> 3) keep allocation and freeing outside of the lock
> * only the assignment of pages stays lock-protected
> * Currently, tmp_page[] pages are allocated in the lock, so
> this patch will improve it (thus the performance numbers)
>
> How:
> ====
>
> 1) replace tmp_page[] with anon_pipe_prealloc in pipe_inode_info
> 2) at write (anon_pipe_write), allocate the pages outside the lock in a helper
> called anon_pipe_prefill()
> a) the assignment into the pool must be lock protected
> * anon_pipe_prefill() does it
> b) anon_pipe_prefill() can populate up to PIPE_PREALLOC_MAX pages in the
> pool
> 3) once anon_pipe_write is done, the pool is trimmed back to at most
> PIPE_PREALLOC_KEEP (2) pages by anon_pipe_trim_pool()
>
> Future:
> =======
>
> Once this lands, we could keep all allocated pages in the pool and rely
> on a shrinker to trim it under memory pressure.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v2:
> - User READ_ONCE to read prealloc.count
> - Trim the pool at the reader side
> - Link to v1: https://lore.kernel.org/r/20260626-b4-pipe-unification-v1-0-d23fa6b1ee27@debian.org
>
> ---
> Breno Leitao (4):
> fs/pipe: move the prealloc pool to per-pipe infrastructure
> fs/pipe: add per-pipe pool push, prefill and trim helpers
> fs/pipe: switch the read and write paths to the per-pipe pool
> fs/pipe: remove the old on-stack prealloc helpers and tmp_page[2]
>
> fs/pipe.c | 169 ++++++++++++++++++++--------------------------
> include/linux/pipe_fs_i.h | 21 +++++-
> 2 files changed, 94 insertions(+), 96 deletions(-)
> ---
> base-commit: 4e5dfb7c84012007c3c7061126491bbc92d71bf1
> change-id: 20260625-b4-pipe-unification-aba7b8525de7
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
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
0 siblings, 1 reply; 11+ messages in thread
From: Breno Leitao @ 2026-07-08 12:09 UTC (permalink / raw)
To: Mateusz Guzik
Cc: Alexander Viro, Christian Brauner, oleg, josh, Jan Kara, jlayton,
axboe, shakeel.butt, linux-fsdevel, linux-kernel, kernel-team
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)
Both tests with:
Kernel: linux-next 20260623 base, production .config
(no KASAN / LOCKDEP / DEBUG_* / nothing useless), baseline vs the
full series, both built with clang, coexisting in grub
Test: your will-it-scale test (pipen.c), PIPEN_SIZE=32678, -t 2
(1 writer + 1 reader on separate cores);
performance governor, writer and reader pinned to two cores
You're right that there is an extra lock/unlock in the prefill path, so
I don't want to wave your result away -- may it be specific to SPR's
topology/cache?
I'll also keep trying to grab a Sapphire Rapids machine so I can run your
will-it-scale case directly on the same uarch, and check if I can reproduce
it..
Thanks,
--breno
~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
2026-07-08 12:09 ` Breno Leitao
@ 2026-07-08 13:18 ` Mateusz Guzik
2026-07-08 15:11 ` Breno Leitao
0 siblings, 1 reply; 11+ messages in thread
From: Mateusz Guzik @ 2026-07-08 13:18 UTC (permalink / raw)
To: Breno Leitao
Cc: Alexander Viro, Christian Brauner, oleg, josh, Jan Kara, jlayton,
axboe, shakeel.butt, linux-fsdevel, linux-kernel, kernel-team
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)
>
>
> Both tests with:
> Kernel: linux-next 20260623 base, production .config
> (no KASAN / LOCKDEP / DEBUG_* / nothing useless), baseline vs the
> full series, both built with clang, coexisting in grub
> Test: your will-it-scale test (pipen.c), PIPEN_SIZE=32678, -t 2
> (1 writer + 1 reader on separate cores);
> performance governor, writer and reader pinned to two cores
>
>
> You're right that there is an extra lock/unlock in the prefill path, so
> I don't want to wave your result away -- may it be specific to SPR's
> topology/cache?
>
> I'll also keep trying to grab a Sapphire Rapids machine so I can run your
> will-it-scale case directly on the same uarch, and check if I can reproduce
> it..
>
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).
diff --git a/fs/pipe.c b/fs/pipe.c
index c163b05ef970..cc4be58aeb5f 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -136,7 +136,7 @@ static bool anon_pipe_prealloc_push(struct anon_pipe_prealloc *prealloc,
* shortfall outside the lock, then briefly take the lock to push the pages in.
* anon_pipe_get_page() then drains the pool instead of allocating under the lock.
*/
-static void anon_pipe_prefill(struct pipe_inode_info *pipe, size_t total_len)
+static void anon_pipe_prefill_and_lock(struct pipe_inode_info *pipe, size_t total_len)
{
struct page *pages[PIPE_PREALLOC_MAX];
unsigned int want, have, need, n = 0;
@@ -147,8 +147,10 @@ static void anon_pipe_prefill(struct pipe_inode_info *pipe, size_t total_len)
have = min_t(unsigned int, READ_ONCE(pipe->prealloc.count), want);
need = want - have;
- if (!need)
+ if (!need) {
+ mutex_lock(&pipe->mutex);
return;
+ }
while (n < need) {
struct page *page = alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
@@ -157,14 +159,10 @@ static void anon_pipe_prefill(struct pipe_inode_info *pipe, size_t total_len)
break;
pages[n++] = page;
}
- if (!n)
- return;
mutex_lock(&pipe->mutex);
while (n && anon_pipe_prealloc_push(&pipe->prealloc, pages[n - 1]))
n--;
- mutex_unlock(&pipe->mutex);
-
/*
* Just flush any extra page that got affected by the TOCTOU
* effect
@@ -174,14 +172,16 @@ static void anon_pipe_prefill(struct pipe_inode_info *pipe, size_t total_len)
}
/* Trim the pool down to PIPE_PREALLOC_KEEP, freeing the excess unlocked. */
-static void anon_pipe_trim_pool(struct pipe_inode_info *pipe)
+static void anon_pipe_trim_pool_and_unlock(struct pipe_inode_info *pipe)
{
struct page *excess[PIPE_PREALLOC_MAX];
unsigned int nexcess = 0;
/* Unlocked fast path; the count is re-checked under the lock below. */
- if (READ_ONCE(pipe->prealloc.count) <= PIPE_PREALLOC_KEEP)
+ if (READ_ONCE(pipe->prealloc.count) <= PIPE_PREALLOC_KEEP) {
+ mutex_unlock(&pipe->mutex);
return;
+ }
mutex_lock(&pipe->mutex);
while (pipe->prealloc.count > PIPE_PREALLOC_KEEP)
@@ -469,9 +469,7 @@ anon_pipe_read(struct kiocb *iocb, struct iov_iter *to)
}
if (pipe_is_empty(pipe))
wake_next_reader = false;
- mutex_unlock(&pipe->mutex);
- /* Consumed buffers may have refilled the pool; trim it back. */
- anon_pipe_trim_pool(pipe);
+ anon_pipe_trim_pool_and_unlock(pipe);
if (wake_writer)
wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM);
@@ -533,8 +531,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
if (unlikely(total_len == 0))
return 0;
- anon_pipe_prefill(pipe, total_len);
- mutex_lock(&pipe->mutex);
+ anon_pipe_prefill_and_lock(pipe, total_len);
if (!pipe->readers) {
if ((iocb->ki_flags & IOCB_NOSIGNAL) == 0)
@@ -657,8 +654,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
out:
if (pipe_is_full(pipe))
wake_next_writer = false;
- mutex_unlock(&pipe->mutex);
- anon_pipe_trim_pool(pipe);
+ anon_pipe_trim_pool_and_unlock(pipe);
/*
* If we do do a wakeup event, we do a 'sync' wakeup, because we
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
2026-07-08 13:18 ` Mateusz Guzik
@ 2026-07-08 15:11 ` Breno Leitao
2026-07-08 15:24 ` Mateusz Guzik
0 siblings, 1 reply; 11+ messages in thread
From: Breno Leitao @ 2026-07-08 15:11 UTC (permalink / raw)
To: Mateusz Guzik
Cc: Alexander Viro, Christian Brauner, oleg, josh, Jan Kara, jlayton,
axboe, shakeel.butt, linux-fsdevel, linux-kernel, kernel-team
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
2026-07-08 15:11 ` Breno Leitao
@ 2026-07-08 15:24 ` Mateusz Guzik
2026-07-08 15:54 ` Breno Leitao
0 siblings, 1 reply; 11+ messages in thread
From: Mateusz Guzik @ 2026-07-08 15:24 UTC (permalink / raw)
To: Breno Leitao
Cc: Alexander Viro, Christian Brauner, oleg, josh, Jan Kara, jlayton,
axboe, shakeel.butt, linux-fsdevel, linux-kernel, kernel-team
On Wed, Jul 8, 2026 at 5:11 PM Breno Leitao <leitao@debian.org> wrote:
> 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?
imo this is too trivial compared to the entirety of the patchset to
warrant attribution. Just take it.
To be clear, I just now verified avoidance of the extra trip resolved
the regression for me so I have no objections to the patchset.
For completeness I note I failed to remove the lock acquire in
anon_pipe_trim_pool_and_unlock, but I presume you noticed (at worst
while testing).
One suggestion I have is to post the entire thing as one patch as I
don't think the split here buys anything, but I'm not going to insist
one way or the other.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] fs/pipe: unify the page pools into a single per-pipe pool
2026-07-08 15:24 ` Mateusz Guzik
@ 2026-07-08 15:54 ` Breno Leitao
0 siblings, 0 replies; 11+ messages in thread
From: Breno Leitao @ 2026-07-08 15:54 UTC (permalink / raw)
To: Mateusz Guzik
Cc: Alexander Viro, Christian Brauner, oleg, josh, Jan Kara, jlayton,
axboe, shakeel.butt, linux-fsdevel, linux-kernel, kernel-team
On Wed, Jul 08, 2026 at 05:24:47PM +0200, Mateusz Guzik wrote:
> On Wed, Jul 8, 2026 at 5:11 PM Breno Leitao <leitao@debian.org> wrote:
> > 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?
>
> imo this is too trivial compared to the entirety of the patchset to
> warrant attribution. Just take it.
>
> To be clear, I just now verified avoidance of the extra trip resolved
> the regression for me so I have no objections to the patchset.
>
> For completeness I note I failed to remove the lock acquire in
> anon_pipe_trim_pool_and_unlock, but I presume you noticed (at worst
> while testing).
yes, otherwise it was self deadlocking it self in AA scenario.
> One suggestion I have is to post the entire thing as one patch as I
> don't think the split here buys anything, but I'm not going to insist
> one way or the other.
Ack, I splitted the changes hoping it would make the review easier.
So, I will respin this with your suggestion in a single patch, after
some bake time (probably tomorrow).
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-08 15:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-07-08 15:24 ` Mateusz Guzik
2026-07-08 15:54 ` Breno Leitao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox