Skip to contentSkip to conversation

Introduce housekeeping subsystem

RFC PATCH In review Frederic Weisbecker 6 participants 33 messages Jul 21, 2017 – Aug 16, 2017

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Introduce housekeeping subsystem

I'm leaving for two weeks so this is food for thoughts in the meantime :)

We have a design issue with nohz_full: it drives the isolation features through the *housekeeping*() functions: kthreads, unpinned timers, watchdog, ...

But things should work the other way around because the tick is just an isolation feature among others.

So we need a housekeeping subsystem to drive all these isolation features, including nohz full in a later iteration. For now this is a basic draft. In the long run this subsystem should also drive the tick offloading (remove residual 1Hz) and all unbound kthreads.

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

HEAD: 68e3af1de5db228bf6c2a5e721bce59a02cfc4e1

Thanks,
Frederic
---

Frederic Weisbecker (9):
housekeeping: Move housekeeping related code to its own file
watchdog: Use housekeeping_cpumask() instead of ad-hoc version
housekeeping: Provide a dynamic off-case to housekeeping_any_cpu()
housekeeping: Make housekeeping cpumask private
housekeeping: Use its own static key
housekeeping: Rename is_housekeeping_cpu to housekeeping_cpu
housekeeping: Use own boot option, independant from nohz
housekeeping: Move it under own config, independant from NO_HZ
workqueue: Affine unbound workqueues to housekeeping cpumask

 drivers/net/ethernet/tile/tilegx.c |  2 +-
 include/linux/housekeeping.h       | 39 +++++++++++++++++++++++
 include/linux/tick.h               | 37 ----------------------
 init/Kconfig                       |  6 ++++
 kernel/Makefile                    |  1 +
 kernel/housekeeping.c              | 65 ++++++++++++++++++++++++++++++++++++++
 kernel/rcu/tree_plugin.h           |  1 +
 kernel/rcu/update.c                |  1 +
 kernel/sched/core.c                |  7 ++--
 kernel/sched/fair.c                |  3 +-
 kernel/time/tick-sched.c           | 18 -----------
 kernel/watchdog.c                  | 12 +++----
 kernel/workqueue.c                 |  3 +-
 13 files changed, 126 insertions(+), 69 deletions(-)
#1 lore raw

RFC PATCH 1/9 housekeeping: Move housekeeping related code to its own file

The housekeeping code is currently tied to the nohz code. As we are planning to make housekeeping independant from it, start with moving the relevant code to its own file.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 drivers/net/ethernet/tile/tilegx.c |  2 +-
 include/linux/housekeeping.h       | 56 ++++++++++++++++++++++++++++++++++++++
 include/linux/tick.h               | 37 -------------------------
 init/main.c                        |  2 ++
 kernel/Makefile                    |  1 +
 kernel/housekeeping.c              | 32 ++++++++++++++++++++++
 kernel/rcu/tree_plugin.h           |  1 +
 kernel/rcu/update.c                |  1 +
 kernel/sched/core.c                |  1 +
 kernel/sched/fair.c                |  1 +
 kernel/time/tick-sched.c           | 18 ------------
 kernel/watchdog.c                  |  1 +
 12 files changed, 97 insertions(+), 56 deletions(-)
 create mode 100644 include/linux/housekeeping.h
 create mode 100644 kernel/housekeeping.c
drivers/net/ethernet/tile/tilegx.c +1 -1
index aec9538..eb74e09 100644
@@ -40,7 +40,7 @@
 #include <linux/tcp.h>
 #include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
-#include <linux/tick.h>
+#include <linux/housekeeping.h>
 
 #include <asm/checksum.h>
 #include <asm/homecache.h>
include/linux/housekeeping.h new file +56 -0

56 lines not on this page · raw message

include/linux/tick.h +0 -37
index fe01e68..68afc09 100644
@@ -137,7 +137,6 @@ static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) { return -1; }
 #ifdef CONFIG_NO_HZ_FULL
 extern bool tick_nohz_full_running;
 extern cpumask_var_t tick_nohz_full_mask;
-extern cpumask_var_t housekeeping_mask;
 
 static inline bool tick_nohz_full_enabled(void)
 {
@@ -161,11 +160,6 @@ static inline void tick_nohz_full_add_cpus_to(struct cpumask *mask)
 		cpumask_or(mask, mask, tick_nohz_full_mask);
 }
 
-static inline int housekeeping_any_cpu(void)
-{
-	return cpumask_any_and(housekeeping_mask, cpu_online_mask);
-}
-
 extern void tick_nohz_dep_set(enum tick_dep_bits bit);
 extern void tick_nohz_dep_clear(enum tick_dep_bits bit);
 extern void tick_nohz_dep_set_cpu(int cpu, enum tick_dep_bits bit);
@@ -235,10 +229,6 @@ static inline void tick_dep_clear_signal(struct signal_struct *signal,
 extern void tick_nohz_full_kick_cpu(int cpu);
 extern void __tick_nohz_task_switch(void);
 #else
-static inline int housekeeping_any_cpu(void)
-{
-	return smp_processor_id();
-}
 static inline bool tick_nohz_full_enabled(void) { return false; }
 static inline bool tick_nohz_full_cpu(int cpu) { return false; }
 static inline void tick_nohz_full_add_cpus_to(struct cpumask *mask) { }
@@ -260,33 +250,6 @@ static inline void tick_nohz_full_kick_cpu(int cpu) { }
 static inline void __tick_nohz_task_switch(void) { }
 #endif
 
-static inline const struct cpumask *housekeeping_cpumask(void)
-{
-#ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
-		return housekeeping_mask;
-#endif
-	return cpu_possible_mask;
-}
-
-static inline bool is_housekeeping_cpu(int cpu)
-{
-#ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
-		return cpumask_test_cpu(cpu, housekeeping_mask);
-#endif
-	return true;
-}
-
-static inline void housekeeping_affine(struct task_struct *t)
-{
-#ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
-		set_cpus_allowed_ptr(t, housekeeping_mask);
-
-#endif
-}
-
 static inline void tick_nohz_task_switch(void)
 {
 	if (tick_nohz_full_enabled())
init/main.c +2 -0
index 9789ab7..9904a1e 100644
@@ -46,6 +46,7 @@
 #include <linux/cgroup.h>
 #include <linux/efi.h>
 #include <linux/tick.h>
+#include <linux/housekeeping.h>
 #include <linux/interrupt.h>
 #include <linux/taskstats_kern.h>
 #include <linux/delayacct.h>
@@ -607,6 +608,7 @@ asmlinkage __visible void __init start_kernel(void)
 	early_irq_init();
 	init_IRQ();
 	tick_init();
+	housekeeping_init();
 	rcu_init_nohz();
 	init_timers();
 	hrtimers_init();
kernel/Makefile +1 -0
index 4cb8e8b..8a85c4b 100644
@@ -109,6 +109,7 @@ obj-$(CONFIG_JUMP_LABEL) += jump_label.o
 obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
 obj-$(CONFIG_TORTURE_TEST) += torture.o
 obj-$(CONFIG_MEMBARRIER) += membarrier.o
+obj-$(CONFIG_NO_HZ_FULL) += housekeeping.o
 
 obj-$(CONFIG_HAS_IOMEM) += memremap.o
 
kernel/housekeeping.c new file +32 -0
new file mode 100644
index 0000000..6d8afd5
@@ -0,0 +1,32 @@
+/*
+ *  Housekeeping management. Manage the targets for routine code that can run on
+ *  any CPU: unbound workqueues, timers, kthreads and any offloadable work.
+ *
+ * Copyright (C) 2017 Red Hat, Inc., Frederic Weisbecker
+ *
+ */
+
+#include <linux/housekeeping.h>
+#include <linux/tick.h>
+#include <linux/init.h>
+
+cpumask_var_t housekeeping_mask;
+
+void __init housekeeping_init(void)
+{
+	if (!tick_nohz_full_enabled())
+		return;
+
+	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
+		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
+		cpumask_clear(tick_nohz_full_mask);
+		tick_nohz_full_running = false;
+		return;
+	}
+
+	cpumask_andnot(housekeeping_mask,
+		       cpu_possible_mask, tick_nohz_full_mask);
+
+	/* We need at least one CPU to handle housekeeping work */
+	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
+}
kernel/rcu/tree_plugin.h +1 -0
index 908b309..c66d162 100644
@@ -29,6 +29,7 @@
 #include <linux/oom.h>
 #include <linux/sched/debug.h>
 #include <linux/smpboot.h>
+#include <linux/housekeeping.h>
 #include <uapi/linux/sched/types.h>
 #include "../time/tick-internal.h"
 
kernel/rcu/update.c +1 -0
index 00e77c4..bfe973d 100644
@@ -51,6 +51,7 @@
 #include <linux/kthread.h>
 #include <linux/tick.h>
 #include <linux/rcupdate_wait.h>
+#include <linux/housekeeping.h>
 
 #define CREATE_TRACE_POINTS
 
kernel/sched/core.c +1 -0
index d3d39a2..751c040 100644
@@ -26,6 +26,7 @@
 #include <linux/profile.h>
 #include <linux/security.h>
 #include <linux/syscalls.h>
+#include <linux/housekeeping.h>
 
 #include <asm/switch_to.h>
 #include <asm/tlb.h>
kernel/sched/fair.c +1 -0
index c95880e..05dfced 100644
@@ -32,6 +32,7 @@
 #include <linux/mempolicy.h>
 #include <linux/migrate.h>
 #include <linux/task_work.h>
+#include <linux/housekeeping.h>
 
 #include <trace/events/sched.h>
 
kernel/time/tick-sched.c +0 -18
index c7a899c..9d29dee 100644
@@ -165,7 +165,6 @@ static void tick_sched_handle(struct tick_sched *ts, struct pt_regs *regs)
 
 #ifdef CONFIG_NO_HZ_FULL
 cpumask_var_t tick_nohz_full_mask;
-cpumask_var_t housekeeping_mask;
 bool tick_nohz_full_running;
 static atomic_t tick_dep_mask;
 
@@ -437,13 +436,6 @@ void __init tick_nohz_init(void)
 			return;
 	}
 
