On Mon, Jan 20, 2025 at 1:42 PM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> Forgot to mention...
>
> On 01/20, Oleg Nesterov wrote:
> >
> > On 01/20, Mateusz Guzik wrote:
> > >
> > > Whatever the long term fate of the patch I think it would be prudent to
> > > skip it in this merge window.
> >
> > Perhaps... I'll try to take another look tomorrow.
> >
> > Just one note right now.
> >
> > > First two notes:
> > > 1. the change only considers performing a wake up if the current
> > > source buf got depleted -- if there is a blocked writer and there is at
> > > least one byte in the current buf nothing happens, which is where the
> > > difference in results is coming from
> >
> > Sorry I don't understand. Unless this patch is buggy, pipe_read() must
> > always wakeup a blocked writer if the writer can write at least one byte.
> >
> > The writer can't write to "current" buf = pipe->bufs[tail & mask] if
> > pipe_full() is still true.
>
> But I'll recheck this logic once again tomorrow, perhaps I misread
> pipe_write() when I made this patch.
>
While I'm too tired to dig into the code at the momen, I did manage to
grab an extra data point for hackerbench. Note on my setup (24-way) it
takes way longer to execute with your patch.
I checked how often the sucker goess off cpu, like so: bpftrace -e
'kprobe:schedule { @[kstack()] = count(); }'
With your patch I reliably get about 38 mln calls from pipe_read.
Without your patch this drops to about 17 mln, as in less than half.
--
Mateusz Guzik <mjguzik gmail.com>
