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

["Bae, Chang Seok" <chang.seok.bae@intel.com>]

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 */