-	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
-		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
-		cpumask_clear(tick_nohz_full_mask);
-		tick_nohz_full_running = false;
-		return;
-	}
-
 	/*
 	 * Full dynticks uses irq work to drive the tick rescheduling on safe
 	 * locking contexts. But then we need irq work to raise its own
@@ -452,7 +444,6 @@ void __init tick_nohz_init(void)
 	if (!arch_irq_work_has_interrupt()) {
 		pr_warn("NO_HZ: Can't run full dynticks because arch doesn't support irq work self-IPIs\n");
 		cpumask_clear(tick_nohz_full_mask);
-		cpumask_copy(housekeeping_mask, cpu_possible_mask);
 		tick_nohz_full_running = false;
 		return;
 	}
@@ -465,9 +456,6 @@ void __init tick_nohz_init(void)
 		cpumask_clear_cpu(cpu, tick_nohz_full_mask);
 	}
 
-	cpumask_andnot(housekeeping_mask,
-		       cpu_possible_mask, tick_nohz_full_mask);
-
 	for_each_cpu(cpu, tick_nohz_full_mask)
 		context_tracking_cpu_set(cpu);
 
@@ -477,12 +465,6 @@ void __init tick_nohz_init(void)
 	WARN_ON(ret < 0);
 	pr_info("NO_HZ: Full dynticks CPUs: %*pbl.\n",
 		cpumask_pr_args(tick_nohz_full_mask));
-
-	/*
-	 * We need at least one CPU to handle housekeeping work such
-	 * as timekeeping, unbound timers, workqueues, ...
-	 */
-	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
 }
 #endif
 
kernel/watchdog.c +1 -0
index 06d3389..7a9df162 100644
@@ -24,6 +24,7 @@
 #include <linux/workqueue.h>
 #include <linux/sched/clock.h>
 #include <linux/sched/debug.h>
+#include <linux/housekeeping.h>
 
 #include <asm/irq_regs.h>
 #include <linux/kvm_para.h>
Signature · 1 line
2.7.4
#2 lore raw

RFC PATCH 2/9 watchdog: Use housekeeping_cpumask() instead of ad-hoc version

In reply to Frederic Weisbecker

While trying to disable the watchog on nohz_full CPUs, the watchdog implements an ad-hoc version of housekeeping_cpumask(). Lets replace those re-invented lines.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 kernel/watchdog.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)
kernel/watchdog.c +3 -8
index 7a9df162..cdd0d11 100644
@@ -941,15 +941,10 @@ void __init lockup_detector_init(void)
 {
 	set_sample_period();
 
-#ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled()) {
+	if (tick_nohz_full_enabled())
 		pr_info("Disabling watchdog on nohz_full cores by default\n");
-		cpumask_copy(&watchdog_cpumask, housekeeping_mask);
-	} else
-		cpumask_copy(&watchdog_cpumask, cpu_possible_mask);
-#else
-	cpumask_copy(&watchdog_cpumask, cpu_possible_mask);
-#endif
+
+	cpumask_copy(&watchdog_cpumask, housekeeping_cpumask());
 
 	if (watchdog_enabled)
 		watchdog_enable_all_cpus();
Signature · 1 line
2.7.4
#3 lore raw

RFC PATCH 3/9 housekeeping: Provide a dynamic off-case to housekeeping_any_cpu()

In reply to Frederic Weisbecker

housekeeping_any_cpu() doesn't handle correctly the case where CONFIG_NO_HZ_FULL=y and no CPU is in nohz_full mode. So far no caller needs this but lets prepare to avoid any future surprise.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 include/linux/housekeeping.h | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)
include/linux/housekeeping.h +8 -13
index 3d6a8e6..64d0ee5 100644
@@ -7,24 +7,19 @@
 
 #ifdef CONFIG_NO_HZ_FULL
 extern cpumask_var_t housekeeping_mask;
-
-static inline int housekeeping_any_cpu(void)
-{
-	return cpumask_any_and(housekeeping_mask, cpu_online_mask);
-}
-
 extern void __init housekeeping_init(void);
-
 #else
-
-static inline int housekeeping_any_cpu(void)
-{
-	return smp_processor_id();
-}
-
 static inline void housekeeping_init(void) { }
 #endif /* CONFIG_NO_HZ_FULL */
 
