lkml.org 
[lkml]   [2016]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] signals: Avoid unnecessary taking of sighand->siglock
On 09/27/2016 08:26 AM, Waiman Long wrote:
> When running certain database workload on a high-end system with many
> CPUs, it was found that spinlock contention in the sigprocmask syscalls
> became a significant portion of the overall CPU cycles as shown below.
>
> 9.30% 9.30% 905387 dataserver /proc/kcore 0x7fff8163f4d2
> [k] _raw_spin_lock_irq
> |
> ---_raw_spin_lock_irq
> |
> |--99.34%-- __set_current_blocked
> | sigprocmask
> | sys_rt_sigprocmask
> | system_call_fastpath
> | |
> | |--50.63%-- __swapcontext
> | | |
> | | |--99.91%-- upsleepgeneric
> | |
> | |--49.36%-- __setcontext
> | | ktskRun
>
> Looking further into the swapcontext function in glibc, it was found
> that the function always call sigprocmask() without checking if there
> are changes in the signal mask.
>
> A check was added to the __set_current_blocked() function to avoid
> taking the sighand->siglock spinlock if there is no change in the
> signal mask. This will prevent unneeded spinlock contention when many
> threads are trying to call sigprocmask().
>
> With this patch applied, the spinlock contention in sigprocmask() was
> gone.
>
> Signed-off-by: Waiman Long<Waiman.Long@hpe.com>
> ---
> v2->v3:
> - Add a sigequalsets() helper in signal.h and use it for comparison.
>
> v1->v2:
> - Fix compiler warning in mips.
>
> include/linux/signal.h | 17 +++++++++++++++++
> kernel/signal.c | 7 +++++++
> 2 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index b63f63e..5308304 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -97,6 +97,23 @@ static inline int sigisemptyset(sigset_t *set)
> }
> }
>
> +static inline int sigequalsets(const sigset_t *set1, const sigset_t *set2)
> +{
> + switch (_NSIG_WORDS) {
> + case 4:
> + return (set1->sig[3] == set2->sig[3])&&
> + (set1->sig[2] == set2->sig[2])&&
> + (set1->sig[1] == set2->sig[1])&&
> + (set1->sig[0] == set2->sig[0]);
> + case 2:
> + return (set1->sig[1] == set2->sig[1])&&
> + (set1->sig[0] == set2->sig[0]);
> + case 1:
> + return set1->sig[0] == set2->sig[0];
> + }
> + return 0;
> +}
> +
> #define sigmask(sig) (1UL<< ((sig) - 1))
>
> #ifndef __HAVE_ARCH_SIG_SETOPS
> diff --git a/kernel/signal.c b/kernel/signal.c
> index af21afc..04e8f50 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2485,6 +2485,13 @@ void __set_current_blocked(const sigset_t *newset)
> {
> struct task_struct *tsk = current;
>
> + /*
> + * In case the signal mask hasn't changed, there is nothing we need
> + * to do. The current->blocked shouldn't be modified by other task.
> + */
> + if (sigequalsets(&tsk->blocked, newset))
> + return;
> +
> spin_lock_irq(&tsk->sighand->siglock);
> __set_task_blocked(tsk, newset);
> spin_unlock_irq(&tsk->sighand->siglock);

This is a pretty simple patch. Is that a chance that it can be pulled
into 4.10?

Cheers,
Longman

\
 
 \ /
  Last update: 2016-10-18 18:13    [W:0.365 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site