Commit d1c6a2aa authored by Linus Torvalds's avatar Linus Torvalds

pipe: simplify signal handling in pipe_read() and add comments

There's no need to separately check for signals while inside the locked
region, since we're going to do "wait_event_interruptible()" right
afterwards anyway, and the error handling is much simpler there.

The check for whether we had already read anything was also redundant,
since we no longer do the odd merging of reads when there are pending
writers.

But perhaps more importantly, this adds commentary about why we still
need to wake up possible writers even though we didn't read any data,
and why we can skip all the finishing touches now if we get a signal (or
had a signal pending) while waiting for more data.

[ This is a split-out cleanup from my "make pipe IO use exclusive wait
  queues" thing, which I can't apply because it triggers a nasty bug in
  the GNU make jobserver   - Linus ]
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 6794862a
...@@ -364,17 +364,39 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) ...@@ -364,17 +364,39 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
ret = -EAGAIN; ret = -EAGAIN;
break; break;
} }
if (signal_pending(current)) {
if (!ret)
ret = -ERESTARTSYS;
break;
}
__pipe_unlock(pipe); __pipe_unlock(pipe);
if (was_full) {
/*
* We only get here if we didn't actually read anything.
*
* However, we could have seen (and removed) a zero-sized
* pipe buffer, and might have made space in the buffers
* that way.
*
* You can't make zero-sized pipe buffers by doing an empty
* write (not even in packet mode), but they can happen if
* the writer gets an EFAULT when trying to fill a buffer
* that already got allocated and inserted in the buffer
* array.
*
* So we still need to wake up any pending writers in the
* _very_ unlikely case that the pipe was full, but we got
* no data.
*/
if (unlikely(was_full)) {
wake_up_interruptible_sync_poll(&pipe->wait, EPOLLOUT | EPOLLWRNORM); wake_up_interruptible_sync_poll(&pipe->wait, EPOLLOUT | EPOLLWRNORM);
kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
} }
wait_event_interruptible(pipe->wait, pipe_readable(pipe));
/*
* But because we didn't read anything, at this point we can
* just return directly with -ERESTARTSYS if we're interrupted,
* since we've done any required wakeups and there's no need
* to mark anything accessed. And we've dropped the lock.
*/
if (wait_event_interruptible(pipe->wait, pipe_readable(pipe)) < 0)
return -ERESTARTSYS;
__pipe_lock(pipe); __pipe_lock(pipe);
was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage); was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment