linux-kernel - Re: [x86/signal] 3aac3ebea0: will-it-scale.per_thread

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 8 Dec 2021 18:00:06 +0000
From:   "Bae, Chang Seok" <chang.seok.bae@...el.com>
To:     "Hansen, Dave" <dave.hansen@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>
CC:     "Sang, Oliver" <oliver.sang@...el.com>,
        Borislav Petkov <bp@...e.de>,
        LKML <linux-kernel@...r.kernel.org>,
        "lkp@...ts.01.org" <lkp@...ts.01.org>, lkp <lkp@...el.com>,
        "Huang, Ying" <ying.huang@...el.com>,
        "Tang, Feng" <feng.tang@...el.com>,
        "zhengjun.xing@...ux.intel.com" <zhengjun.xing@...ux.intel.com>,
        "Yin, Fengwei" <fengwei.yin@...el.com>
Subject: Re: [x86/signal] 3aac3ebea0: will-it-scale.per_thread_ops -11.9%
 regression

On Dec 7, 2021, at 15:14, Hansen, Dave <dave.hansen@...el.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 */