pipe: wakeup wr_wait after setting max_usage

Commit Message

Christian Brauner Dec. 1, 2023, 10:11 a.m. UTC

From: Lukas Schauer <[e-mail]>

Commit c73be61cede5 ("pipe: Add general notification queue support") a
regression was introduced that would lock up resized pipes under certain
conditions. See the reproducer in [1].

The commit resizing the pipe ring size was moved to a different
function, doing that moved the wakeup for pipe->wr_wait before actually
raising pipe->max_usage. If a pipe was full before the resize occured it
would result in the wakeup never actually triggering pipe_write.

Set @max_usage and @nr_accounted before waking writers if this isn't a
watch queue.

Fixes: c73be61cede5 ("pipe: Add general notification queue support")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=212295 [1]
Cc: <[e-mail]>
[Christian Brauner <[e-mail]>: rewrite to account for watch queues]
Signed-off-by: Christian Brauner <[e-mail]>
---
 fs/pipe.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

David Howells Dec. 1, 2023, 11:56 a.m. UTC | #1

Christian Brauner <[e-mail]> wrote:

> From: Lukas Schauer <[e-mail]>
> 
> Commit c73be61cede5 ("pipe: Add general notification queue support") a
> regression was introduced that would lock up resized pipes under certain
> conditions. See the reproducer in [1].
> 
> The commit resizing the pipe ring size was moved to a different
> function, doing that moved the wakeup for pipe->wr_wait before actually
> raising pipe->max_usage. If a pipe was full before the resize occured it
> would result in the wakeup never actually triggering pipe_write.
> 
> Set @max_usage and @nr_accounted before waking writers if this isn't a
> watch queue.
> 
> Fixes: c73be61cede5 ("pipe: Add general notification queue support")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=212295 [1]
> Cc: <[e-mail]>
> [Christian Brauner <[e-mail]>: rewrite to account for watch queues]
> Signed-off-by: Christian Brauner <[e-mail]>

Reviewed-by: David Howells <[e-mail]>

Christian Brauner Dec. 1, 2023, 1:01 p.m. UTC | #2

On Fri, 01 Dec 2023 11:11:28 +0100, Christian Brauner wrote:
> Commit c73be61cede5 ("pipe: Add general notification queue support") a
> regression was introduced that would lock up resized pipes under certain
> conditions. See the reproducer in [1].
> 
> The commit resizing the pipe ring size was moved to a different
> function, doing that moved the wakeup for pipe->wr_wait before actually
> raising pipe->max_usage. If a pipe was full before the resize occured it
> would result in the wakeup never actually triggering pipe_write.
> 
> [...]

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[1/1] pipe: wakeup wr_wait after setting max_usage
      https://git.kernel.org/vfs/vfs/c/348806de39e0

Patch

diff --git a/fs/pipe.c b/fs/pipe.c
index 226e7f66b590..8d9286a1f2e8 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1324,6 +1324,11 @@  int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
 	pipe->tail = tail;
 	pipe->head = head;
 
+	if (!pipe_has_watch_queue(pipe)) {
+		pipe->max_usage = nr_slots;
+		pipe->nr_accounted = nr_slots;
+	}
+
 	spin_unlock_irq(&pipe->rd_wait.lock);
 
 	/* This might have made more room for writers */
@@ -1375,8 +1380,6 @@  static long pipe_set_size(struct pipe_inode_info *pipe, unsigned int arg)
 	if (ret < 0)
 		goto out_revert_acct;
 
-	pipe->max_usage = nr_slots;
-	pipe->nr_accounted = nr_slots;
 	return pipe->max_usage * PAGE_SIZE;
 
 out_revert_acct: