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

From: Hillf Danton
Date: Mon Mar 10 2025 - 07:38:36 EST


On Mon, 10 Mar 2025 12:09:15 +0100 Oleg Nesterov
> On 03/10, Hillf Danton wrote:
> > On Sun, 9 Mar 2025 18:02:55 +0100 Oleg Nesterov
> > >
> > > So (again, in this particular case) we could apply the patch below
> > > on top of Linus's tree.
> > >
> > > So, with or without these changes, the writer should be woken up at
> > > step-03 in your scenario.
> > >
> > Fine, before checking my scenario once more, feel free to pinpoint the
> > line number where writer is woken up, with the change below applied.
>
> 381 if (wake_writer)
> ==> 382 wake_up_interruptible_sync_poll(&pipe->wr_wait, EPOLLOUT | EPOLLWRNORM);
> 383 if (wake_next_reader)
> 384 wake_up_interruptible_sync_poll(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
> 385 kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
> 386 if (ret > 0)
> 387 file_accessed(filp);
> 388 return ret;
>
> line 382, no?
>
Yes, but how is the wait loop at line-370 broken?

360 }
361 mutex_unlock(&pipe->mutex);
362
363 BUG_ON(wake_writer);
364 /*
365 * But because we didn't read anything, at this point we can
366 * just return directly with -ERESTARTSYS if we're interrupted,
367 * since we've done any required wakeups and there's no need
368 * to mark anything accessed. And we've dropped the lock.
369 */
370 if (wait_event_interruptible_exclusive(pipe->rd_wait, pipe_readable(pipe)) < 0)
371 return -ERESTARTSYS;
372
373 wake_writer = false;
374 wake_next_reader = true;
375 mutex_lock(&pipe->mutex);