Re: [PATCH] pipe_read: don't wake up the writer if the pipe is still full

[Oleg Nesterov <oleg@redhat.com>]

On 03/05, Hillf Danton wrote:
>
> On Tue, 4 Mar 2025 13:34:57 +0100 Oleg Nesterov <oleg@redhat.com>
> > > >
> > > Note wakeup can occur even if pipe is full,
> >
> > Perhaps I misunderstood you, but I don't think pipe_read() can ever do
> > wake_up(pipe->wr_wait) if pipe is full...
> >
> > > 		 * 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.
> > > 		 */
> >
> > Only if wake_writer is true,
> >
> > 		if (unlikely(wake_writer))
> > 			wake_up_interruptible_sync_poll(...);
> >
> > and in this case the pipe is no longer full. A zero-sized buffer was
> > removed.
> >
> > Of course this pipe can be full again when the woken writer checks the
> > condition, but this is another story. And in this case, with your
> > proposed change, the woken writer will take pipe->mutex for no reason.
> >
> See the following sequence,
>
> 	1) waker makes full false
> 	2) waker makes full true
> 	3) waiter checks full
> 	4) waker makes full false

I don't really understand this sequence, but

> waiter has no real idea of full without lock held, perhaps regardless
> the code cut below.

Of course! Again, whatever the woken writer checks in pipe_writable()
lockless, another writer can make pipe_full() true again.

But why do we care? Why do you think that the change you propose makes
more sense than the fix from Prateek or the (already merged) Linus's fix?

Oleg.