+static inline int housekeeping_any_cpu(void)
+{
+#ifdef CONFIG_NO_HZ_FULL
+	if (tick_nohz_full_enabled())
+		return cpumask_any_and(housekeeping_mask, cpu_online_mask);
+#endif
+	return smp_processor_id();
+}
 
 static inline const struct cpumask *housekeeping_cpumask(void)
 {
Signature · 1 line
2.7.4
#4 lore raw

RFC PATCH 4/9 housekeeping: Make housekeeping cpumask private

In reply to Frederic Weisbecker

Nobody needs to access this detail. housekeeping_cpumask() already takes care about it.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 include/linux/housekeeping.h | 31 ++++++++++---------------------
 kernel/housekeeping.c        | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 22 deletions(-)
include/linux/housekeeping.h +10 -21
index 64d0ee5..31a1401 100644
@@ -6,46 +6,35 @@
 #include <linux/tick.h>
 
 #ifdef CONFIG_NO_HZ_FULL
-extern cpumask_var_t housekeeping_mask;
+extern int housekeeping_any_cpu(void);
+extern const struct cpumask *housekeeping_cpumask(void);
+extern void housekeeping_affine(struct task_struct *t);
+extern bool housekeeping_test_cpu(int cpu);
 extern void __init housekeeping_init(void);
+
 #else
-static inline void housekeeping_init(void) { }
-#endif /* CONFIG_NO_HZ_FULL */
 
 static inline int housekeeping_any_cpu(void)
 {
-#ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
-		return cpumask_any_and(housekeeping_mask, cpu_online_mask);
-#endif
 	return smp_processor_id();
 }
 
 static inline const struct cpumask *housekeeping_cpumask(void)
 {
-#ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
-		return housekeeping_mask;
-#endif
 	return cpu_possible_mask;
 }
 
+static inline void housekeeping_affine(struct task_struct *t) { }
+static inline void housekeeping_init(void) { }
+#endif /* CONFIG_NO_HZ_FULL */
+
 static inline bool is_housekeeping_cpu(int cpu)
 {
 #ifdef CONFIG_NO_HZ_FULL
 	if (tick_nohz_full_enabled())
-		return cpumask_test_cpu(cpu, housekeeping_mask);
+		return housekeeping_test_cpu(cpu);
 #endif
 	return true;
 }
 
-static inline void housekeeping_affine(struct task_struct *t)
-{
-#ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
-		set_cpus_allowed_ptr(t, housekeeping_mask);
-
-#endif
-}
-
 #endif /* _LINUX_HOUSEKEEPING_H */
kernel/housekeeping.c +32 -1
index 6d8afd5..0183e75 100644
@@ -10,7 +10,38 @@
 #include <linux/tick.h>
 #include <linux/init.h>
 
-cpumask_var_t housekeeping_mask;
+static cpumask_var_t housekeeping_mask;
+
+int housekeeping_any_cpu(void)
+{
+	if (tick_nohz_full_enabled())
+		return cpumask_any_and(housekeeping_mask, cpu_online_mask);
+
+	return smp_processor_id();
+}
+
+const struct cpumask *housekeeping_cpumask(void)
+{
+	if (tick_nohz_full_enabled())
+		return housekeeping_mask;
+
+	return cpu_possible_mask;
+}
+
+void housekeeping_affine(struct task_struct *t)
+{
+	if (tick_nohz_full_enabled())
+		set_cpus_allowed_ptr(t, housekeeping_mask);
+}
+
+bool housekeeping_test_cpu(int cpu)
+{
+	if (tick_nohz_full_enabled())
+		return cpumask_test_cpu(cpu, housekeeping_mask);
+
+	return true;
+}
+
 
 void __init housekeeping_init(void)
 {
Signature · 1 line
2.7.4
#5 lore raw

RFC PATCH 5/9 housekeeping: Use its own static key

In reply to Frederic Weisbecker

Housekeeping code still depends on nohz_full static key. Since we want to decouple housekeeping from nohz, lets create a housekeeping own static key. It's mostly relevant for calls to is_housekeeping_cpu() from the scheduler.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 include/linux/housekeeping.h |  3 ++-
 kernel/housekeeping.c        | 14 +++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)
include/linux/housekeeping.h +2 -1
index 31a1401..cbe8d63 100644
@@ -6,6 +6,7 @@
 #include <linux/tick.h>
 
 #ifdef CONFIG_NO_HZ_FULL
+DECLARE_STATIC_KEY_FALSE(housekeeping_overriden);
 extern int housekeeping_any_cpu(void);
 extern const struct cpumask *housekeeping_cpumask(void);
 extern void housekeeping_affine(struct task_struct *t);
@@ -31,7 +32,7 @@ static inline void housekeeping_init(void) { }
 static inline bool is_housekeeping_cpu(int cpu)
 {
 #ifdef CONFIG_NO_HZ_FULL
-	if (tick_nohz_full_enabled())
+	if (static_branch_unlikely(&housekeeping_overriden))
 		return housekeeping_test_cpu(cpu);
 #endif
 	return true;
kernel/housekeeping.c +9 -5
index 0183e75..f8be7e6 100644
@@ -9,12 +9,15 @@
 #include <linux/housekeeping.h>
 #include <linux/tick.h>
 #include <linux/init.h>
+#include <linux/static_key.h>
 
+DEFINE_STATIC_KEY_FALSE(housekeeping_overriden);
+EXPORT_SYMBOL_GPL(housekeeping_overriden);
 static cpumask_var_t housekeeping_mask;
 
 int housekeeping_any_cpu(void)
 {
-	if (tick_nohz_full_enabled())
+	if (static_branch_unlikely(&housekeeping_overriden))
 		return cpumask_any_and(housekeeping_mask, cpu_online_mask);
 
 	return smp_processor_id();
@@ -22,7 +25,7 @@ int housekeeping_any_cpu(void)
 
 const struct cpumask *housekeeping_cpumask(void)
 {
-	if (tick_nohz_full_enabled())
+	if (static_branch_unlikely(&housekeeping_overriden))
 		return housekeeping_mask;
 
 	return cpu_possible_mask;
@@ -30,19 +33,18 @@ const struct cpumask *housekeeping_cpumask(void)
 
 void housekeeping_affine(struct task_struct *t)
 {
-	if (tick_nohz_full_enabled())
+	if (static_branch_unlikely(&housekeeping_overriden))
 		set_cpus_allowed_ptr(t, housekeeping_mask);
 }
 
 bool housekeeping_test_cpu(int cpu)
 {
-	if (tick_nohz_full_enabled())
+	if (static_branch_unlikely(&housekeeping_overriden))
 		return cpumask_test_cpu(cpu, housekeeping_mask);
 
 	return true;
 }
 
-
 void __init housekeeping_init(void)
 {
 	if (!tick_nohz_full_enabled())
@@ -58,6 +60,8 @@ void __init housekeeping_init(void)
 	cpumask_andnot(housekeeping_mask,
 		       cpu_possible_mask, tick_nohz_full_mask);
 
+	static_branch_enable(&housekeeping_overriden);
+
 	/* We need at least one CPU to handle housekeeping work */
 	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
 }
Signature · 1 line
2.7.4
#6 lore raw

RFC PATCH 6/9 housekeeping: Rename is_housekeeping_cpu to housekeeping_cpu

In reply to Frederic Weisbecker

To keep a proper housekeeping namespace.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 include/linux/housekeeping.h | 2 +-
 kernel/sched/core.c          | 6 +++---
 kernel/sched/fair.c          | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
include/linux/housekeeping.h +1 -1
index cbe8d63..320cc2b 100644
@@ -29,7 +29,7 @@ static inline void housekeeping_affine(struct task_struct *t) { }
 static inline void housekeeping_init(void) { }
 #endif /* CONFIG_NO_HZ_FULL */
 
-static inline bool is_housekeeping_cpu(int cpu)
+static inline bool housekeeping_cpu(int cpu)
 {
 #ifdef CONFIG_NO_HZ_FULL
 	if (static_branch_unlikely(&housekeeping_overriden))
kernel/sched/core.c +3 -3
index 751c040..adfee7f 100644
@@ -527,7 +527,7 @@ int get_nohz_timer_target(void)
 	int i, cpu = smp_processor_id();
 	struct sched_domain *sd;
 
-	if (!idle_cpu(cpu) && is_housekeeping_cpu(cpu))
+	if (!idle_cpu(cpu) && housekeeping_cpu(cpu))
 		return cpu;
 
 	rcu_read_lock();
@@ -536,14 +536,14 @@ int get_nohz_timer_target(void)
 			if (cpu == i)
 				continue;
 
-			if (!idle_cpu(i) && is_housekeeping_cpu(i)) {
+			if (!idle_cpu(i) && housekeeping_cpu(i)) {
 				cpu = i;
 				goto unlock;
 			}
 		}
 	}
 
-	if (!is_housekeeping_cpu(cpu))
+	if (!housekeeping_cpu(cpu))
 		cpu = housekeeping_any_cpu();
 unlock:
 	rcu_read_unlock();
kernel/sched/fair.c +1 -1
index 05dfced..8ecd204 100644
@@ -8600,7 +8600,7 @@ void nohz_balance_enter_idle(int cpu)
 		return;
 
 	/* Spare idle load balancing on CPUs that don't want to be disturbed: */
-	if (!is_housekeeping_cpu(cpu))
+	if (!housekeeping_cpu(cpu))
 		return;
 
 	if (test_bit(NOHZ_TICK_STOPPED, nohz_flags(cpu)))
Signature · 1 line
2.7.4
#7 lore raw

RFC PATCH 7/9 housekeeping: Use own boot option, independant from nohz

In reply to Frederic Weisbecker

The housekeeping is currently driven by nohz_full where any CPU that is not in the nohz_full range is considered as a housekeeper. This is a design mistake because nohz is just a detail among all the existing isolation features. Nohz shouldn't imply anything else than tick related things.

We rather want to drive all the isolation features from the housekeeping subsystem which is responsible for all the work that can be either affined (unpinned workqueues, timers, kthreads, ...) or offloaded (scheduler tick, ...).

Let's start with a boot option to define the housekeepers. We should be able to further enhance that through cpusets.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 include/linux/housekeeping.h |  2 --
 init/main.c                  |  2 --
 kernel/housekeeping.c        | 22 ++++++++++------------
 3 files changed, 10 insertions(+), 16 deletions(-)
include/linux/housekeeping.h +0 -2
index 320cc2b..ba769c8 100644
@@ -11,7 +11,6 @@ extern int housekeeping_any_cpu(void);
 extern const struct cpumask *housekeeping_cpumask(void);
 extern void housekeeping_affine(struct task_struct *t);
 extern bool housekeeping_test_cpu(int cpu);
-extern void __init housekeeping_init(void);
 
 #else
 
@@ -26,7 +25,6 @@ static inline const struct cpumask *housekeeping_cpumask(void)
 }
 
 static inline void housekeeping_affine(struct task_struct *t) { }
-static inline void housekeeping_init(void) { }
 #endif /* CONFIG_NO_HZ_FULL */
 
 static inline bool housekeeping_cpu(int cpu)
init/main.c +0 -2
index 9904a1e..9789ab7 100644
@@ -46,7 +46,6 @@
 #include <linux/cgroup.h>
 #include <linux/efi.h>
 #include <linux/tick.h>
-#include <linux/housekeeping.h>
 #include <linux/interrupt.h>
 #include <linux/taskstats_kern.h>
 #include <linux/delayacct.h>
@@ -608,7 +607,6 @@ asmlinkage __visible void __init start_kernel(void)
 	early_irq_init();
 	init_IRQ();
 	tick_init();
-	housekeeping_init();
 	rcu_init_nohz();
 	init_timers();
 	hrtimers_init();
kernel/housekeeping.c +10 -12
index f8be7e6..a54765d 100644
@@ -45,23 +45,21 @@ bool housekeeping_test_cpu(int cpu)
 	return true;
 }
 
-void __init housekeeping_init(void)
+/* Parse the boot-time housekeeping CPU list from the kernel parameters. */
+static int __init housekeeping_setup(char *str)
 {
-	if (!tick_nohz_full_enabled())
-		return;
-
-	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
-		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
-		cpumask_clear(tick_nohz_full_mask);
-		tick_nohz_full_running = false;
-		return;
+	alloc_bootmem_cpumask_var(&housekeeping_mask);
+	if (cpulist_parse(str, housekeeping_mask) < 0) {
+		pr_warn("Housekeeping: Incorrect cpumask\n");
+		free_bootmem_cpumask_var(housekeeping_mask);
+		return 1;
 	}
 
-	cpumask_andnot(housekeeping_mask,
-		       cpu_possible_mask, tick_nohz_full_mask);
-
 	static_branch_enable(&housekeeping_overriden);
 
 	/* We need at least one CPU to handle housekeeping work */
 	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
+
+	return 1;
 }
+__setup("housekeeping=", housekeeping_setup);
Signature · 1 line
2.7.4
#8 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 10 lines · On Fri, 21 Jul 2017 15:21:28 +0200 Frederic Weisbecker <[email protected]>…

The housekeeping is currently driven by nohz_full where any CPU that is not in the nohz_full range is considered as a housekeeper. This is a design mistake because nohz is just a detail among all the existing isolation features. Nohz shouldn't imply anything else than tick related things.

We rather want to drive all the isolation features from the housekeeping subsystem which is responsible for all the work that can be either affined (unpinned workqueues, timers, kthreads, ...) or offloaded (scheduler tick, ...).

That makes a lot of sense. I think this is moving in the right direction. I have a comment below though.

Quoted 89 lines

Let's start with a boot option to define the housekeepers. We should be able to further enhance that through cpusets.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 include/linux/housekeeping.h |  2 --
 init/main.c                  |  2 --
 kernel/housekeeping.c        | 22 ++++++++++------------
 3 files changed, 10 insertions(+), 16 deletions(-)
include/linux/housekeeping.h +0 -2
index 320cc2b..ba769c8 100644
@@ -11,7 +11,6 @@ extern int housekeeping_any_cpu(void);
 extern const struct cpumask *housekeeping_cpumask(void);
 extern void housekeeping_affine(struct task_struct *t);
 extern bool housekeeping_test_cpu(int cpu);
-extern void __init housekeeping_init(void);
 
 #else
 
@@ -26,7 +25,6 @@ static inline const struct cpumask *housekeeping_cpumask(void)
 }
 
 static inline void housekeeping_affine(struct task_struct *t) { }
-static inline void housekeeping_init(void) { }
 #endif /* CONFIG_NO_HZ_FULL */
 
 static inline bool housekeeping_cpu(int cpu)
init/main.c +0 -2
index 9904a1e..9789ab7 100644
@@ -46,7 +46,6 @@
 #include <linux/cgroup.h>
 #include <linux/efi.h>
 #include <linux/tick.h>
-#include <linux/housekeeping.h>
 #include <linux/interrupt.h>
 #include <linux/taskstats_kern.h>
 #include <linux/delayacct.h>
@@ -608,7 +607,6 @@ asmlinkage __visible void __init start_kernel(void)
 	early_irq_init();
 	init_IRQ();
 	tick_init();
-	housekeeping_init();
 	rcu_init_nohz();
 	init_timers();
 	hrtimers_init();
kernel/housekeeping.c +7 -10
index f8be7e6..a54765d 100644
@@ -45,23 +45,21 @@ bool housekeeping_test_cpu(int cpu)
 	return true;
 }
 
