* [patch 0/2] nohz_full: only wakeup target CPUs when notifying new tick dependency (v3)
@ 2020-10-08 19:11 Marcelo Tosatti
2020-10-08 19:11 ` [patch 1/2] nohz: only wakeup a single target cpu when kicking a task Marcelo Tosatti
2020-10-08 19:11 ` [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks Marcelo Tosatti
0 siblings, 2 replies; 6+ messages in thread
From: Marcelo Tosatti @ 2020-10-08 19:11 UTC (permalink / raw)
To: linux-kernel
Cc: Frederic Weisbecker, Peter Zijlstra, Nitesh Narayan Lal, Peter Xu
When enabling per-CPU posix timers, an IPI to nohz_full CPUs might be
performed (to re-read the dependencies and possibly not re-enter
nohz_full on a given CPU).
A common case is for applications that run on nohz_full= CPUs
to not use POSIX timers (eg DPDK). This patch changes the notification
to only IPI the target CPUs where the task(s) whose tick dependencies
are being updated are executing.
This reduces interruptions to nohz_full= CPUs.
---
v3: replace superfluous rcu_read_lock with lockdep_assert (PeterZ)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 1/2] nohz: only wakeup a single target cpu when kicking a task
2020-10-08 19:11 [patch 0/2] nohz_full: only wakeup target CPUs when notifying new tick dependency (v3) Marcelo Tosatti
@ 2020-10-08 19:11 ` Marcelo Tosatti
2020-10-08 19:11 ` [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks Marcelo Tosatti
1 sibling, 0 replies; 6+ messages in thread
From: Marcelo Tosatti @ 2020-10-08 19:11 UTC (permalink / raw)
To: linux-kernel
Cc: Frederic Weisbecker, Peter Zijlstra, Nitesh Narayan Lal,
Peter Xu, Marcelo Tosatti
When adding a tick dependency to a task, its necessary to
wakeup the CPU where the task resides to reevaluate tick
dependencies on that CPU.
However the current code wakes up all nohz_full CPUs, which
is unnecessary.
Switch to waking up a single CPU, by using ordering of writes
to task->cpu and task->tick_dep_mask.
From: Frederic Weisbecker <frederic@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
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
@@ -274,6 +274,31 @@ void tick_nohz_full_kick_cpu(int cpu)
irq_work_queue_on(&per_cpu(nohz_full_kick_work, cpu), cpu);
}
+static void tick_nohz_kick_task(struct task_struct *tsk)
+{
+ int cpu = task_cpu(tsk);
+
+ /*
+ * If the task concurrently migrates to another cpu,
+ * we guarantee it sees the new tick dependency upon
+ * schedule.
+ *
+ *
+ * set_task_cpu(p, cpu);
+ * STORE p->cpu = @cpu
+ * __schedule() (switch to task 'p')
+ * LOCK rq->lock
+ * smp_mb__after_spin_lock() STORE p->tick_dep_mask
+ * tick_nohz_task_switch() smp_mb() (atomic_fetch_or())
+ * LOAD p->tick_dep_mask LOAD p->cpu
+ */
+
+ preempt_disable();
+ if (cpu_online(cpu))
+ tick_nohz_full_kick_cpu(cpu);
+ preempt_enable();
+}
+
/*
* Kick all full dynticks CPUs in order to force these to re-evaluate
* their dependency on the tick and restart it if necessary.
@@ -356,19 +381,8 @@ EXPORT_SYMBOL_GPL(tick_nohz_dep_clear_cp
*/
void tick_nohz_dep_set_task(struct task_struct *tsk, enum tick_dep_bits bit)
{
- if (!atomic_fetch_or(BIT(bit), &tsk->tick_dep_mask)) {
- if (tsk == current) {
- preempt_disable();
- tick_nohz_full_kick();
- preempt_enable();
- } else {
- /*
- * Some future tick_nohz_full_kick_task()
- * should optimize this.
- */
- tick_nohz_full_kick_all();
- }
- }
+ if (!atomic_fetch_or(BIT(bit), &tsk->tick_dep_mask))
+ tick_nohz_kick_task(tsk);
}
EXPORT_SYMBOL_GPL(tick_nohz_dep_set_task);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
2020-10-08 19:11 [patch 0/2] nohz_full: only wakeup target CPUs when notifying new tick dependency (v3) Marcelo Tosatti
2020-10-08 19:11 ` [patch 1/2] nohz: only wakeup a single target cpu when kicking a task Marcelo Tosatti
@ 2020-10-08 19:11 ` Marcelo Tosatti
1 sibling, 0 replies; 6+ messages in thread
From: Marcelo Tosatti @ 2020-10-08 19:11 UTC (permalink / raw)
To: linux-kernel
Cc: Frederic Weisbecker, Peter Zijlstra, Nitesh Narayan Lal,
Peter Xu, Marcelo Tosatti
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 <mtosatti@redhat.com>
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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
2020-10-08 12:35 ` Peter Zijlstra
@ 2020-10-08 18:04 ` Marcelo Tosatti
0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Tosatti @ 2020-10-08 18:04 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Frederic Weisbecker, Nitesh Narayan Lal, Peter Xu,
Oleg Nesterov
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 <mtosatti@redhat.com>
> >
> > 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
2020-10-07 18:01 ` [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks Marcelo Tosatti
@ 2020-10-08 12:35 ` Peter Zijlstra
2020-10-08 18:04 ` Marcelo Tosatti
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2020-10-08 12:35 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: linux-kernel, Frederic Weisbecker, Nitesh Narayan Lal, Peter Xu,
Oleg Nesterov
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 <mtosatti@redhat.com>
>
> 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);
?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks
2020-10-07 18:01 [patch 0/2] nohz_full: only wakeup target CPUs when notifying new tick dependency (v2) Marcelo Tosatti
@ 2020-10-07 18:01 ` Marcelo Tosatti
2020-10-08 12:35 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2020-10-07 18:01 UTC (permalink / raw)
To: linux-kernel
Cc: Frederic Weisbecker, Peter Zijlstra, Nitesh Narayan Lal,
Peter Xu, Marcelo Tosatti
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 <mtosatti@redhat.com>
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-08 19:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 19:11 [patch 0/2] nohz_full: only wakeup target CPUs when notifying new tick dependency (v3) Marcelo Tosatti
2020-10-08 19:11 ` [patch 1/2] nohz: only wakeup a single target cpu when kicking a task Marcelo Tosatti
2020-10-08 19:11 ` [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks Marcelo Tosatti
-- strict thread matches above, loose matches on Subject: below --
2020-10-07 18:01 [patch 0/2] nohz_full: only wakeup target CPUs when notifying new tick dependency (v2) Marcelo Tosatti
2020-10-07 18:01 ` [patch 2/2] nohz: change signal tick dependency to wakeup CPUs of member tasks Marcelo Tosatti
2020-10-08 12:35 ` Peter Zijlstra
2020-10-08 18:04 ` Marcelo Tosatti
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.