On Mon, Jan 13, 2025 at 07:52:57PM +0100, Oleg Nesterov wrote:
> Well, I guess I need to react somehow...
>
> On 01/10, kernel test robot wrote:
> >
> > kernel test robot noticed a 7.5% regression of hackbench.throughput on:
> >
> > commit: aaec5a95d59615523db03dd53c2052f0a87beea7 ("pipe_read: don't wake up the writer if the pipe is still full")
> > https://git.kernel.org/cgit/linux/kernel/git/vfs/vfs.git vfs-6.14.misc
>
> Hmm. Not good ;)
>
> But otoh,
>
> > In addition to that, the commit also has significant impact on the following tests:
> >
> > +------------------+-------------------------------------------------------------------------------------------+
> > | testcase: change | stress-ng: stress-ng.tee.ops_per_sec 500.7% improvement |
>
> So I hope we do not need to revert this patch?
>
> -------------------------------------------------------------------------------
> I am looking at
>
> https://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git/tree/src/hackbench/hackbench.c
>
> and I don't understand how can this patch make a noticable difference.
> And can't reproduce,
>
> hackbench -g 4 -f 10 --process --pipe -l 50000 -s 100
>
> on my laptop under qemu doesn't show any regression.
>
> OK, in this case the early/unnecessary wakeup (removed by this patch) is
> not necessarily bad, when the woken writer actually gets CPU pipe_full()
> will be likely false, plus receiver() can wakeup more writers when it does
> the next read()s. But 7.5% ?
>
> Perhaps this is another case which shows that "artificial" benchmarks like
> this one are very sensitive... Or perhaps I am trying to deny the problem.
>
> So, Christian, et al, unless you think I should try to investigate, I am
> going to forget this report. If nothing else, "500.7% improvement" doesn't
> look bad even if I have no idea whether the stress-ng.tee.ops_per_sec test
> realistic or not (I have no idea what does it do).
Fwiw, I ignored the report too and I intend to keep the patch. IOW, I
agree with you. Thanks for your work!