-void __init housekeeping_init(void)
+/* Parse the boot-time housekeeping CPU list from the kernel parameters. */
+static int __init housekeeping_setup(char *str)
 {
-	if (!tick_nohz_full_enabled())
-		return;
-
-	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
-		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
-		cpumask_clear(tick_nohz_full_mask);
-		tick_nohz_full_running = false;
-		return;
+	alloc_bootmem_cpumask_var(&housekeeping_mask);
+	if (cpulist_parse(str, housekeeping_mask) < 0) {
+		pr_warn("Housekeeping: Incorrect cpumask\n");
+		free_bootmem_cpumask_var(housekeeping_mask);
+		return 1;
 	}
 
-	cpumask_andnot(housekeeping_mask,

kernel/housekeeping.c bool housekeeping_test_cpu(int cpu) · line 65

Quoted 10 lines
kernel/housekeeping.c +3 -2
-		       cpu_possible_mask, tick_nohz_full_mask);
-
 	static_branch_enable(&housekeeping_overriden);
 
 	/* We need at least one CPU to handle housekeeping work */
 	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
+
+	return 1;
 }
+__setup("housekeeping=", housekeeping_setup);

Am I right that from now on nohz_full= users will also have to specify housekeeping= in order to get nohz_full working? If that's correct, then won't this patch break nohz_full for existing setups?

Also, I just give this series a try and got this:

[    0.000000] Kernel command line: BOOT_IMAGE=/vmlinuz-4.13.0-rc4+ root=/dev/mapper/rhel_virtlab508-root ro crashkernel=auto rd.lvm.lv=rhel_virtlab508/root rd.lvm.lv=rhel_virtlab508/swap console=ttyS1,115200 LANG=en_US.UTF-8 housekeeping=0,2,4,6,8,10,12,14,1 isolcpus=15 nohz_full=15 intel_pstate=disable
[    0.000000] static_key_slow_inc used before call to jump_label_init
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:108 static_key_slow_inc+0x86/0xa0
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.13.0-rc4+ #2
[    0.000000] Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.2.6 06/08/2015
[    0.000000] task: ffffffffb6010480 task.stack: ffffffffb6000000
[    0.000000] RIP: 0010:static_key_slow_inc+0x86/0xa0
[    0.000000] RSP: 0000:ffffffffb6003d98 EFLAGS: 00010046 ORIG_RAX: 0000000000000000
[    0.000000] RAX: 0000000000000037 RBX: ffffffffb66aa780 RCX: ffffffffb6061308
[    0.000000] RDX: 0000000000000000 RSI: 0000000000000082 RDI: 0000000000000002
[    0.000000] RBP: ffffffffb6003da0 R08: 6b5f636974617473 R09: 00000000000001e4
[    0.000000] R10: 776f6c735f79656b R11: 0000000000000000 R12: ffff972c3ffd1cfe
[    0.000000] R13: ffffffffffffffff R14: 0000000000000000 R15: 000000000000000d
[    0.000000] FS:  0000000000000000(0000) GS:ffff97282ea00000(0000) knlGS:0000000000000000
[    0.000000] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.000000] CR2: ffff972905974000 CR3: 0000000545209000 CR4: 00000000000406b0
[    0.000000] Call Trace:
[    0.000000]  static_key_enable+0x1d/0x30
[    0.000000]  housekeeping_setup+0x5a/0x7e
[    0.000000]  unknown_bootoption+0x8b/0x19a
[    0.000000]  parse_args+0x224/0x3b0
[    0.000000]  ? set_init_arg+0x5a/0x5a
[    0.000000]  start_kernel+0x209/0x4cd
[    0.000000]  ? set_init_arg+0x5a/0x5a
[    0.000000]  ? early_idt_handler_array+0x120/0x120
[    0.000000]  x86_64_start_reservations+0x24/0x26
[    0.000000]  x86_64_start_kernel+0x14c/0x16f
[    0.000000]  secondary_startup_64+0x9f/0x9f
#9 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 26 lines · Luiz Capitulino
Quoted 24 lines · On Fri, 21 Jul 2017 15:21:28 +0200 Frederic Weisbecker <[email protected]>…
kernel/housekeeping.c +7 -12
-void __init housekeeping_init(void)
+/* Parse the boot-time housekeeping CPU list from the kernel parameters. */
+static int __init housekeeping_setup(char *str)
 {
-	if (!tick_nohz_full_enabled())
-		return;
-
-	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
-		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
-		cpumask_clear(tick_nohz_full_mask);
-		tick_nohz_full_running = false;
-		return;
+	alloc_bootmem_cpumask_var(&housekeeping_mask);
+	if (cpulist_parse(str, housekeeping_mask) < 0) {
+		pr_warn("Housekeeping: Incorrect cpumask\n");
+		free_bootmem_cpumask_var(housekeeping_mask);
+		return 1;
 	}
 
-	cpumask_andnot(housekeeping_mask,
-		       cpu_possible_mask, tick_nohz_full_mask);
-
 	static_branch_enable(&housekeeping_overriden);
Quoted 11 lines

kernel/housekeeping.c bool housekeeping_test_cpu(int cpu) · line 65

Quoted 6 lines
kernel/housekeeping.c +3 -0
 	/* We need at least one CPU to handle housekeeping work */
 	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
+
+	return 1;
 }
+__setup("housekeeping=", housekeeping_setup);

