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

[Oleg Nesterov]

Hi Hillf,

again, I am not sure we understand each other, at least me...

On 03/05, Hillf Danton wrote:
>
> On Wed, 5 Mar 2025 00:49:09 +0100 Oleg Nesterov <oleg@xxxxxxxxxx>
> >
> > 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
>
> Because of the hang reported.

The hang happened because pipe_writable() could wrongly return false
when the buffer is not full.

Afaics, the Prateek's or Linus's fix solve this problem, and this is
all we need.

> > more sense than the fix from Prateek or the (already merged) Linus's fix?
> >
> See the loop in  ___wait_event(),
>
> 	for (;;) {
> 		prepare_to_wait_event();
>
> 		// flip
> 		if (condition)
> 			break;
>
> 		schedule();
> 	}

I will assume that this "// flip" means the case I described above:
before this writer checks the condition, another writer comes and
increments pipe->head.

> After wakeup, waiter will sleep again if condition flips false on the waker
> side before waiter checks condition, even if condition is atomic, no?

Yes, but in this case pipe_full() == true is correct, this writer can
safely sleep.

Even if flips again and becomes false right after the writer called
pipe_writable().

Note that it checks the condition after set_current_state(INTERRUPTIBLE)
and it is still on the pipe->wr_wait->head list.

The 2nd "flip" is only possible if some reader flushes another buffer
and updates pipe->tail, and in this case it will wake this writer again.

So I still can't understand your concerns, sorry.

Oleg.