Re: [dennis-percpu:for-6.11] [percpu_counter] 05dbad003f: ltp.getrusage03.fail

[Dennis Zhou <dennis@kernel.org>]

On Tue, May 28, 2024 at 11:05:54AM +0200, Mateusz Guzik wrote:
> On Tue, May 28, 2024 at 02:45:35PM +0800, kernel test robot wrote:
> > Hello,
> > 
> > kernel test robot noticed "ltp.getrusage03.fail" on:
> > 
> > commit: 05dbad003f2b2ececb1cc7428bfa8f470cc34b95 ("percpu_counter: add a cmpxchg-based _add_batch variant")
> > https://git.kernel.org/cgit/linux/kernel/git/dennis/percpu.git for-6.11
> > 
> > in testcase: ltp
> > version: ltp-x86_64-14c1f76-1_20240525
> > with following parameters:
> > 
> > 	disk: 1HDD
> > 	fs: xfs
> > 	test: syscalls-03/getrusage03
> > 
> > 
> > 
> > Running tests.......
> > <<<test_start>>>
> > tag=getrusage03 stime=1716734654
> > cmdline="getrusage03"
> > contacts=""
> > analysis=exit
> > <<<test_output>>>
> > tst_test.c:1733: TINFO: LTP version: 20240524-4-g22004c7db
> > tst_test.c:1617: TINFO: Timeout per run is 0h 02m 30s
> > getrusage03.c:43: TPASS: initial.self ~= child.self
> > getrusage03.c:59: TFAIL: initial.children = 0, expected 102400
> > getrusage03.c:66: TPASS: child.children == 0
> > getrusage03.c:86: TFAIL: child.children = 0, expected 307200
> > getrusage03.c:104: TPASS: initial.children ~= pre_wait.children
> > getrusage03.c:114: TFAIL: post_wait.children = 0, expected 409600
> > getrusage03.c:133: TPASS: initial.children ~= after_zombie.children
> > getrusage03_child.c:57: TPASS: initial.self ~= exec.self
> > getrusage03_child.c:62: TPASS: initial.children ~= exec.children
> > 
> 
> I confirm this patch broke things, thanks for the report.
> 
> Make sure to sit before you take a look at the fix:
> 
> commit 6e8b4caf667fb6fad1c63b061e303faab6d917ef
> Author: Mateusz Guzik <mjguzik@gmail.com>
> Date:   Tue May 28 04:52:10 2024 -0400
> 
>     lol
> 
> diff --git a/lib/percpu_counter.c b/lib/percpu_counter.c
> index c3140276bb36..51bc5246986d 100644
> --- a/lib/percpu_counter.c
> +++ b/lib/percpu_counter.c
> @@ -97,7 +97,7 @@ void percpu_counter_add_batch(struct percpu_counter *fbc, s64 amount, s32 batch)
> 
>         count = this_cpu_read(*fbc->counters);
>         do {
> -               if (unlikely(abs(count + amount)) >= batch) {
> +               if (unlikely(abs(count + amount) >= batch)) {
>                         raw_spin_lock_irqsave(&fbc->lock, flags);
>                         /*
>                          * Note: by now we might have migrated to another CPU
> 
> 
> Dennis, do I need to submit a v4? Given that this is a trivial one line
> fixup perhaps it would be handier if you massaged the stuff on your
> branch. I'm fine either way.
> 

Well that's awkward.. Can you please send a v4?

> That said this really should have been reported by something.

That's fair. I think I starred at parenthesis for too long that day so I
missed it too. The nice thing is these branches get longer running CI so
we can track these things.

Thanks,
Dennis