[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250227125040.GA25639@redhat.com>
Date: Thu, 27 Feb 2025 13:50:41 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: "Sapkal, Swapnil" <swapnil.sapkal@....com>,
Mateusz Guzik <mjguzik@...il.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Manfred Spraul <manfred@...orfullife.com>,
Christian Brauner <brauner@...nel.org>,
David Howells <dhowells@...hat.com>,
WangYuli <wangyuli@...ontech.com>, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org,
K Prateek Nayak <kprateek.nayak@....com>,
"Shenoy, Gautham Ranjal" <gautham.shenoy@....com>,
Neeraj.Upadhyay@....com
Subject: Re: [PATCH] pipe_read: don't wake up the writer if the pipe is still
full
Hmm...
Suppose that pipe is full, a writer W tries to write a single byte
and sleeps on pipe->wr_wait.
A reader reads PAGE_SIZE bytes, updates pipe->tail, and wakes W up.
But, before the woken W takes pipe->mutex, another writer comes and
writes 1 byte. This updates ->head and makes pipe_full() true again.
Now, W could happily merge its "small" write into the last buffer,
but it will sleep again, despite the fact the last buffer has room
for 4095 bytes.
Sapkal, I don't think this can explain the hang, receiver()->read()
should wake this writer later anyway. But could you please retest
with the patch below?
Thanks,
Oleg.
---
diff --git a/fs/pipe.c b/fs/pipe.c
index b0641f75b1ba..222881559c30 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -455,6 +455,7 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
* page-aligns the rest of the writes for large writes
* spanning multiple pages.
*/
+again:
head = pipe->head;
was_empty = pipe_empty(head, pipe->tail);
chars = total_len & (PAGE_SIZE-1);
@@ -559,8 +560,8 @@ anon_pipe_write(struct kiocb *iocb, struct iov_iter *from)
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
wait_event_interruptible_exclusive(pipe->wr_wait, pipe_writable(pipe));
mutex_lock(&pipe->mutex);
- was_empty = pipe_empty(pipe->head, pipe->tail);
wake_next_writer = true;
+ goto again;
}
out:
if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
Powered by blists - more mailing lists