isolation: 1Hz residual tick offloading v4

When a CPU runs in full dynticks mode, a 1Hz tick remains in order to keep the scheduler stats alive. However this residual tick is a burden for bare metal tasks that can't stand any interruption at all, or want to minimize them.

The usual boot parameters "nohz_full=" or "isolcpus=nohz" will now outsource these scheduler ticks to the global workqueue so that a housekeeping CPU handles those remotely.

Note that in the case of using isolcpus, it's still up to the user to affine the global workqueues to the housekeeping CPUs through /sys/devices/virtual/workqueue/cpumask or domains isolation "isolcpus=nohz,domain".

kernel/sched/core.c +19 -1

index d72d0e9..c79500c 100644
@@ -3062,7 +3062,82 @@ u64 scheduler_tick_max_deferment(void)
 
 	return jiffies_to_nsecs(next - now);
 }
-#endif
+
+struct tick_work {
+	int			cpu;
+	struct delayed_work	work;
+};
+
+static struct tick_work __percpu *tick_work_cpu;
+
+static void sched_tick_remote(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct tick_work *twork = container_of(dwork, struct tick_work, work);
+	int cpu = twork->cpu;
+	struct rq *rq = cpu_rq(cpu);
+	struct rq_flags rf;
+
+	/*
+	 * Handle the tick only if it appears the remote CPU is running
+	 * in full dynticks mode. The check is racy by nature, but

Quoted 9 lines

When a CPU runs in full dynticks mode, a 1Hz tick remains in order to keep the scheduler stats alive. However this residual tick is a burden for bare metal tasks that can't stand any interruption at all, or want to minimize them.

The usual boot parameters "nohz_full=" or "isolcpus=nohz" will now outsource these scheduler ticks to the global workqueue so that a housekeeping CPU handles those remotely.

Quoted 4 lines

Note that in the case of using isolcpus, it's still up to the user to affine the global workqueues to the housekeeping CPUs through /sys/devices/virtual/workqueue/cpumask or domains isolation "isolcpus=nohz,domain".

I would very much like a few words on why sched_class::task_tick() is safe to call remote -- from a quick look I think it actually is, but it would be good to have some words here.

Quoted 35 lines

kernel/sched/core.c +25 -1

index d72d0e9..c79500c 100644
@@ -3062,7 +3062,82 @@ u64 scheduler_tick_max_deferment(void)
 
 	return jiffies_to_nsecs(next - now);
 }
-#endif
+
+struct tick_work {
+	int			cpu;
+	struct delayed_work	work;
+};
+
+static struct tick_work __percpu *tick_work_cpu;
+
+static void sched_tick_remote(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct tick_work *twork = container_of(dwork, struct tick_work, work);
+	int cpu = twork->cpu;
+	struct rq *rq = cpu_rq(cpu);
+	struct rq_flags rf;
+
+	/*
+	 * Handle the tick only if it appears the remote CPU is running
+	 * in full dynticks mode. The check is racy by nature, but
+	 * missing a tick or having one too much is no big deal.
+	 */
+	if (!idle_cpu(cpu) && tick_nohz_tick_stopped_cpu(cpu)) {
+		rq_lock_irq(rq, &rf);
+		update_rq_clock(rq);
+		rq->curr->sched_class->task_tick(rq, rq->curr, 0);

kernel/sched/core.c u64 scheduler_tick_max_deferment(void) · line 3093

Quoted 4 lines

kernel/sched/core.c +4 -0

+		rq_unlock_irq(rq, &rf);
+	}
+
+	queue_delayed_work(system_unbound_wq, dwork, HZ);

Do we want something that tracks the actual interrer arrival time of this work, such that we can detect and warn if the book-keeping thing is failing to keep up?

kernel/sched/core.c u64 scheduler_tick_max_deferment(void) · line 3101

Quoted 9 lines

kernel/sched/core.c +8 -0

+}
+
+static void sched_tick_start(int cpu)
+{
+	struct tick_work *twork;
+
+	if (housekeeping_cpu(cpu, HK_FLAG_TICK))
+		return;

This all looks very static :-(, you can't reconfigure this nohz_full crud after boot?

kernel/sched/core.c u64 scheduler_tick_max_deferment(void) · line 3109

Quoted 8 lines

kernel/sched/core.c +7 -0

+	WARN_ON_ONCE(!tick_work_cpu);
+
+	twork = per_cpu_ptr(tick_work_cpu, cpu);
+	twork->cpu = cpu;
+	INIT_DELAYED_WORK(&twork->work, sched_tick_remote);
+	queue_delayed_work(system_unbound_wq, &twork->work, HZ);
+}

Similarly, I think we want a few words about how unbound workqueues are expected to behave vs NUMA.

AFAICT unbound workqueues by default prefer to run on a cpu in the same node, but if no cpu is available, it doesn't go looking for the nearest node that does have a cpu, it just punts to whatever random cpu.

Quoted 3 lines · Peter Zijlstra

I would very much like a few words on why sched_class::task_tick() is safe to call remote -- from a quick look I think it actually is, but it would be good to have some words here.

Let's rather say I can't prove that it is safe, given the amount of code that is behind throughout the various flavour of scheduler features.

But as far as I checked several times, it seems that nothing is accessed locally on ::scheduler_tick(). Everything looks fetched from the runqueue struct target while it is locked.

If we ever find local references such as "current" or "__this_cpu_*" in the path, we'll have to fix them.

Quoted 9 lines

kernel/sched/core.c u64 scheduler_tick_max_deferment(void) · line 3101

Quoted 6 lines

kernel/sched/core.c +6 -0

+static void sched_tick_start(int cpu)
+{
+	struct tick_work *twork;
+
+	if (housekeeping_cpu(cpu, HK_FLAG_TICK))
+		return;

This all looks very static :-(, you can't reconfigure this nohz_full crud after boot?

Unfortunately yes. In fact making the nohz interface dynamically available through cpuset is the next big step.

Quoted 11 lines

kernel/sched/core.c u64 scheduler_tick_max_deferment(void) · line 3109

Quoted 7 lines

kernel/sched/core.c +7 -0

+	WARN_ON_ONCE(!tick_work_cpu);
+
+	twork = per_cpu_ptr(tick_work_cpu, cpu);
+	twork->cpu = cpu;
+	INIT_DELAYED_WORK(&twork->work, sched_tick_remote);
+	queue_delayed_work(system_unbound_wq, &twork->work, HZ);
+}

Similarly, I think we want a few words about how unbound workqueues are expected to behave vs NUMA.

Quoted 3 lines

AFAICT unbound workqueues by default prefer to run on a cpu in the same node, but if no cpu is available, it doesn't go looking for the nearest node that does have a cpu, it just punts to whatever random cpu.

Yes, and in fact you just made me look into wq_select_unbound_cpu() and it looks worse than that. If the current CPU is not in the wq_unbound_cpumask, a random one is picked up from that global cpumask without trying a near one in the current node.

Looks like room for improvement on the workqueue side. I'll see what I can do.

Ingo,

Please pull the sched/0hz-v2 branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git

	sched/0hz-v2

HEAD: 9b14d5204490f9acd03998a5e406ecadb87cddba

Changes in v4:

* Remove the nohz_offload option, just stick with the existing interface, the change is transparent. Suggested by Luiz.

* Automatically pin workqueues to housekeepers.

---
Now that scheduler_tick() has become resilient towards the absence of ticks, current->sched_class->task_tick() is the last piece that needs at least 1Hz tick to keep scheduler stats alive.

This patchset offloads this residual 1Hz tick to workqueues. This way the nohz full CPUs don't have anymore tick (assuming nothing else requires it) as their residual 1Hz tick get handled by the housekeepers.

Nothing special is required for testing, just use the usual kernel parameters, say on CPUs 1-7:

        "nohz_full=1-7"
or
        "isolcpus=nohz_offload,domain,1-7"

Thanks,
Frederic
---

Frederic Weisbecker (6):
sched: Rename init_rq_hrtick to hrtick_rq_init
nohz: Allow to check if remote CPU tick is stopped sched/isolation: Isolate workqueues when "nohz_full=" is set sched/isolation: Residual 1Hz scheduler tick offload sched/nohz: Remove the 1 Hz tick code sched/isolation: Tick offload documentation

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/sched/isolation.h                 |   1 +
 include/linux/sched/nohz.h                      |   4 -
 include/linux/tick.h                            |   2 +
 kernel/sched/core.c                             | 104 +++++++++++++++++-------
 kernel/sched/idle_task.c                        |   1 -
 kernel/sched/isolation.c                        |   8 +-
 kernel/sched/sched.h                            |  13 +--
 kernel/time/tick-sched.c                        |  13 +--
 kernel/workqueue.c                              |   3 +-
 10 files changed, 103 insertions(+), 52 deletions(-)

Quoted 7 lines · On Fri, 19 Jan 2018 01:02:14 +0100 Frederic Weisbecker <[email protected]>…

Ingo,

Please pull the sched/0hz-v2 branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git

	sched/0hz-v2

Quoted 8 lines

HEAD: 9b14d5204490f9acd03998a5e406ecadb87cddba

Changes in v4:

* Remove the nohz_offload option, just stick with the existing interface, the change is transparent. Suggested by Luiz.

* Automatically pin workqueues to housekeepers.

I've been testing this series and the tick doesn't go completely away for me: it ticks at around 8 seconds interval.

I've debugged this down to the clocksource_watchdog() timer, which is created by clocksource_start_watchdog(). This timer cycles over all online CPUs. I couldn't find a way to disable it. It seems to be always enabled for x86 by CONFIG_CLOCKSOURCE_WATCHDOG since commit 6471b825c4.

Since the 1Hz tick offload worked for you, I must be missing a way to disable this timer or the kernel is thinking my CPU has unstable TSC (which it doesn't AFAIK).

Quoted 22 lines · Luiz Capitulino

Quoted 7 lines · On Fri, 19 Jan 2018 01:02:14 +0100 Frederic Weisbecker <[email protected]>…

Ingo,

Please pull the sched/0hz-v2 branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git

	sched/0hz-v2

Quoted 8 lines

HEAD: 9b14d5204490f9acd03998a5e406ecadb87cddba

Changes in v4:

* Remove the nohz_offload option, just stick with the existing interface, the change is transparent. Suggested by Luiz.

* Automatically pin workqueues to housekeepers.

I've been testing this series and the tick doesn't go completely away for me: it ticks at around 8 seconds interval.

Quoted 8 lines

I've debugged this down to the clocksource_watchdog() timer, which is created by clocksource_start_watchdog(). This timer cycles over all online CPUs. I couldn't find a way to disable it. It seems to be always enabled for x86 by CONFIG_CLOCKSOURCE_WATCHDOG since commit 6471b825c4.

Since the 1Hz tick offload worked for you, I must be missing a way to disable this timer or the kernel is thinking my CPU has unstable TSC (which it doesn't AFAIK).

It's beyond the scope of this patchset but indeed that's right, I run my kernels with tsc=reliable because my CPUs don't have the TSC_RELIABLE flag. That's the only way I found to shutdown the tick completely on my test machine, otherwise I keep having that clocksource watchdog.

You can try "tsc=reliable" but that's at your own risks and it's hard to tell what exactly are those risks depending on your CPU model (and perhaps BIOS?).

You likely already had that watchdog timer before this patchset but didn't notice because the 1Hz was a more frequent annoyance.

Cool, passing tsc=reliable worked for me. I finally got to the tick to go completely away. While I agree that fixing that is beyond the scope of this series, I think we should improve it anyway since it will probably come up for people trying the new nohz_full=.

Quoted 4 lines · Luiz Capitulino

Cool, passing tsc=reliable worked for me. I finally got to the tick to go completely away. While I agree that fixing that is beyond the scope of this series, I think we should improve it anyway since it will probably come up for people trying the new nohz_full=.

The only way to fix that is to audit all BIOS code :/ Short of that, we need to periodically test the TSC on each CPU/SOCKET to verify its still in step with expectation.

Sadly MSR_IA32_TSC and MSR_IA32_TSC_ADJUST are writable registers and BIOS monkeys occasionally write to them for 'raisins-of-insanity'.

It's beyond the scope of this patchset but indeed that's right, I run my kernels with tsc=reliable because my CPUs don't have the TSC_RELIABLE flag. That's the only way I found to shutdown the tick completely on my test machine, otherwise I keep having that clocksource watchdog.

You can try "tsc=reliable" but that's at your own risks and it's hard to tell what exactly are those risks depending on your CPU model (and perhaps BIOS?).

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line · Frederic Weisbecker

On Wed, Jan 24, 2018 at 10:46:08AM -0500, Luiz Capitulino wrote:

Quoted 3 lines

Since the 1Hz tick offload worked for you, I must be missing a way to disable this timer or the kernel is thinking my CPU has unstable TSC (which it doesn't AFAIK).

It's beyond the scope of this patchset but indeed that's right, I run my kernels with tsc=reliable because my CPUs don't have the TSC_RELIABLE flag. That's the only way I found to shutdown the tick completely on my test machine, otherwise I keep having that clocksource watchdog.

Hi, Frederic!

Quoted 2 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line · Frederic Weisbecker

On Wed, Jan 24, 2018 at 10:46:08AM -0500, Luiz Capitulino wrote:

[...]

Quoted 9 lines

Quoted 3 lines

Since the 1Hz tick offload worked for you, I must be missing a way to disable this timer or the kernel is thinking my CPU has unstable TSC (which it doesn't AFAIK).

It's beyond the scope of this patchset but indeed that's right, I run my kernels with tsc=reliable because my CPUs don't have the TSC_RELIABLE flag. That's the only way I found to shutdown the tick completely on my test machine, otherwise I keep having that clocksource watchdog.

[...]

Thanks, it helps. But I have accounting problem:

if I run user busy loop on the nohz cpu, the task accounting works correctly (top shows the task takes 100% cpu), but cpu accounting is wrong (cpu is 100% idle, in the per-core view as well).

If I understand correctly, the stats are updated by account_user_time() -> task_group_account_field() but there is no call for it in case of offloading (it is called from irqtime_account_process_tick, account_process_tick, vtime_user_exit).

Moreover, task_group_account_field() uses __this_cpu_add() which will be wrong for offloading.

For testing I used kcpustat_cpu(task_cpu(p)) in task_group_account_field() and added call account_user_time(curr, delta) to the sched_tick_remote() what fixes it for me, but what would be the proper fix?

Quoted 21 lines · Yauheni Kaliuta

Hi, Frederic!

Quoted 2 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line · Frederic Weisbecker

On Wed, Jan 24, 2018 at 10:46:08AM -0500, Luiz Capitulino wrote:

[...]

Quoted 9 lines

Quoted 3 lines

Since the 1Hz tick offload worked for you, I must be missing a way to disable this timer or the kernel is thinking my CPU has unstable TSC (which it doesn't AFAIK).

It's beyond the scope of this patchset but indeed that's right, I run my kernels with tsc=reliable because my CPUs don't have the TSC_RELIABLE flag. That's the only way I found to shutdown the tick completely on my test machine, otherwise I keep having that clocksource watchdog.

[...]

Thanks, it helps. But I have accounting problem:

Quoted 8 lines

if I run user busy loop on the nohz cpu, the task accounting works correctly (top shows the task takes 100% cpu), but cpu accounting is wrong (cpu is 100% idle, in the per-core view as well).

If I understand correctly, the stats are updated by account_user_time() -> task_group_account_field() but there is no call for it in case of offloading (it is called from irqtime_account_process_tick, account_process_tick, vtime_user_exit).

Ah I forgot about kcpustat accounting. I remember I wanted to fix that a few years ago but I forgot about it when I removed the last tick. That thing was lurking behind 1Hz.

Quoted 8 lines

Moreover, task_group_account_field() uses __this_cpu_add() which will be wrong for offloading.

For testing I used kcpustat_cpu(task_cpu(p)) in task_group_account_field() and added call account_user_time(curr, delta) to the sched_tick_remote() what fixes it for me, but what would be the proper fix?

Yeah unfortunately that's unsafe. Task accounting is not designed for remote update. You could race with an update from another CPU, especially the local updater.

I fear we need to take the same approach than task cputime, which is using a seqcount for updates. Then the reader would fetch the kcpustat values + the delta vtime from the task executing.

Things can get complicated once we dive into corner cases: CPUTIME_IRQ, CPUTIME_SOFTIRQ, and CPUTIME_STEAL. At least we don't need to care about CPUTIME_IDLE and CPUTIME_IOWAIT that have their own delta.

I'm trying that.

Ingo,

Please pull the sched/0hz-v2 branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git

	sched/0hz-v2

HEAD: 9b14d5204490f9acd03998a5e406ecadb87cddba

Changes in v4:

* Remove the nohz_offload option, just stick with the existing interface, the change is transparent. Suggested by Luiz.

* Automatically pin workqueues to housekeepers.

---
Now that scheduler_tick() has become resilient towards the absence of ticks, current->sched_class->task_tick() is the last piece that needs at least 1Hz tick to keep scheduler stats alive.

This patchset offloads this residual 1Hz tick to workqueues. This way the nohz full CPUs don't have anymore tick (assuming nothing else requires it) as their residual 1Hz tick get handled by the housekeepers.

Nothing special is required for testing, just use the usual kernel parameters, say on CPUs 1-7:

        "nohz_full=1-7"
or
        "isolcpus=nohz_offload,domain,1-7"

Thanks,
Frederic
---

Frederic Weisbecker (6):
sched: Rename init_rq_hrtick to hrtick_rq_init
nohz: Allow to check if remote CPU tick is stopped sched/isolation: Isolate workqueues when "nohz_full=" is set sched/isolation: Residual 1Hz scheduler tick offload sched/nohz: Remove the 1 Hz tick code sched/isolation: Tick offload documentation

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/sched/isolation.h                 |   1 +
 include/linux/sched/nohz.h                      |   4 -
 include/linux/tick.h                            |   2 +
 kernel/sched/core.c                             | 104 +++++++++++++++++-------
 kernel/sched/idle_task.c                        |   1 -
 kernel/sched/isolation.c                        |   8 +-
 kernel/sched/sched.h                            |  13 +--
 kernel/time/tick-sched.c                        |  13 +--
 kernel/workqueue.c                              |   3 +-
 10 files changed, 103 insertions(+), 52 deletions(-)