Skip to content

Commit 3d5b9f3

Browse files
committed
Merge patch series "some pipe + wait stuff"
Mateusz Guzik <mjguzik@gmail.com> says: As a side effect of looking at the pipe hang I came up with 3 changes to consider for -next. The first one is a trivial clean up which I wont mind if it merely gets folded into someone else's change for pipes. The second one reduces page alloc/free calls for the backing area (60% less during a kernel build in my testing). I already posted this, but the cc list was not proper. The last one concerns the wait/wakeup mechanism and drops one lock trip in the common case after waking up. * patches from https://lore.kernel.org/r/20250303230409.452687-1-mjguzik@gmail.com: wait: avoid spurious calls to prepare_to_wait_event() in ___wait_event() pipe: cache 2 pages instead of 1 pipe: drop an always true check in anon_pipe_write() Link: https://lore.kernel.org/r/20250303230409.452687-1-mjguzik@gmail.com Signed-off-by: Christian Brauner <brauner@kernel.org>
2 parents ee5eda8 + 84654c7 commit 3d5b9f3

3 files changed

Lines changed: 45 additions & 23 deletions

File tree

fs/pipe.c

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -112,20 +112,40 @@ void pipe_double_lock(struct pipe_inode_info *pipe1,
112112
pipe_lock(pipe2);
113113
}
114114

115+
static struct page *anon_pipe_get_page(struct pipe_inode_info *pipe)
116+
{
117+
for (int i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
118+
if (pipe->tmp_page[i]) {
119+
struct page *page = pipe->tmp_page[i];
120+
pipe->tmp_page[i] = NULL;
121+
return page;
122+
}
123+
}
124+
125+
return alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
126+
}
127+
128+
static void anon_pipe_put_page(struct pipe_inode_info *pipe,
129+
struct page *page)
130+
{
131+
if (page_count(page) == 1) {
132+
for (int i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
133+
if (!pipe->tmp_page[i]) {
134+
pipe->tmp_page[i] = page;
135+
return;
136+
}
137+
}
138+
}
139+
140+
put_page(page);
141+
}
142+
115143
static void anon_pipe_buf_release(struct pipe_inode_info *pipe,
116144
struct pipe_buffer *buf)
117145
{
118146
struct page *page = buf->page;
119147

120-
/*
121-
* If nobody else uses this page, and we don't already have a
122-
* temporary page, let's keep track of it as a one-deep
123-
* allocation cache. (Otherwise just release our reference to it)
124-
*/
125-
if (page_count(page) == 1 && !pipe->tmp_page)
126-
pipe->tmp_page = page;
127-
else
128-
put_page(page);
148+
anon_pipe_put_page(pipe, page);
129149
}
130150

131151
static bool anon_pipe_buf_try_steal(struct pipe_inode_info *pipe,
@@ -493,27 +513,25 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
493513
if (!pipe_full(head, pipe->tail, pipe->max_usage)) {
494514
unsigned int mask = pipe->ring_size - 1;
495515
struct pipe_buffer *buf;
496-
struct page *page = pipe->tmp_page;
516+
struct page *page;
497517
int copied;
498518

499-
if (!page) {
500-
page = alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
501-
if (unlikely(!page)) {
502-
ret = ret ? : -ENOMEM;
503-
break;
504-
}
505-
pipe->tmp_page = page;
519+
page = anon_pipe_get_page(pipe);
520+
if (unlikely(!page)) {
521+
if (!ret)
522+
ret = -ENOMEM;
523+
break;
506524
}
507525

508526
copied = copy_page_from_iter(page, 0, PAGE_SIZE, from);
509527
if (unlikely(copied < PAGE_SIZE && iov_iter_count(from))) {
528+
anon_pipe_put_page(pipe, page);
510529
if (!ret)
511530
ret = -EFAULT;
512531
break;
513532
}
514533

515534
pipe->head = head + 1;
516-
pipe->tmp_page = NULL;
517535
/* Insert it into the buffer array */
518536
buf = &pipe->bufs[head & mask];
519537
buf->page = page;
@@ -529,10 +547,9 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
529547

530548
if (!iov_iter_count(from))
531549
break;
532-
}
533550

534-
if (!pipe_full(head, pipe->tail, pipe->max_usage))
535551
continue;
552+
}
536553

537554
/* Wait for buffer space to become available. */
538555
if ((filp->f_flags & O_NONBLOCK) ||
@@ -847,8 +864,10 @@ void free_pipe_info(struct pipe_inode_info *pipe)
847864
if (pipe->watch_queue)
848865
put_watch_queue(pipe->watch_queue);
849866
#endif
850-
if (pipe->tmp_page)
851-
__free_page(pipe->tmp_page);
867+
for (i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
868+
if (pipe->tmp_page[i])
869+
__free_page(pipe->tmp_page[i]);
870+
}
852871
kfree(pipe->bufs);
853872
kfree(pipe);
854873
}

include/linux/pipe_fs_i.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ struct pipe_inode_info {
7272
#ifdef CONFIG_WATCH_QUEUE
7373
bool note_loss;
7474
#endif
75-
struct page *tmp_page;
75+
struct page *tmp_page[2];
7676
struct fasync_struct *fasync_readers;
7777
struct fasync_struct *fasync_writers;
7878
struct pipe_buffer *bufs;

include/linux/wait.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,9 @@ extern void init_wait_entry(struct wait_queue_entry *wq_entry, int flags);
316316
} \
317317
\
318318
cmd; \
319+
\
320+
if (condition) \
321+
break; \
319322
} \
320323
finish_wait(&wq_head, &__wq_entry); \
321324
__out: __ret; \

0 commit comments

Comments
 (0)