Re: [x86/signal] 3aac3ebea0: will-it-scale.per_thread_ops -11.9% regression

Bae, Chang Seok
2021-12-08T18:01:03Z

On Dec 7, 2021, at 15:14, Hansen, Dave <dave.hansen@intel.com> wrote:
>
> Oliver or Chang, could you try to reproduce this by hand on one of the
> suspect systems? Build:
>
> 1bdda24c4a ("signal: Add an optional check for altstack size")
>
> then run will-it-scale by hand. Then build:
>
> 3aac3ebea0 ("x86/signal: Implement sigaltstack size validation")
>
> and run it again. Also, do we see any higher core-count regressions?
> These all seem to happen with:
>
> mode=thread
> nr_task=16
>
> That's really odd to see that for these systems with probably ~50 cores
> each. I'd expect to see it get worse at higher core counts.

tglx also asked a similar thing -- test it without the python script between
5.16-rcX vs 5.15.

So, ran this with those kernel versions and some patches:
$ ./signal1_threads -s 10 -t 16

The test accounts total number of delivered signals in 16 threads in a second.
Repeat this 10 times and then average out. That’s what '{ average }' tells. So,
higher is better.

In this test, the lock contention came from this signal return path:
rt_sigreturn()
--> restore_altstack()
--> do_sigaltstack()
But do_sigaltstack() is not needed here as no sigaltstack changes. So, perhaps
skip it like [2].

Then, I got this:

{ kernel version } : { average }
5.15.0 : 585577
5.16.0-rc4 : 514959
5.16.0-rc4 + [1] : 575066
5.16.0-rc4 + [2] : 597201

I think at least the last case serves as a proof of concept for this issue.

Thanks,
Chang

[1] tglx’s diff shown in here: https://lore.kernel.org/lkml/87bl1s357p.ffs@tglx/

[2]
diff --git a/kernel/signal.c b/kernel/signal.c
index a629b11bf3e0..8194d2f38bf1 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -4224,6 +4224,11 @@ int restore_altstack(const stack_t __user *uss)
stack_t new;
if (copy_from_user(&new, uss, sizeof(stack_t)))
return -EFAULT;
+ if (current->sas_ss_sp == (unsigned long) new.ss_sp &&
+ current->sas_ss_size == new.ss_size &&
+ current->sas_ss_flags == new.ss_flags)
+ return 0;
+
(void)do_sigaltstack(&new, NULL, current_user_stack_pointer(),
MINSIGSTKSZ);
/* squash all but EFAULT for now */