mirrors/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2025-11-01 00:58:39 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

pipe_read: don't wake up the writer if the pipe is still full

Browse source 
wake_up(pipe->wr_wait) makes no sense if pipe_full() is still true after
the reading, the writer sleeping in wait_event(wr_wait, pipe_writable())
will check the pipe_writable() == !pipe_full() condition and sleep again.

Only wake the writer if we actually released a pipe buf, and the pipe was
full before we did so.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/all/20241229135737.GA3293@redhat.com/
Link: https://lore.kernel.org/r/20250102140715.GA7091@redhat.com
Reported-by: WangYuli <wangyuli@uniontech.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
This commit is contained in:
Oleg Nesterov 2025-01-02 15:07:15 +01:00 committed by Christian Brauner
parent d2fc0ed52a
commit aaec5a95d5
No known key found for this signature in database
GPG key ID: 91C61BC06578DCA2
1 changed files with 10 additions and 9 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

19
fs/pipe.c
View file

@ -253,7 +253,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
size_t total_len = iov_iter_count(to); size_t total_len = iov_iter_count(to);
struct file *filp = iocb->ki_filp; struct file *filp = iocb->ki_filp;
struct pipe_inode_info *pipe = filp->private_data; struct pipe_inode_info *pipe = filp->private_data;
bool was_full, wake_next_reader = false; bool wake_writer = false, wake_next_reader = false;
ssize_t ret; ssize_t ret;
/* Null read succeeds. */ /* Null read succeeds. */
@ -264,14 +264,13 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
mutex_lock(&pipe->mutex); mutex_lock(&pipe->mutex);
/* /*
* We only wake up writers if the pipe was full when we started * We only wake up writers if the pipe was full when we started reading
* reading in order to avoid unnecessary wakeups. * and it is no longer full after reading to avoid unnecessary wakeups.
* *
* But when we do wake up writers, we do so using a sync wakeup * But when we do wake up writers, we do so using a sync wakeup
* (WF_SYNC), because we want them to get going and generate more * (WF_SYNC), because we want them to get going and generate more
* data for us. * data for us.
*/ */
was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
for (;;) { for (;;) {
/* Read ->head with a barrier vs post_one_notification() */ /* Read ->head with a barrier vs post_one_notification() */
unsigned int head = smp_load_acquire(&pipe->head); unsigned int head = smp_load_acquire(&pipe->head);
@ -340,8 +339,10 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
buf->len = 0; buf->len = 0;
} }
if (!buf->len) if (!buf->len) {
wake_writer |= pipe_full(head, tail, pipe->max_usage);
tail = pipe_update_tail(pipe, buf, tail); tail = pipe_update_tail(pipe, buf, tail);
}
total_len -= chars; total_len -= chars;
if (!total_len) if (!total_len)
break; /* common path: read succeeded */ break; /* common path: read succeeded */
@ -377,7 +378,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
* _very_ unlikely case that the pipe was full, but we got * _very_ unlikely case that the pipe was full, but we got
* no data. * no data.
*/ */
if (unlikely(was_full)) if (unlikely(wake_writer))
wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM); wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM);
kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
@ -390,15 +391,15 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
if (wait_event_interruptible_exclusive(pipe->rd_wait, pipe_readable(pipe)) < 0) if (wait_event_interruptible_exclusive(pipe->rd_wait, pipe_readable(pipe)) < 0)
return -ERESTARTSYS; return -ERESTARTSYS;
mutex_lock(&pipe->mutex); wake_writer = false;
was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
wake_next_reader = true; wake_next_reader = true;
mutex_lock(&pipe->mutex);
} }
if (pipe_empty(pipe->head, pipe->tail)) if (pipe_empty(pipe->head, pipe->tail))
wake_next_reader = false; wake_next_reader = false;
mutex_unlock(&pipe->mutex); mutex_unlock(&pipe->mutex);
if (was_full) if (wake_writer)
wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM); wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM);
if (wake_next_reader) if (wake_next_reader)
wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM); wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);