Am I right that from now on nohz_full= users will also have to specify housekeeping= in order to get nohz_full working? If that's correct, then won't this patch break nohz_full for existing setups?

nohz_full= will still work but will only imply tick stop. A few isolation details that were enabled by nohz_full= won't be handled anymore such as:
unbound timers affinity, watchdog disablement, rcu threads affinity, sched idle load balancing... Those are now handled by housekeeping=

So yes in a sense, this can break some setup that assume nohz_full= does more than stopping the tick.

Perhaps I should remove the nohz_full= parameter altogether and let nohz_full controlled by housekeeping= only. How much can kernel parameters be considered as kernel ABIs?

Also I'm wondering if "housekeeping=" is a clear name for users. "isolation=" or "cpu_isolation=" would be better and more obvious. Housekeeping based naming would only be internal implementation detail. And deactivating the tick through "cpu_isolation=" would be clearer than if we did through "housekeeping=".

Of course the problem is that we already have "isolcpus=". But re-implementing isolcpus on top of housekeeping might be a good idea. I believe that the current implementation on top of NULL domains isn't much beloved. A less controversial implementation might even allow us to control it though cpusets.

Quoted 7 lines

Also, I just give this series a try and got this:

[    0.000000] Kernel command line: BOOT_IMAGE=/vmlinuz-4.13.0-rc4+ root=/dev/mapper/rhel_virtlab508-root ro crashkernel=auto rd.lvm.lv=rhel_virtlab508/root rd.lvm.lv=rhel_virtlab508/swap console=ttyS1,115200 LANG=en_US.UTF-8 housekeeping=0,2,4,6,8,10,12,14,1 isolcpus=15 nohz_full=15 intel_pstate=disable
[    0.000000] static_key_slow_inc used before call to jump_label_init
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:108 static_key_slow_inc+0x86/0xa0

Oops ^_^

Thanks.

#10 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 39 lines · On Sat, 12 Aug 2017 16:10:06 +0200 Frederic Weisbecker <[email protected]>…
Quoted 26 lines · Luiz Capitulino
Quoted 24 lines · On Fri, 21 Jul 2017 15:21:28 +0200 Frederic Weisbecker <[email protected]>…
kernel/housekeeping.c +7 -12
-void __init housekeeping_init(void)
+/* Parse the boot-time housekeeping CPU list from the kernel parameters. */
+static int __init housekeeping_setup(char *str)
 {
-	if (!tick_nohz_full_enabled())
-		return;
-
-	if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) {
-		WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n");
-		cpumask_clear(tick_nohz_full_mask);
-		tick_nohz_full_running = false;
-		return;
+	alloc_bootmem_cpumask_var(&housekeeping_mask);
+	if (cpulist_parse(str, housekeeping_mask) < 0) {
+		pr_warn("Housekeeping: Incorrect cpumask\n");
+		free_bootmem_cpumask_var(housekeeping_mask);
+		return 1;
 	}
 
-	cpumask_andnot(housekeeping_mask,
-		       cpu_possible_mask, tick_nohz_full_mask);
-
 	static_branch_enable(&housekeeping_overriden);
Quoted 11 lines

kernel/housekeeping.c bool housekeeping_test_cpu(int cpu) · line 65

Quoted 6 lines
kernel/housekeeping.c +3 -0
 	/* We need at least one CPU to handle housekeeping work */
 	WARN_ON_ONCE(cpumask_empty(housekeeping_mask));
+
+	return 1;
 }
+__setup("housekeeping=", housekeeping_setup);  

Am I right that from now on nohz_full= users will also have to specify housekeeping= in order to get nohz_full working? If that's correct, then won't this patch break nohz_full for existing setups?

Quoted 7 lines

nohz_full= will still work but will only imply tick stop. A few isolation details that were enabled by nohz_full= won't be handled anymore such as:
unbound timers affinity, watchdog disablement, rcu threads affinity, sched idle load balancing... Those are now handled by housekeeping=

So yes in a sense, this can break some setup that assume nohz_full= does more than stopping the tick.

Yes, the problem is that this is how it has always worked. Also, the breakage will be very subtle and hard to debug.

Quoted 2 lines

Perhaps I should remove the nohz_full= parameter altogether and let nohz_full controlled by housekeeping= only. How much can kernel parameters be considered as kernel ABIs?

That's a very good question, I don't have an answer for that.

Quoted 4 lines

Also I'm wondering if "housekeeping=" is a clear name for users. "isolation=" or "cpu_isolation=" would be better and more obvious. Housekeeping based naming would only be internal implementation detail. And deactivating the tick through "cpu_isolation=" would be clearer than if we did through "housekeeping=".

That's exactly my thinking while I was reviewing the series!

Quoted 4 lines

Of course the problem is that we already have "isolcpus=". But re-implementing isolcpus on top of housekeeping might be a good idea. I believe that the current implementation on top of NULL domains isn't much beloved. A less controversial implementation might even allow us to control it though cpusets.

