Messages in this thread | | | Date | Tue, 18 Oct 2016 12:12:40 -0400 | From | Waiman Long <> | Subject | Re: [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
| |