ANDROID: fs: pipe: wakeup readers on small writes even if pipe had data
commit '1b6b26ae7053 ("pipe: fix and clarify pipe write wakeup logic")' change `pipe_write()` wakeup logic to wakeup readers only if the pipe was empty. This meant that applications that are not draining the pipe before each write were exposed to unexpected timeouts / hangs in epoll_wait() waiting for data in a pipe using EPOLLIN | EPOLLET flags. This behaviour can be easily tested with android12-5.4 kernel where the test that uses pipes for notifications in this way works while it fails 100% with android12-5.10. This change restores the old behavior to wakeup all pipe_readers if any new data is written to the pipe. Bug: 193851993 Bug: 193846582 Change-Id: If0c5a844091ccf16d5236bd072326325d4d5447a Signed-off-by: Sandeep Patil <sspatil@google.com>
This commit is contained in:
19
fs/pipe.c
19
fs/pipe.c
@@ -406,7 +406,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
|
|||||||
ssize_t ret = 0;
|
ssize_t ret = 0;
|
||||||
size_t total_len = iov_iter_count(from);
|
size_t total_len = iov_iter_count(from);
|
||||||
ssize_t chars;
|
ssize_t chars;
|
||||||
bool was_empty = false;
|
bool do_wakeup = false;
|
||||||
bool wake_next_writer = false;
|
bool wake_next_writer = false;
|
||||||
|
|
||||||
/* Null write succeeds. */
|
/* Null write succeeds. */
|
||||||
@@ -429,10 +429,11 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
|
|||||||
#endif
|
#endif
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Only wake up if the pipe started out empty, since
|
* Wake up readers if the pipe was written to. Regardless
|
||||||
* otherwise there should be no readers waiting.
|
* of whether it was empty or not. Otherwise, threads
|
||||||
|
* waiting with EPOLLET will hang until the pipe is emptied.
|
||||||
*
|
*
|
||||||
* If it wasn't empty we try to merge new data into
|
* If pipe wasn't empty we try to merge new data into
|
||||||
* the last buffer.
|
* the last buffer.
|
||||||
*
|
*
|
||||||
* That naturally merges small writes, but it also
|
* That naturally merges small writes, but it also
|
||||||
@@ -440,9 +441,8 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
|
|||||||
* spanning multiple pages.
|
* spanning multiple pages.
|
||||||
*/
|
*/
|
||||||
head = pipe->head;
|
head = pipe->head;
|
||||||
was_empty = pipe_empty(head, pipe->tail);
|
|
||||||
chars = total_len & (PAGE_SIZE-1);
|
chars = total_len & (PAGE_SIZE-1);
|
||||||
if (chars && !was_empty) {
|
if (chars && !pipe_empty(head, pipe->tail)) {
|
||||||
unsigned int mask = pipe->ring_size - 1;
|
unsigned int mask = pipe->ring_size - 1;
|
||||||
struct pipe_buffer *buf = &pipe->bufs[(head - 1) & mask];
|
struct pipe_buffer *buf = &pipe->bufs[(head - 1) & mask];
|
||||||
int offset = buf->offset + buf->len;
|
int offset = buf->offset + buf->len;
|
||||||
@@ -460,6 +460,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
|
|||||||
}
|
}
|
||||||
|
|
||||||
buf->len += ret;
|
buf->len += ret;
|
||||||
|
do_wakeup = true;
|
||||||
if (!iov_iter_count(from))
|
if (!iov_iter_count(from))
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
@@ -526,6 +527,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
|
|||||||
ret += copied;
|
ret += copied;
|
||||||
buf->offset = 0;
|
buf->offset = 0;
|
||||||
buf->len = copied;
|
buf->len = copied;
|
||||||
|
do_wakeup = true;
|
||||||
|
|
||||||
if (!iov_iter_count(from))
|
if (!iov_iter_count(from))
|
||||||
break;
|
break;
|
||||||
@@ -553,13 +555,12 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
|
|||||||
* become empty while we dropped the lock.
|
* become empty while we dropped the lock.
|
||||||
*/
|
*/
|
||||||
__pipe_unlock(pipe);
|
__pipe_unlock(pipe);
|
||||||
if (was_empty) {
|
if (do_wakeup) {
|
||||||
wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
|
wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
|
||||||
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
|
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
|
||||||
}
|
}
|
||||||
wait_event_interruptible_exclusive(pipe->wr_wait, pipe_writable(pipe));
|
wait_event_interruptible_exclusive(pipe->wr_wait, pipe_writable(pipe));
|
||||||
__pipe_lock(pipe);
|
__pipe_lock(pipe);
|
||||||
was_empty = pipe_empty(pipe->head, pipe->tail);
|
|
||||||
wake_next_writer = true;
|
wake_next_writer = true;
|
||||||
}
|
}
|
||||||
out:
|
out:
|
||||||
@@ -576,7 +577,7 @@ out:
|
|||||||
* how (for example) the GNU make jobserver uses small writes to
|
* how (for example) the GNU make jobserver uses small writes to
|
||||||
* wake up pending jobs
|
* wake up pending jobs
|
||||||
*/
|
*/
|
||||||
if (was_empty) {
|
if (do_wakeup) {
|
||||||
wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
|
wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
|
||||||
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
|
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user