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:
Sandeep Patil
2021-07-23 05:00:22 +00:00
parent 989fb724d5
commit 76879a1964

View File

@@ -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);
} }