Re: [PATCH AUTOSEL 5.5 542/542] pipe: use exclusive waits when reading or writing

[Linus Torvalds]

On Tue, Feb 18, 2020 at 10:20 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> You don't want to move wake_up_partner() up and call it from pipe_release()?

I was actually thinking of going the other way - two of three users of
wake_up_partner() are redundantly waking up the wrong side, and the
third user is pointlessly written too.

So I was _thinking_ of a patch like the appended (which is on top of
the previous patch), but ended up not doing it. Until you brought it
up.

But I won't bother committing this, since it shouldn't really matter.

                 Linus
 fs/pipe.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 2144507447c5..79ba61430f9c 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1025,12 +1025,6 @@ static int wait_for_partner(struct pipe_inode_info *pipe, unsigned int *cnt)
 	return cur == *cnt ? -ERESTARTSYS : 0;
 }
 
-static void wake_up_partner(struct pipe_inode_info *pipe)
-{
-	wake_up_interruptible_all(&pipe->rd_wait);
-	wake_up_interruptible_all(&pipe->wr_wait);
-}
-
 static int fifo_open(struct inode *inode, struct file *filp)
 {
 	struct pipe_inode_info *pipe;
@@ -1078,7 +1072,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
 	 */
 		pipe->r_counter++;
 		if (pipe->readers++ == 0)
-			wake_up_partner(pipe);
+			wake_up_interruptible_all(&pipe->wr_wait);
 
 		if (!is_pipe && !pipe->writers) {
 			if ((filp->f_flags & O_NONBLOCK)) {
@@ -1104,7 +1098,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
 
 		pipe->w_counter++;
 		if (!pipe->writers++)
-			wake_up_partner(pipe);
+			wake_up_interruptible_all(&pipe->rd_wait);
 
 		if (!is_pipe && !pipe->readers) {
 			if (wait_for_partner(pipe, &pipe->r_counter))
@@ -1120,12 +1114,12 @@ static int fifo_open(struct inode *inode, struct file *filp)
 	 *  the process can at least talk to itself.
 	 */
 
-		pipe->readers++;
-		pipe->writers++;
+		if (pipe->readers++ == 0)
+			wake_up_interruptible_all(&pipe->wr_wait);
+		if (pipe->writers++ == 0)
+			wake_up_interruptible_all(&pipe->rd_wait);
 		pipe->r_counter++;
 		pipe->w_counter++;
-		if (pipe->readers == 1 || pipe->writers == 1)
-			wake_up_partner(pipe);
 		break;
 
 	default: