pipe: remove 'waiting_writers' merging logic
authorLinus Torvalds <torvalds@linux-foundation.org>
Sat, 7 Dec 2019 21:21:01 +0000 (13:21 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sat, 7 Dec 2019 21:21:01 +0000 (13:21 -0800)
This code is ancient, and goes back to when we only had a single page
for the pipe buffers.  The exact history is hidden in the mists of time
(ie "before git", and in fact predates the BK repository too).

At that long-ago point in time, it actually helped to try to merge big
back-and-forth pipe reads and writes, and not limit pipe reads to the
single pipe buffer in length just because that was all we had at a time.

However, since then we've expanded the pipe buffers to multiple pages,
and this logic really doesn't seem to make sense.  And a lot of it is
somewhat questionable (ie "hmm, the user asked for a non-blocking read,
but we see that there's a writer pending, so let's wait anyway to get
the extra data that the writer will have").

But more importantly, it makes the "go to sleep" logic much less
obvious, and considering the wakeup issues we've had, I want to make for
less of those kinds of things.

Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/pipe.c
fs/splice.c
include/linux/pipe_fs_i.h

index bcc2192d33e29270d12003e124c87283e325cf67..58f236c65beabde14a1d51dca78f7f94bd93b50d 100644 (file)
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -348,18 +348,11 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
 
                if (!pipe->writers)
                        break;
-               if (!pipe->waiting_writers) {
-                       /* syscall merging: Usually we must not sleep
-                        * if O_NONBLOCK is set, or if we got some data.
-                        * But if a writer sleeps in kernel space, then
-                        * we can wait for that data without violating POSIX.
-                        */
-                       if (ret)
-                               break;
-                       if (filp->f_flags & O_NONBLOCK) {
-                               ret = -EAGAIN;
-                               break;
-                       }
+               if (ret)
+                       break;
+               if (filp->f_flags & O_NONBLOCK) {
+                       ret = -EAGAIN;
+                       break;
                }
                if (signal_pending(current)) {
                        if (!ret)
@@ -540,9 +533,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
                        wake_up_interruptible_sync_poll(&pipe->wait, EPOLLIN | EPOLLRDNORM);
                        kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
                }
-               pipe->waiting_writers++;
                pipe_wait(pipe);
-               pipe->waiting_writers--;
 
                was_empty = pipe_empty(head, pipe->tail);
        }
index fa1f3773c8cd2f875b6de26c9a09b7f07ccdb362..3009652a41c85c28e048b7902dbce03a159cfc2e 100644 (file)
@@ -559,7 +559,7 @@ static int splice_from_pipe_next(struct pipe_inode_info *pipe, struct splice_des
                if (!pipe->writers)
                        return 0;
 
-               if (!pipe->waiting_writers && sd->num_spliced)
+               if (sd->num_spliced)
                        return 0;
 
                if (sd->flags & SPLICE_F_NONBLOCK)
@@ -1098,9 +1098,7 @@ static int wait_for_space(struct pipe_inode_info *pipe, unsigned flags)
                        return -EAGAIN;
                if (signal_pending(current))
                        return -ERESTARTSYS;
-               pipe->waiting_writers++;
                pipe_wait(pipe);
-               pipe->waiting_writers--;
        }
 }
 
@@ -1482,11 +1480,9 @@ static int ipipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
                }
                if (!pipe->writers)
                        break;
-               if (!pipe->waiting_writers) {
-                       if (flags & SPLICE_F_NONBLOCK) {
-                               ret = -EAGAIN;
-                               break;
-                       }
+               if (flags & SPLICE_F_NONBLOCK) {
+                       ret = -EAGAIN;
+                       break;
                }
                pipe_wait(pipe);
        }
@@ -1527,9 +1523,7 @@ static int opipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
                        ret = -ERESTARTSYS;
                        break;
                }
-               pipe->waiting_writers++;
                pipe_wait(pipe);
-               pipe->waiting_writers--;
        }
 
        pipe_unlock(pipe);
@@ -1751,13 +1745,6 @@ static int link_pipe(struct pipe_inode_info *ipipe,
                i_tail++;
        } while (len);
 
-       /*
-        * return EAGAIN if we have the potential of some data in the
-        * future, otherwise just return 0
-        */
-       if (!ret && ipipe->waiting_writers && (flags & SPLICE_F_NONBLOCK))
-               ret = -EAGAIN;
-
        pipe_unlock(ipipe);
        pipe_unlock(opipe);
 
index 44f2245debda4f839e85fdbfdc748dacc74c1c4c..dbcfa68923842d41747938e042aa0f9a54f95eab 100644 (file)
@@ -38,7 +38,6 @@ struct pipe_buffer {
  *     @readers: number of current readers of this pipe
  *     @writers: number of current writers of this pipe
  *     @files: number of struct file referring this pipe (protected by ->i_lock)
- *     @waiting_writers: number of writers blocked waiting for room
  *     @r_counter: reader counter
  *     @w_counter: writer counter
  *     @fasync_readers: reader side fasync
@@ -56,7 +55,6 @@ struct pipe_inode_info {
        unsigned int readers;
        unsigned int writers;
        unsigned int files;
-       unsigned int waiting_writers;
        unsigned int r_counter;
        unsigned int w_counter;
        struct page *tmp_page;