You're completely right. Some people don't use isolcpus= because it disables load balancing and that may be a problem for setups where tasks are pinned to a set of CPUs where the number of tasks is greater than the number of CPUs. However, for the cases where you have a single task pinned to a CPU, having load balancing taking place adds an extra latency (I won't remember how much, but I guess it was more than 10us).

If there's a way to "disable" load balancing from user-space, say with cpusets, then I think we should keep the isolated CPUs attached to a domain as you suggest.

Another detail about isolcpus= is that it doesn't isolate the CPU from kernel threads. That is, unpinned kernel threads are allowed to run on CPUs not isolated with isolcpus=. We might consider changing that for a new isolation option.

I know that there are many arguments against isolcpus= and some people advice using cpusets. The problem with that advice is that isolcpus= goes a bit beyond isolating a CPU from user-space tasks. One additional thing is does for example, is pinning the kernel_init() thread to housekeeping CPUs. This is key, because that thread will create timers at early boot that will pin themselves to the CPU they run.

Finally, I'm wondering how all this will fit together with TASK_ISOLATION. One of the questions I ask myself is: can/should the things TASK_ISOLATION does be done by a kernel command-line parameter instead? Or should we try to come up with a list of global things to control (eg. the tick, kernel thread affinity, etc) and per-task controls?

#11 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 17 lines · Luiz Capitulino
Quoted 5 lines · On Sat, 12 Aug 2017 16:10:06 +0200 Frederic Weisbecker <[email protected]>…
Quoted 4 lines

Am I right that from now on nohz_full= users will also have to specify housekeeping= in order to get nohz_full working? If that's correct, then won't this patch break nohz_full for existing setups?

Quoted 7 lines

nohz_full= will still work but will only imply tick stop. A few isolation details that were enabled by nohz_full= won't be handled anymore such as:
unbound timers affinity, watchdog disablement, rcu threads affinity, sched idle load balancing... Those are now handled by housekeeping=

So yes in a sense, this can break some setup that assume nohz_full= does more than stopping the tick.

Yes, the problem is that this is how it has always worked. Also, the breakage will be very subtle and hard to debug.

[...]

Quoted 5 lines
Quoted 3 lines

Perhaps I should remove the nohz_full= parameter altogether and let nohz_full controlled by housekeeping= only. How much can kernel parameters be considered as kernel ABIs?

That's a very good question, I don't have an answer for that.

That said, "nohz_full=" never implied too much isolation features so far, and those have often changed over time, as in RCU. I think unbound timer affinity is the most important one.

Perhaps we can keep "nohz_full=1-15" as an alias for a future "cpu_isolation=nohz,1-15" and at least imply unbound timer affinity with it.

Quoted 13 lines
Quoted 5 lines

Also I'm wondering if "housekeeping=" is a clear name for users. "isolation=" or "cpu_isolation=" would be better and more obvious. Housekeeping based naming would only be internal implementation detail. And deactivating the tick through "cpu_isolation=" would be clearer than if we did through "housekeeping=".

That's exactly my thinking while I was reviewing the series!

Quoted 4 lines

Of course the problem is that we already have "isolcpus=". But re-implementing isolcpus on top of housekeeping might be a good idea. I believe that the current implementation on top of NULL domains isn't much beloved. A less controversial implementation might even allow us to control it though cpusets.

Quoted 7 lines

You're completely right. Some people don't use isolcpus= because it disables load balancing and that may be a problem for setups where tasks are pinned to a set of CPUs where the number of tasks is greater than the number of CPUs. However, for the cases where you have a single task pinned to a CPU, having load balancing taking place adds an extra latency (I won't remember how much, but I guess it was more than 10us).

What is the source of the load balancing inducing such latency when a single task is affine to a CPU? If this is idle load balancing, it is now affine to housekeepers. If this is task wakeup then it's suprising because select_task_rq() is optimized toward single CPU affinity.

Is there another source I'm overlooking?

Quoted 3 lines

If there's a way to "disable" load balancing from user-space, say with cpusets, then I think we should keep the isolated CPUs attached to a domain as you suggest.

I'm not sure such a solution would be accepted. The most sensible way to disable load balancing is still to tune the affinity of tasks. If there is an off-case overhead with load balancing (ie: when no more than one task is affine to that CPU) then we should solve that with a fast path.

Quoted 4 lines

Another detail about isolcpus= is that it doesn't isolate the CPU from kernel threads. That is, unpinned kernel threads are allowed to run on CPUs not isolated with isolcpus=. We might consider changing that for a new isolation option.

You mean unpinned kernel threads are allowed to run on isolcpus, right? That definetly can be solved.

Quoted 7 lines

I know that there are many arguments against isolcpus= and some people advice using cpusets. The problem with that advice is that isolcpus= goes a bit beyond isolating a CPU from user-space tasks. One additional thing is does for example, is pinning the kernel_init() thread to housekeeping CPUs. This is key, because that thread will create timers at early boot that will pin themselves to the CPU they run.

Right, but also unbound timers are affine to housekeepers, we needed that for nohz_full.

Quoted 5 lines

Finally, I'm wondering how all this will fit together with TASK_ISOLATION. One of the questions I ask myself is: can/should the things TASK_ISOLATION does be done by a kernel command-line parameter instead? Or should we try to come up with a list of global things to control (eg. the tick, kernel thread affinity, etc) and per-task controls?

So I've been thinking a lot about that lately. I told Chris that TASK_ISOLATION shouldn't be a CPU feature but a task feature. Then I realized that it doesn't work either, my bad :-) In the end I think that the most part of it must be a CPU
property: nohz, task isolation, timers and workqueue affinity, etc... Then what's left for the per task thing is to tell it when it is unexpectingly interrupted by noise.

Therefore I think most of the isolation features should be controlled by command line and cpusets (through a new cpuset subsystem maybe) then TASK_ISOLATION through prtcl() for the noise monitoring.

Thanks.

#12 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 11 lines · On Mon, 14 Aug 2017 19:01:09 +0200 Frederic Weisbecker <[email protected]>…
Quoted 4 lines
Quoted 2 lines

Perhaps I should remove the nohz_full= parameter altogether and let nohz_full controlled by housekeeping= only. How much can kernel parameters be considered as kernel ABIs?

That's a very good question, I don't have an answer for that.

That said, "nohz_full=" never implied too much isolation features so far, and those have often changed over time, as in RCU. I think unbound timer affinity is the most important one.

Perhaps we can keep "nohz_full=1-15" as an alias for a future "cpu_isolation=nohz,1-15" and at least imply unbound timer affinity with it.

That would work for me.

Quoted 20 lines
Quoted 12 lines
Quoted 4 lines

Also I'm wondering if "housekeeping=" is a clear name for users. "isolation=" or "cpu_isolation=" would be better and more obvious. Housekeeping based naming would only be internal implementation detail. And deactivating the tick through "cpu_isolation=" would be clearer than if we did through "housekeeping=".

That's exactly my thinking while I was reviewing the series!

Quoted 4 lines

Of course the problem is that we already have "isolcpus=". But re-implementing isolcpus on top of housekeeping might be a good idea. I believe that the current implementation on top of NULL domains isn't much beloved. A less controversial implementation might even allow us to control it though cpusets.

Quoted 7 lines

You're completely right. Some people don't use isolcpus= because it disables load balancing and that may be a problem for setups where tasks are pinned to a set of CPUs where the number of tasks is greater than the number of CPUs. However, for the cases where you have a single task pinned to a CPU, having load balancing taking place adds an extra latency (I won't remember how much, but I guess it was more than 10us).

Quoted 4 lines

What is the source of the load balancing inducing such latency when a single task is affine to a CPU? If this is idle load balancing, it is now affine to housekeepers. If this is task wakeup then it's suprising because select_task_rq() is optimized toward single CPU affinity.

I guess it was idle load balancing, but I don't remember because this was a few years ago. I think this might be reproducible without using isolcpus=. I'll give it a try shortly and let you know.

Quoted 10 lines

Is there another source I'm overlooking?

Quoted 3 lines

If there's a way to "disable" load balancing from user-space, say with cpusets, then I think we should keep the isolated CPUs attached to a domain as you suggest.

I'm not sure such a solution would be accepted. The most sensible way to disable load balancing is still to tune the affinity of tasks. If there is an off-case overhead with load balancing (ie: when no more than one task is affine to that CPU) then we should solve that with a fast path.

OK, I'll take a look.

Quoted 6 lines
Quoted 4 lines

Another detail about isolcpus= is that it doesn't isolate the CPU from kernel threads. That is, unpinned kernel threads are allowed to run on CPUs not isolated with isolcpus=. We might consider changing that for a new isolation option.

You mean unpinned kernel threads are allowed to run on isolcpus, right?

Exactly.

Quoted 25 lines

That definetly can be solved.

Quoted 7 lines

I know that there are many arguments against isolcpus= and some people advice using cpusets. The problem with that advice is that isolcpus= goes a bit beyond isolating a CPU from user-space tasks. One additional thing is does for example, is pinning the kernel_init() thread to housekeeping CPUs. This is key, because that thread will create timers at early boot that will pin themselves to the CPU they run.

Right, but also unbound timers are affine to housekeepers, we needed that for nohz_full.

Quoted 5 lines

Finally, I'm wondering how all this will fit together with TASK_ISOLATION. One of the questions I ask myself is: can/should the things TASK_ISOLATION does be done by a kernel command-line parameter instead? Or should we try to come up with a list of global things to control (eg. the tick, kernel thread affinity, etc) and per-task controls?

So I've been thinking a lot about that lately. I told Chris that TASK_ISOLATION shouldn't be a CPU feature but a task feature. Then I realized that it doesn't work either, my bad :-) In the end I think that the most part of it must be a CPU
property: nohz, task isolation, timers and workqueue affinity, etc... Then what's left for the per task thing is to tell it when it is unexpectingly interrupted by noise.

Quoted 3 lines

Therefore I think most of the isolation features should be controlled by command line and cpusets (through a new cpuset subsystem maybe) then TASK_ISOLATION through prtcl() for the noise monitoring.

I agree.

#13 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 10 lines · Luiz Capitulino

On Mon, 14 Aug 2017 19:01:09 +0200

Quoted 4 lines

What is the source of the load balancing inducing such latency when a single task is affine to a CPU? If this is idle load balancing, it is now affine to housekeepers. If this is task wakeup then it's suprising because select_task_rq() is optimized toward single CPU affinity.

I guess it was idle load balancing, but I don't remember because this was a few years ago. I think this might be reproducible without using isolcpus=. I'll give it a try shortly and let you know.

idle_balance() can swamp other noise by a couple orders of magnitude,

-Mike

#14 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 8 lines · Mike Galbraith
Quoted 7 lines · Luiz Capitulino

On Mon, 14 Aug 2017 19:01:09 +0200

Quoted 4 lines

What is the source of the load balancing inducing such latency when a single task is affine to a CPU? If this is idle load balancing, it is now affine to housekeepers. If this is task wakeup then it's suprising because select_task_rq() is optimized toward single CPU affinity.

Quoted 5 lines
Quoted 3 lines

I guess it was idle load balancing, but I don't remember because this was a few years ago. I think this might be reproducible without using isolcpus=. I'll give it a try shortly and let you know.

idle_balance() can swamp other noise by a couple orders of magnitude,

Ah I missed that one. Is there any way we can also lower the overhead there? It looks unfortunately hard to tell if there is only one task affine to a given CPU, assertion on top of which we could make a fast exit.

#15 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 9 lines · Frederic Weisbecker
Quoted 8 lines · Mike Galbraith
Quoted 7 lines · Luiz Capitulino

On Mon, 14 Aug 2017 19:01:09 +0200

Quoted 4 lines

What is the source of the load balancing inducing such latency when a single task is affine to a CPU? If this is idle load balancing, it is now affine to housekeepers. If this is task wakeup then it's suprising because select_task_rq() is optimized toward single CPU affinity.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

I guess it was idle load balancing, but I don't remember because this was a few years ago. I think this might be reproducible without using isolcpus=. I'll give it a try shortly and let you know.

idle_balance() can swamp other noise by a couple orders of magnitude,

Ah I missed that one. Is there any way we can also lower the overhead there?

Why?  HPC proggies won't benefit from a partially filled pothole any more that a ~zero ground clearance formula 1 car would.  The pothole intolerant either isolate, killing (most) LB, or they meet a wall.

-Mike

#16 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 14 lines · Mike Galbraith
Quoted 13 lines · Frederic Weisbecker
Quoted 12 lines · Mike Galbraith
Quoted 10 lines · Luiz Capitulino

On Mon, 14 Aug 2017 19:01:09 +0200

Quoted 4 lines

What is the source of the load balancing inducing such latency when a single task is affine to a CPU? If this is idle load balancing, it is now affine to housekeepers. If this is task wakeup then it's suprising because select_task_rq() is optimized toward single CPU affinity.

I guess it was idle load balancing, but I don't remember because this was a few years ago. I think this might be reproducible without using isolcpus=. I'll give it a try shortly and let you know.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

idle_balance() can swamp other noise by a couple orders of magnitude,

Ah I missed that one. Is there any way we can also lower the overhead there?

Why?  HPC proggies won't benefit from a partially filled pothole any more that a ~zero ground clearance formula 1 car would.  The pothole intolerant either isolate, killing (most) LB, or they meet a wall.

Don't the HPC guys just disable idle_balance(), or am I out of date again?

Thanx, Paul

#17 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 1 line · Paul E. McKenney

Don't the HPC guys just disable idle_balance(), or am I out of date again?

Ummm.. Why does idle management matter when your goal is to keep all processor busy working at maximum throughput?

#18 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 6 lines · Christopher Lameter
Quoted 1 line · Paul E. McKenney

Don't the HPC guys just disable idle_balance(), or am I out of date again?

Ummm.. Why does idle management matter when your goal is to keep all processor busy working at maximum throughput?

If you _never_ idle, you never have to worry about it.  Is 100% CPU until the end of time all there is to HPC?

-Mike

#19 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 10 lines · Mike Galbraith
Quoted 6 lines · Christopher Lameter
Quoted 1 line · Paul E. McKenney

Don't the HPC guys just disable idle_balance(), or am I out of date again?

Ummm.. Why does idle management matter when your goal is to keep all processor busy working at maximum throughput?

If you _never_ idle, you never have to worry about it.  Is 100% CPU until the end of time all there is to HPC?

Most of the time that is true for HPC loads. They may also go through a I/O throughput constrained processing phase or synchronization phase where idle activity occurs.

However, there are also low latency loads that are often confused with HPC. Those are usually waiting idle until an event happens and then have to react to it in the fastest way possible. After that they go back to idle. You could call this a RT load (since the term seems to be so flexible...) but its more event based than the typical idea of realtime (do something at this and thata time).

#20 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

Quoted 10 lines · Christopher Lameter
Quoted 8 lines · Mike Galbraith
Quoted 6 lines · Christopher Lameter
Quoted 1 line · Paul E. McKenney

Don't the HPC guys just disable idle_balance(), or am I out of date again?

Ummm.. Why does idle management matter when your goal is to keep all processor busy working at maximum throughput?

Quoted 6 lines
Quoted 2 lines

If you _never_ idle, you never have to worry about it.  Is 100% CPU until the end of time all there is to HPC?

Most of the time that is true for HPC loads. They may also go through a I/O throughput constrained processing phase or synchronization phase where idle activity occurs.

That synchronization is what I tend to get all hung up on pondering HPC vs RT terminology.  Care and feeding of parallel pipelines has got to be loaded to the gills with synchronization (intermediate math results etc), rendering the sum event driven.

Doesn't matter, both acronyms reduce to latency intolerant.

-Mike

#21 lore raw

Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz

In reply to Paul E. McKenney
Quoted 17 lines · Paul E. McKenney
Quoted 16 lines · Mike Galbraith
Quoted 15 lines · Frederic Weisbecker
Quoted 13 lines · Mike Galbraith
Quoted 10 lines · Luiz Capitulino

On Mon, 14 Aug 2017 19:01:09 +0200

Quoted 4 lines

What is the source of the load balancing inducing such latency when a single task is affine to a CPU? If this is idle load balancing, it is now affine to housekeepers. If this is task wakeup then it's suprising because select_task_rq() is optimized toward single CPU affinity.

I guess it was idle load balancing, but I don't remember because this was a few years ago. I think this might be reproducible without using isolcpus=. I'll give it a try shortly and let you know.

idle_balance() can swamp other noise by a couple orders of magnitude,

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Ah I missed that one. Is there any way we can also lower the overhead there?

Why?  HPC proggies won't benefit from a partially filled pothole any more that a ~zero ground clearance formula 1 car would.  The pothole intolerant either isolate, killing (most) LB, or they meet a wall.

Don't the HPC guys just disable idle_balance(), or am I out of date again?

They could do just that if what they're doing is not really critical. I'm not an HPC guy, so can only speculate.  I don't see much difference between HPC and RT though, the rules of the game seem to be about the same (them both being HPC;).. what you can control, you do control.

-Mike

#22 lore raw

RFC PATCH 8/9 housekeeping: Move it under own config, independant from NO_HZ

In reply to Frederic Weisbecker

Complete the housekeeping split from CONFIG_NO_HZ_FULL by moving it under its own config. This way we finally separate the isolation code from nohz.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 include/linux/housekeeping.h | 6 +++---
 init/Kconfig                 | 6 ++++++
 kernel/Makefile              | 2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)
include/linux/housekeeping.h +3 -3
index ba769c8..d79a665 100644
@@ -5,7 +5,7 @@
 #include <linux/init.h>
 #include <linux/tick.h>
 
-#ifdef CONFIG_NO_HZ_FULL
+#ifdef CONFIG_HOUSEKEEPING
 DECLARE_STATIC_KEY_FALSE(housekeeping_overriden);
 extern int housekeeping_any_cpu(void);
 extern const struct cpumask *housekeeping_cpumask(void);
@@ -25,11 +25,11 @@ static inline const struct cpumask *housekeeping_cpumask(void)
 }
 
 static inline void housekeeping_affine(struct task_struct *t) { }
-#endif /* CONFIG_NO_HZ_FULL */
+#endif /* CONFIG_HOUSEKEEPING */
 
 static inline bool housekeeping_cpu(int cpu)
 {
-#ifdef CONFIG_NO_HZ_FULL
+#ifdef CONFIG_HOUSEKEEPING
 	if (static_branch_unlikely(&housekeeping_overriden))
 		return housekeeping_test_cpu(cpu);
 #endif
init/Kconfig +6 -0
index 8514b25..cc7af32 100644
@@ -472,6 +472,12 @@ config TASK_IO_ACCOUNTING
 
 endmenu # "CPU/Task time and stats accounting"
 
+config HOUSEKEEPING
+	bool "Housekeeping work tuning"
+	help
+	  Allow to affine and offload kernel internal routine jobs that are
+	  usually executed on any CPU.
+
 source "kernel/rcu/Kconfig"
 
 config BUILD_BIN2C
kernel/Makefile +1 -1
index 8a85c4b..58be459 100644
@@ -109,7 +109,7 @@ obj-$(CONFIG_JUMP_LABEL) += jump_label.o
 obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
 obj-$(CONFIG_TORTURE_TEST) += torture.o
 obj-$(CONFIG_MEMBARRIER) += membarrier.o
-obj-$(CONFIG_NO_HZ_FULL) += housekeeping.o
+obj-$(CONFIG_HOUSEKEEPING) += housekeeping.o
 
 obj-$(CONFIG_HAS_IOMEM) += memremap.o
 
Signature · 1 line
2.7.4
#23 lore raw

RFC PATCH 9/9 workqueue: Affine unbound workqueues to housekeeping cpumask

In reply to Frederic Weisbecker

Although the unbound workqueue cpumask can be overriden through sysfs, the housekeeping cpumask which drives the CPU isolation provides a relevant default value.

Cc Chris Metcalf <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Wanpeng Li <[email protected]>
Cc Luiz Capitulino <[email protected]>
---
 kernel/workqueue.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/workqueue.c +2 -1
index a86688f..4303c06 100644
@@ -48,6 +48,7 @@
 #include <linux/nodemask.h>
 #include <linux/moduleparam.h>
 #include <linux/uaccess.h>
+#include <linux/housekeeping.h>
 
 #include "workqueue_internal.h"
 
@@ -5524,7 +5525,7 @@ int __init workqueue_init_early(void)
 	WARN_ON(__alignof__(struct pool_workqueue) < __alignof__(long long));
 
 	BUG_ON(!alloc_cpumask_var(&wq_unbound_cpumask, GFP_KERNEL));
-	cpumask_copy(wq_unbound_cpumask, cpu_possible_mask);
+	cpumask_copy(wq_unbound_cpumask, housekeeping_cpumask());
 
 	pwq_cache = KMEM_CACHE(pool_workqueue, SLAB_PANIC);
 
Signature · 1 line
2.7.4
Reviewed-by on the v1 series
#24 lore raw
In reply to Frederic Weisbecker
Quoted 14 lines · Frederic Weisbecker

I'm leaving for two weeks so this is food for thoughts in the meantime :)

We have a design issue with nohz_full: it drives the isolation features through the *housekeeping*() functions: kthreads, unpinned timers, watchdog, ...

But things should work the other way around because the tick is just an isolation feature among others.

So we need a housekeeping subsystem to drive all these isolation features, including nohz full in a later iteration. For now this is a basic draft. In the long run this subsystem should also drive the tick offloading (remove residual 1Hz) and all unbound kthreads.

Quoted 4 lines

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

HEAD: 68e3af1de5db228bf6c2a5e721bce59a02cfc4e1

For the series:

I spotted a few typos that you should grep for and fix for your next version:
"watchog", "Lets/lets" instead of "Let's/let's", "overriden" (should have two d's).

The new housekeeping=MASK boot option seems like it might make it a little irritating to specify nohz_full=MASK as well. I guess if setting NO_HZ_FULL_ALL implied "all but housekeeping", it becomes a reasonably tidy solution. To make this work right you might have to make the housekeeping option early_param instead so its value is available early enough.

Signature · 2 lines
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com
#25 lore raw
Quoted 20 lines · Chris Metcalf
Quoted 14 lines · Frederic Weisbecker

I'm leaving for two weeks so this is food for thoughts in the meantime :)

We have a design issue with nohz_full: it drives the isolation features through the *housekeeping*() functions: kthreads, unpinned timers, watchdog, ...

But things should work the other way around because the tick is just an isolation feature among others.

So we need a housekeeping subsystem to drive all these isolation features, including nohz full in a later iteration. For now this is a basic draft. In the long run this subsystem should also drive the tick offloading (remove residual 1Hz) and all unbound kthreads.

Quoted 4 lines

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

HEAD: 68e3af1de5db228bf6c2a5e721bce59a02cfc4e1

Quoted 8 lines

For the series:

I spotted a few typos that you should grep for and fix for your next version:
"watchog", "Lets/lets" instead of "Let's/let's", "overriden" (should have two d's).

Thanks, I'll check those.

Quoted 8 lines

The new housekeeping=MASK boot option seems like it might make it a little irritating to specify nohz_full=MASK as well. I guess if setting NO_HZ_FULL_ALL
implied "all but housekeeping", it becomes a reasonably tidy solution. To make
this work right you might have to make the housekeeping option early_param instead so its value is available early enough.

Good point. But perhaps I should add a new NO_HZ_FULL_BUT_HOUSEKEEPING option. Otherwise we'll change the meaning of NO_HZ_FULL_ALL way too much, to the point that its default behaviour will be the exact opposite of the current one: by default every CPU is housekeeping, so NO_HZ_FULL_ALL would have no effect anymore if we don't set housekeeping boot option.

Also I plan to add a housekeeping option to offload the residual 1Hz tick from nohz_full CPUs. So having "housekeeping=0,tick_offload" would make CPU 0 the housekeeper, make the other CPUs nohz_full and handle their 1hz tick from CPU 0.

Just a few thoughts.

#26 lore raw
Quoted 5 lines · Frederic Weisbecker

But perhaps I should add a new NO_HZ_FULL_BUT_HOUSEKEEPING option. Otherwise we'll change the meaning of NO_HZ_FULL_ALL way too much, to the point that its default behaviour will be the exact opposite of the current one: by default every CPU is housekeeping, so NO_HZ_FULL_ALL would have no effect anymore if we don't set housekeeping boot option.

Maybe a CONFIG_HOUSEKEEPING_BOOT_ONLY as a way to restrict housekeeping by default to just the boot cpu. In conjunction with NOHZ_FULL_ALL you would then get the expected semantics.

Quoted 3 lines

Also I plan to add a housekeeping option to offload the residual 1Hz tick from nohz_full CPUs. So having "housekeeping=0,tick_offload" would make CPU 0 the housekeeper, make the other CPUs nohz_full and handle their 1hz tick from CPU 0.

It does seem like that might be implied by requesting NOHZ_FULL on the core... or maybe it's just implied by TASK_ISOLATION. I've done a bad job of finding time to work on that since last year's Plumbers, but September looks good :)

Signature · 2 lines
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com
#27 lore raw
Quoted 10 lines · Chris Metcalf
Quoted 5 lines · Frederic Weisbecker

But perhaps I should add a new NO_HZ_FULL_BUT_HOUSEKEEPING option. Otherwise we'll change the meaning of NO_HZ_FULL_ALL way too much, to the point that its default behaviour will be the exact opposite of the current one: by default every CPU is housekeeping, so NO_HZ_FULL_ALL would have no effect anymore if we don't set housekeeping boot option.

Maybe a CONFIG_HOUSEKEEPING_BOOT_ONLY as a way to restrict housekeeping by default to just the boot cpu. In conjunction with NOHZ_FULL_ALL you would then get the expected semantics.

A big box with only the boot cpu for housekeeping is likely screwed.

Personally, I think NOHZ_FULL_ALL should just die.

-Mike

#28 lore raw
Quoted 8 lines · Mike Galbraith
Quoted 7 lines · Chris Metcalf
Quoted 5 lines · Frederic Weisbecker

But perhaps I should add a new NO_HZ_FULL_BUT_HOUSEKEEPING option. Otherwise we'll change the meaning of NO_HZ_FULL_ALL way too much, to the point that its default behaviour will be the exact opposite of the current one: by default every CPU is housekeeping, so NO_HZ_FULL_ALL would have no effect anymore if we don't set housekeeping boot option.

Quoted 5 lines
Quoted 3 lines

Maybe a CONFIG_HOUSEKEEPING_BOOT_ONLY as a way to restrict housekeeping by default to just the boot cpu. In conjunction with NOHZ_FULL_ALL you would then get the expected semantics.

A big box with only the boot cpu for housekeeping is likely screwed.

Indeed we probably shouldn't introduce new config that affine housekeeping to a single CPU.

Quoted 1 line

Personally, I think NOHZ_FULL_ALL should just die.

Yeah, although it's still useful for automatic boot testing to detect issues with nohz_full on.

Thanks.

#29 lore raw
Quoted 4 lines · On Fri, 11 Aug 2017 17:01:30 +0200 Frederic Weisbecker <[email protected]>…
Quoted 1 line

Personally, I think NOHZ_FULL_ALL should just die.

Yeah, although it's still useful for automatic boot testing to detect issues with nohz_full on.

Maybe we could rename/modify it to be a boot-time testing option for nohz_full?

I think we may want to simplify config and kernel command-line options. I don't think it's a good idea to break down every single isolation feature into a kernel command-line option. What we want in the end is having the ability to remove all the noise from a CPU right? Why not have a single option for that?

#30 lore raw
In reply to Mike Galbraith
Quoted 11 lines · Mike Galbraith
Quoted 9 lines · Chris Metcalf
Quoted 5 lines · Frederic Weisbecker

But perhaps I should add a new NO_HZ_FULL_BUT_HOUSEKEEPING option. Otherwise we'll change the meaning of NO_HZ_FULL_ALL way too much, to the point that its default behaviour will be the exact opposite of the current one: by default every CPU is housekeeping, so NO_HZ_FULL_ALL would have no effect anymore if we don't set housekeeping boot option.

Maybe a CONFIG_HOUSEKEEPING_BOOT_ONLY as a way to restrict housekeeping by default to just the boot cpu. In conjunction with NOHZ_FULL_ALL you would then get the expected semantics.

A big box with only the boot cpu for housekeeping is likely screwed.

Fair point - this kind of configuration would be primarily useful for dedicated systems that were running a high-traffic-rate networking application on many cores, for example. In this mode you don't end up putting a lot of burden on the housekeeping core. In any case, probably not worth adding an additional kernel config for.

Signature · 2 lines
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com
#31 lore raw
Quoted 11 lines · Chris Metcalf
Quoted 5 lines
Quoted 4 lines

Maybe a CONFIG_HOUSEKEEPING_BOOT_ONLY as a way to restrict housekeeping by default to just the boot cpu. In conjunction with NOHZ_FULL_ALL you would
then get the expected semantics.

A big box with only the boot cpu for housekeeping is likely screwed.

Fair point - this kind of configuration would be primarily useful for dedicated systems that were running a high-traffic-rate networking application on many cores, for example. In this mode you don't end up putting a lot of burden on the housekeeping core. In any case, probably not worth adding an additional kernel config for.

The standard server config at this point is a two NUMA node with lots of cores on each. For such a thing a single housekeeping cpu is usually sufficient. Having a rather large number of NUMA nodes is unusual.

The question is also what is considered a "large" system at this point? Lots of cores? Lots of NUMA nodes?

Ah, Chris since you are here: What is happening with the dataplane patches?

#32 lore raw
Quoted 2 lines · Christopher Lameter

Ah, Chris since you are here: What is happening with the dataplane patches?

Work has been crazy and I keep expecting to have a chunk of time to work on it and it doesn't happen.

September is looking relatively good though for my having time to work on it. I really would like to get out a new spin. Fingers crossed.

Signature · 2 lines
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com