Skip to content

Commit 85190d1

Browse files
committed
pipe: don't use 'pipe_wait() for basic pipe IO
pipe_wait() may be simple, but since it relies on the pipe lock, it means that we have to do the wakeup while holding the lock. That's unfortunate, because the very first thing the waked entity will want to do is to get the pipe lock for itself. So get rid of the pipe_wait() usage by simply releasing the pipe lock, doing the wakeup (if required) and then using wait_event_interruptible() to wait on the right condition instead. wait_event_interruptible() handles races on its own by comparing the wakeup condition before and after adding itself to the wait queue, so you can use an optimistic unlocked condition for it. Cc: David Howells <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent a28c8b9 commit 85190d1

File tree

1 file changed

+27
-3
lines changed

1 file changed

+27
-3
lines changed

fs/pipe.c

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,16 @@ static bool pipe_buf_can_merge(struct pipe_buffer *buf)
270270
return buf->ops == &anon_pipe_buf_ops;
271271
}
272272

273+
/* Done while waiting without holding the pipe lock - thus the READ_ONCE() */
274+
static inline bool pipe_readable(const struct pipe_inode_info *pipe)
275+
{
276+
unsigned int head = READ_ONCE(pipe->head);
277+
unsigned int tail = READ_ONCE(pipe->tail);
278+
unsigned int writers = READ_ONCE(pipe->writers);
279+
280+
return !pipe_empty(head, tail) || !writers;
281+
}
282+
273283
static ssize_t
274284
pipe_read(struct kiocb *iocb, struct iov_iter *to)
275285
{
@@ -359,11 +369,13 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
359369
ret = -ERESTARTSYS;
360370
break;
361371
}
372+
__pipe_unlock(pipe);
362373
if (was_full) {
363374
wake_up_interruptible_sync_poll(&pipe->wait, EPOLLOUT | EPOLLWRNORM);
364375
kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
365376
}
366-
pipe_wait(pipe);
377+
wait_event_interruptible(pipe->wait, pipe_readable(pipe));
378+
__pipe_lock(pipe);
367379
was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
368380
}
369381
__pipe_unlock(pipe);
@@ -382,6 +394,17 @@ static inline int is_packetized(struct file *file)
382394
return (file->f_flags & O_DIRECT) != 0;
383395
}
384396

397+
/* Done while waiting without holding the pipe lock - thus the READ_ONCE() */
398+
static inline bool pipe_writable(const struct pipe_inode_info *pipe)
399+
{
400+
unsigned int head = READ_ONCE(pipe->head);
401+
unsigned int tail = READ_ONCE(pipe->tail);
402+
unsigned int max_usage = READ_ONCE(pipe->max_usage);
403+
404+
return !pipe_full(head, tail, max_usage) ||
405+
!READ_ONCE(pipe->readers);
406+
}
407+
385408
static ssize_t
386409
pipe_write(struct kiocb *iocb, struct iov_iter *from)
387410
{
@@ -529,12 +552,13 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
529552
* after waiting we need to re-check whether the pipe
530553
* become empty while we dropped the lock.
531554
*/
555+
__pipe_unlock(pipe);
532556
if (was_empty) {
533557
wake_up_interruptible_sync_poll(&pipe->wait, EPOLLIN | EPOLLRDNORM);
534558
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
535559
}
536-
pipe_wait(pipe);
537-
560+
wait_event_interruptible(pipe->wait, pipe_writable(pipe));
561+
__pipe_lock(pipe);
538562
was_empty = pipe_empty(head, pipe->tail);
539563
}
540564
out:

0 commit comments

Comments
 (0)