[patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
Rather than waking up all nohz_full CPUs on the system, only wakeup the target CPUs of member threads of the signal. Reduces interruptions to nohz_full CPUs. Signed-off-by: Marcelo Tosatti Index: linux-2.6/kernel/time/tick-sched.c === --- linux-2.6.orig/kernel/time/tick-sched.c +++ linux-2.6/kernel/time/tick-sched.c @@ -396,9 +396,17 @@ EXPORT_SYMBOL_GPL(tick_nohz_dep_clear_ta * Set a per-taskgroup tick dependency. Posix CPU timers need this in order to elapse * per process timers. */ -void tick_nohz_dep_set_signal(struct signal_struct *sig, enum tick_dep_bits bit) +void tick_nohz_dep_set_signal(struct task_struct *tsk, enum tick_dep_bits bit) { - tick_nohz_dep_set_all(&sig->tick_dep_mask, bit); + int prev; + struct task_struct *p; + + prev = atomic_fetch_or(BIT(bit), &tsk->signal->tick_dep_mask); + if (!prev) { + lockdep_assert_held(&tsk->sighand->siglock); + for_each_thread(tsk, p) + tick_nohz_kick_task(p); + } } void tick_nohz_dep_clear_signal(struct signal_struct *sig, enum tick_dep_bits bit) Index: linux-2.6/include/linux/tick.h === --- linux-2.6.orig/include/linux/tick.h +++ linux-2.6/include/linux/tick.h @@ -207,7 +207,7 @@ extern void tick_nohz_dep_set_task(struc enum tick_dep_bits bit); extern void tick_nohz_dep_clear_task(struct task_struct *tsk, enum tick_dep_bits bit); -extern void tick_nohz_dep_set_signal(struct signal_struct *signal, +extern void tick_nohz_dep_set_signal(struct task_struct *tsk, enum tick_dep_bits bit); extern void tick_nohz_dep_clear_signal(struct signal_struct *signal, enum tick_dep_bits bit); @@ -252,11 +252,11 @@ static inline void tick_dep_clear_task(s if (tick_nohz_full_enabled()) tick_nohz_dep_clear_task(tsk, bit); } -static inline void tick_dep_set_signal(struct signal_struct *signal, +static inline void tick_dep_set_signal(struct task_struct *tsk, enum tick_dep_bits bit) { if (tick_nohz_full_enabled()) - tick_nohz_dep_set_signal(signal, bit); + tick_nohz_dep_set_signal(tsk, bit); } static inline void tick_dep_clear_signal(struct signal_struct *signal, enum tick_dep_bits bit) @@ -284,7 +284,7 @@ static inline void tick_dep_set_task(str enum tick_dep_bits bit) { } static inline void tick_dep_clear_task(struct task_struct *tsk, enum tick_dep_bits bit) { } -static inline void tick_dep_set_signal(struct signal_struct *signal, +static inline void tick_dep_set_signal(struct task_struct *tsk, enum tick_dep_bits bit) { } static inline void tick_dep_clear_signal(struct signal_struct *signal, enum tick_dep_bits bit) { } Index: linux-2.6/kernel/time/posix-cpu-timers.c === --- linux-2.6.orig/kernel/time/posix-cpu-timers.c +++ linux-2.6/kernel/time/posix-cpu-timers.c @@ -523,7 +523,7 @@ static void arm_timer(struct k_itimer *t if (CPUCLOCK_PERTHREAD(timer->it_clock)) tick_dep_set_task(p, TICK_DEP_BIT_POSIX_TIMER); else - tick_dep_set_signal(p->signal, TICK_DEP_BIT_POSIX_TIMER); + tick_dep_set_signal(p, TICK_DEP_BIT_POSIX_TIMER); } /* @@ -1358,7 +1358,7 @@ void set_process_cpu_timer(struct task_s if (*newval < *nextevt) *nextevt = *newval; - tick_dep_set_signal(tsk->signal, TICK_DEP_BIT_POSIX_TIMER); + tick_dep_set_signal(tsk, TICK_DEP_BIT_POSIX_TIMER); } static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
Re: [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
On Thu, Oct 08, 2020 at 02:35:44PM +0200, Peter Zijlstra wrote: > On Wed, Oct 07, 2020 at 03:01:53PM -0300, Marcelo Tosatti wrote: > > Rather than waking up all nohz_full CPUs on the system, only wakeup > > the target CPUs of member threads of the signal. > > > > Reduces interruptions to nohz_full CPUs. > > > > Signed-off-by: Marcelo Tosatti > > > > Index: linux-2.6/kernel/time/tick-sched.c > > === > > --- linux-2.6.orig/kernel/time/tick-sched.c > > +++ linux-2.6/kernel/time/tick-sched.c > > @@ -398,7 +398,15 @@ EXPORT_SYMBOL_GPL(tick_nohz_dep_clear_ta > > */ > > void tick_nohz_dep_set_signal(struct signal_struct *sig, enum > > tick_dep_bits bit) > > { > > - tick_nohz_dep_set_all(&sig->tick_dep_mask, bit); > > + int prev; > > + > > + prev = atomic_fetch_or(BIT(bit), &sig->tick_dep_mask); > > + if (!prev) { > > + rcu_read_lock(); > > + for_each_thread(sig, t) > > + tick_nohz_kick_task(t); > > + rcu_read_unlock(); > > + } > > } > > AFAICT, and this makes perfect sense, this function is only ever used > while holding sighand->siglock, which makes the RCU read lock > superfluous. > > Would it make sense to change the signal_struct argument to task_struct, > such that we can write: > > lockdep_assert_held(&p->sighand->siglock); > for_each_thread(p->signal, t) > tick_nohz_kick_task(t); > > ? Makes sense, resending -v3.
Re: [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
On Wed, Oct 07, 2020 at 03:01:53PM -0300, Marcelo Tosatti wrote: > Rather than waking up all nohz_full CPUs on the system, only wakeup > the target CPUs of member threads of the signal. > > Reduces interruptions to nohz_full CPUs. > > Signed-off-by: Marcelo Tosatti > > Index: linux-2.6/kernel/time/tick-sched.c > === > --- linux-2.6.orig/kernel/time/tick-sched.c > +++ linux-2.6/kernel/time/tick-sched.c > @@ -398,7 +398,15 @@ EXPORT_SYMBOL_GPL(tick_nohz_dep_clear_ta > */ > void tick_nohz_dep_set_signal(struct signal_struct *sig, enum tick_dep_bits > bit) > { > - tick_nohz_dep_set_all(&sig->tick_dep_mask, bit); > + int prev; > + > + prev = atomic_fetch_or(BIT(bit), &sig->tick_dep_mask); > + if (!prev) { > + rcu_read_lock(); > + for_each_thread(sig, t) > + tick_nohz_kick_task(t); > + rcu_read_unlock(); > + } > } AFAICT, and this makes perfect sense, this function is only ever used while holding sighand->siglock, which makes the RCU read lock superfluous. Would it make sense to change the signal_struct argument to task_struct, such that we can write: lockdep_assert_held(&p->sighand->siglock); for_each_thread(p->signal, t) tick_nohz_kick_task(t); ?
[patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
Rather than waking up all nohz_full CPUs on the system, only wakeup the target CPUs of member threads of the signal. Reduces interruptions to nohz_full CPUs. Signed-off-by: Marcelo Tosatti Index: linux-2.6/kernel/time/tick-sched.c === --- linux-2.6.orig/kernel/time/tick-sched.c +++ linux-2.6/kernel/time/tick-sched.c @@ -398,7 +398,15 @@ EXPORT_SYMBOL_GPL(tick_nohz_dep_clear_ta */ void tick_nohz_dep_set_signal(struct signal_struct *sig, enum tick_dep_bits bit) { - tick_nohz_dep_set_all(&sig->tick_dep_mask, bit); + int prev; + + prev = atomic_fetch_or(BIT(bit), &sig->tick_dep_mask); + if (!prev) { + rcu_read_lock(); + for_each_thread(sig, t) + tick_nohz_kick_task(t); + rcu_read_unlock(); + } } void tick_nohz_dep_clear_signal(struct signal_struct *sig, enum tick_dep_bits bit)