[PATCH 0/4] nohz: Full dynticks fixes/improvements

[PATCH 0/4] nohz: Full dynticks fixes/improvements

[Frederic Weisbecker]

Hi,

Changes against tip:timers/nohz :

* Fixes for timekeeping
* Handle RCU nocb restriction
* Improvements on info and tunables

I'll send these to Ingo for tip:timers/nohz in a few days if no
comment arise.

You can fetch from:

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

Thanks.

Frederic Weisbecker (4):
  nohz: Force boot CPU outside full dynticks range
  nohz: Print final full dynticks CPUs range on boot
  nohz: Ensure full dynticks CPUs are RCU nocbs
  nohz: New option to force all CPUs in full dynticks range

 Documentation/kernel-parameters.txt |    6 ++-
 include/linux/rcupdate.h            |    7 +++
 include/linux/tick.h                |    2 +
 init/main.c                         |    1 +
 kernel/rcutree.c                    |    6 +-
 kernel/rcutree.h                    |    1 -
 kernel/rcutree_plugin.h             |   13 ++----
 kernel/time/Kconfig                 |   10 +++++
 kernel/time/tick-sched.c            |   71 ++++++++++++++++++-----------------
 9 files changed, 67 insertions(+), 50 deletions(-)

-- 
1.7.5.4

[PATCH 1/4] nohz: Force boot CPU outside full dynticks range

[Frederic Weisbecker]

The timekeeping job must be able to run early on boot
because there may be some pre-SMP (and thus pre-initcalls )
components that rely on it. The IO-APIC is one such users
as it tests the timer health by watching jiffies progression.

Given that it happens before we know the initial online
set, we can't rely on it to select a timekeeper. We need
one before SMP time otherwise we simply crash on boot.

To fix this and keep things simple for now, force the boot CPU
outside of the full dynticks range in any case and do this early
on kernel parameter parsing time.

We might want a trickier solution later, expecially for aSMP
architectures that need to assign housekeeping tasks to arbitrary
low power CPUs.

But it's still first pass KISS time for now.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Geoff Levand <geoff@infradead.org>
Cc: Gilad Ben Yossef <gilad@benyossef.com>
Cc: Hakan Akkan <hakanakkan@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/kernel-parameters.txt |    4 +-
 kernel/time/tick-sched.c            |   52 ++++++++++-------------------------
 2 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 231698f..28e039c 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1916,8 +1916,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	nohz_extended=  [KNL,BOOT]
 			In kernels built with CONFIG_NO_HZ_EXTENDED=y, set
 			the specified list of CPUs whose tick will be stopped
-			whenever possible. You need to keep at least one online
-			CPU outside the range to maintain the timekeeping.
+			whenever possible. The boot CPU will be forced outside
+			the range to maintain the timekeeping.
 
 	noiotrap	[SH] Disables trapped I/O port accesses.
 
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 57bb3fe..74bc7f1 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -158,11 +158,21 @@ int tick_nohz_extended_cpu(int cpu)
 /* Parse the boot-time nohz CPU list from the kernel parameters. */
 static int __init tick_nohz_extended_setup(char *str)
 {
+	int cpu;
+
 	alloc_bootmem_cpumask_var(&nohz_extended_mask);
-	if (cpulist_parse(str, nohz_extended_mask) < 0)
+	if (cpulist_parse(str, nohz_extended_mask) < 0) {
 		pr_warning("NOHZ: Incorrect nohz_extended cpumask\n");
-	else
-		have_nohz_extended_mask = true;
+		return 1;
+	}
+
+	cpu = smp_processor_id();
+	if (cpumask_test_cpu(cpu, nohz_extended_mask)) {
+		pr_warning("NO_HZ: Clearing %d from nohz_extended range for timekeeping\n", cpu);
+		cpumask_clear_cpu(cpu, nohz_extended_mask);
+	}
+	have_nohz_extended_mask = true;
+
 	return 1;
 }
 __setup("nohz_extended=", tick_nohz_extended_setup);
@@ -188,42 +198,10 @@ static int __cpuinit tick_nohz_cpu_down_callback(struct notifier_block *nfb,
 
 static int __init init_tick_nohz_extended(void)
 {
-	cpumask_var_t online_nohz;
 	int cpu;
 
-	if (!have_nohz_extended_mask)
-		return 0;
-
-	cpu_notifier(tick_nohz_cpu_down_callback, 0);
-
-	if (!zalloc_cpumask_var(&online_nohz, GFP_KERNEL)) {
-		pr_warning("NO_HZ: Not enough memory to check extended nohz mask\n");
-		return -ENOMEM;
-	}
-
-	/*
-	 * CPUs can probably not be concurrently offlined on initcall time.
-	 * But we are paranoid, aren't we?
-	 */
-	get_online_cpus();
-
-	/* Ensure we keep a CPU outside the dynticks range for timekeeping */
-	cpumask_and(online_nohz, cpu_online_mask, nohz_extended_mask);
-	if (cpumask_equal(online_nohz, cpu_online_mask)) {
-		pr_warning("NO_HZ: Must keep at least one online CPU "
-			   "out of nohz_extended range\n");
-		/*
-		 * We know the current CPU doesn't have its tick stopped.
-		 * Let's use it for the timekeeping duty.
-		 */
-		preempt_disable();
-		cpu = smp_processor_id();
-		pr_warning("NO_HZ: Clearing %d from nohz_extended range\n", cpu);
-		cpumask_clear_cpu(cpu, nohz_extended_mask);
-		preempt_enable();
-	}
-	put_online_cpus();
-	free_cpumask_var(online_nohz);
+	if (have_nohz_extended_mask)
+		cpu_notifier(tick_nohz_cpu_down_callback, 0);
 
 	return 0;
 }
-- 
1.7.5.4

[PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Frederic Weisbecker]

Given that we are applying a few restrictions on the
full dynticks CPUs range (boot CPU excluded, then
soon the RCU nocb subset requirement), let's print
the final resulting range of full dynticks CPUs to
the user so that he knows what's really going to run.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Geoff Levand <geoff@infradead.org>
Cc: Gilad Ben Yossef <gilad@benyossef.com>
Cc: Hakan Akkan <hakanakkan@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/tick-sched.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 74bc7f1..1eb0e35 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -196,12 +196,24 @@ static int __cpuinit tick_nohz_cpu_down_callback(struct notifier_block *nfb,
 	return NOTIFY_OK;
 }
 
+/*
+ * Worst case string length in chunks of CPU range seems 2 steps
+ * separations: 0,2,4,6,...
+ * This is NR_CPUS + sizeof('\0')
+ */
+static char __initdata nohz_ext_buf[NR_CPUS + 1];
+
 static int __init init_tick_nohz_extended(void)
 {
 	int cpu;
 
-	if (have_nohz_extended_mask)
-		cpu_notifier(tick_nohz_cpu_down_callback, 0);
+	if (!have_nohz_extended_mask)
+		return 0;
+
+	cpu_notifier(tick_nohz_cpu_down_callback, 0);
+
+	cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
+	pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
 
 	return 0;
 }
-- 
1.7.5.4

[PATCH 3/4] nohz: Ensure full dynticks CPUs are RCU nocbs

[Frederic Weisbecker]

We need full dynticks CPU to also be RCU nocb so
that we don't have to keep the tick to handle RCU
callbacks.

Make sure the range passed to nohz_extended= boot
parameter is a subset of rcu_nocbs=

The CPUs that fail to meet this requirement will be
excluded from the nohz_extended range. This is checked
early in boot time, before any CPU has the opportunity
to stop its tick.

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Geoff Levand <geoff@infradead.org>
Cc: Gilad Ben Yossef <gilad@benyossef.com>
Cc: Hakan Akkan <hakanakkan@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/kernel-parameters.txt |    2 ++
 include/linux/rcupdate.h            |    7 +++++++
 include/linux/tick.h                |    2 ++
 init/main.c                         |    1 +
 kernel/rcutree.c                    |    6 +++---
 kernel/rcutree.h                    |    1 -
 kernel/rcutree_plugin.h             |   13 ++++---------
 kernel/time/tick-sched.c            |   16 +++++++++++-----
 8 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 28e039c..80d17d0 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1918,6 +1918,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			the specified list of CPUs whose tick will be stopped
 			whenever possible. The boot CPU will be forced outside
 			the range to maintain the timekeeping.
+			The CPUs in this range must also be included in the
+			rcu_nocbs= set.
 
 	noiotrap	[SH] Disables trapped I/O port accesses.
 
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index b758ce1..8e0948c 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -999,4 +999,11 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
 #define kfree_rcu(ptr, rcu_head)					\
 	__kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
 
+#ifdef CONFIG_RCU_NOCB_CPU
+extern bool rcu_is_nocb_cpu(int cpu);
+#else
+static inline bool rcu_is_nocb_cpu(int cpu) { return false; }
+#endif /* #else #ifdef CONFIG_RCU_NOCB_CPU */
+
+
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/tick.h b/include/linux/tick.h
index 44bfa8a..7484034 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -158,8 +158,10 @@ static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) { return -1; }
 # endif /* !NO_HZ */
 
 #ifdef CONFIG_NO_HZ_EXTENDED
+extern void tick_nohz_init(void);
 extern int tick_nohz_extended_cpu(int cpu);
 #else
+static inline void tick_nohz_init(void) { }
 static inline int tick_nohz_extended_cpu(int cpu) { return 0; }
 #endif
 
diff --git a/init/main.c b/init/main.c
index 63534a1..2acb5bb 100644
--- a/init/main.c
+++ b/init/main.c
@@ -547,6 +547,7 @@ asmlinkage void __init start_kernel(void)
 	idr_init_cache();
 	perf_event_init();
 	rcu_init();
+	tick_nohz_init();
 	radix_tree_init();
 	/* init some links before init_ISA_irqs() */
 	early_irq_init();
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 5b8ad82..7cd29b9 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1685,7 +1685,7 @@ rcu_send_cbs_to_orphanage(int cpu, struct rcu_state *rsp,
 			  struct rcu_node *rnp, struct rcu_data *rdp)
 {
 	/* No-CBs CPUs do not have orphanable callbacks. */
-	if (is_nocb_cpu(rdp->cpu))
+	if (rcu_is_nocb_cpu(rdp->cpu))
 		return;
 
 	/*
@@ -2747,10 +2747,10 @@ static void _rcu_barrier(struct rcu_state *rsp)
 	 * corresponding CPU's preceding callbacks have been invoked.
 	 */
 	for_each_possible_cpu(cpu) {
-		if (!cpu_online(cpu) && !is_nocb_cpu(cpu))
+		if (!cpu_online(cpu) && !rcu_is_nocb_cpu(cpu))
 			continue;
 		rdp = per_cpu_ptr(rsp->rda, cpu);
-		if (is_nocb_cpu(cpu)) {
+		if (rcu_is_nocb_cpu(cpu)) {
 			_rcu_barrier_trace(rsp, "OnlineNoCB", cpu,
 					   rsp->n_barrier_done);
 			atomic_inc(&rsp->barrier_cpu_count);
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index c896b50..57300ad 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -529,7 +529,6 @@ static void print_cpu_stall_info(struct rcu_state *rsp, int cpu);
 static void print_cpu_stall_info_end(void);
 static void zero_cpu_stall_ticks(struct rcu_data *rdp);
 static void increment_cpu_stall_ticks(void);
-static bool is_nocb_cpu(int cpu);
 static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
 			    bool lazy);
 static bool rcu_nocb_adopt_orphan_cbs(struct rcu_state *rsp,
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index c1cc7e1..1dec0b2 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -2166,7 +2166,7 @@ static int __init parse_rcu_nocb_poll(char *arg)
 early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
 
 /* Is the specified CPU a no-CPUs CPU? */
-static bool is_nocb_cpu(int cpu)
+bool rcu_is_nocb_cpu(int cpu)
 {
 	if (have_rcu_nocb_mask)
 		return cpumask_test_cpu(cpu, rcu_nocb_mask);
@@ -2224,7 +2224,7 @@ static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
 			    bool lazy)
 {
 
-	if (!is_nocb_cpu(rdp->cpu))
+	if (!rcu_is_nocb_cpu(rdp->cpu))
 		return 0;
 	__call_rcu_nocb_enqueue(rdp, rhp, &rhp->next, 1, lazy);
 	return 1;
@@ -2241,7 +2241,7 @@ static bool __maybe_unused rcu_nocb_adopt_orphan_cbs(struct rcu_state *rsp,
 	long qll = rsp->qlen_lazy;
 
 	/* If this is not a no-CBs CPU, tell the caller to do it the old way. */
-	if (!is_nocb_cpu(smp_processor_id()))
+	if (!rcu_is_nocb_cpu(smp_processor_id()))
 		return 0;
 	rsp->qlen = 0;
 	rsp->qlen_lazy = 0;
@@ -2281,7 +2281,7 @@ static bool nocb_cpu_expendable(int cpu)
 	 * If there are no no-CB CPUs or if this CPU is not a no-CB CPU,
 	 * then offlining this CPU is harmless.  Let it happen.
 	 */
-	if (!have_rcu_nocb_mask || is_nocb_cpu(cpu))
+	if (!have_rcu_nocb_mask || rcu_is_nocb_cpu(cpu))
 		return 1;
 
 	/* If no memory, play it safe and keep the CPU around. */
@@ -2463,11 +2463,6 @@ static void __init rcu_init_nocb(void)
 
 #else /* #ifdef CONFIG_RCU_NOCB_CPU */
 
-static bool is_nocb_cpu(int cpu)
-{
-	return false;
-}
-
 static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
 			    bool lazy)
 {
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 1eb0e35..a76d95e 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -203,21 +203,27 @@ static int __cpuinit tick_nohz_cpu_down_callback(struct notifier_block *nfb,
  */
 static char __initdata nohz_ext_buf[NR_CPUS + 1];
 
-static int __init init_tick_nohz_extended(void)
+void __init tick_nohz_init(void)
 {
 	int cpu;
 
 	if (!have_nohz_extended_mask)
-		return 0;
+		return;
 
 	cpu_notifier(tick_nohz_cpu_down_callback, 0);
 
+	/* Make sure full dynticks CPU are also RCU nocbs */
+	for_each_cpu(cpu, nohz_extended_mask) {
+		if (!rcu_is_nocb_cpu(cpu)) {
+			pr_warning("NO_HZ: CPU %d is not RCU nocb: "
+				   "cleared from nohz_extended range", cpu);
+			cpumask_clear_cpu(cpu, nohz_extended_mask);
+		}
+	}
+
 	cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
 	pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
-
-	return 0;
 }
-core_initcall(init_tick_nohz_extended);
 #else
 #define have_nohz_extended_mask (0)
 #endif
-- 
1.7.5.4

[PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Frederic Weisbecker]

Provide a new kernel config that forces all CPUs to be part
of the full dynticks range, except the boot one for timekeeping.

This is helpful for those who don't need a finegrained range
of full dynticks CPU and also for automated testing.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Geoff Levand <geoff@infradead.org>
Cc: Gilad Ben Yossef <gilad@benyossef.com>
Cc: Hakan Akkan <hakanakkan@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/Kconfig      |   10 ++++++++++
 kernel/time/tick-sched.c |    5 +++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 5a87c03..407d771 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -98,6 +98,16 @@ config NO_HZ_EXTENDED
 
 	 Say N.
 
+config NO_HZ_EXTENDED_ALL
+       bool "Full dynticks system on all CPUs"
+       depends on NO_HZ_EXTENDED
+       help
+	 Force all CPUs to be full dynticks. The range specified in the
+	 nohz_extended boot option will then be ignored.
+
+	 Note the boot CPU will still be kept outside the range to handle
+	 the timekeeping duty.
+
 config HIGH_RES_TIMERS
 	bool "High Resolution Timer Support"
 	depends on !ARCH_USES_GETTIMEOFFSET && GENERIC_CLOCKEVENTS
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index a76d95e..0a3952c 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -161,10 +161,15 @@ static int __init tick_nohz_extended_setup(char *str)
 	int cpu;
 
 	alloc_bootmem_cpumask_var(&nohz_extended_mask);
+
+#ifdef CONFIG_NO_HZ_EXTENDED_ALL
+	cpumask_setall(nohz_extended_mask);
+#else
 	if (cpulist_parse(str, nohz_extended_mask) < 0) {
 		pr_warning("NOHZ: Incorrect nohz_extended cpumask\n");
 		return 1;
 	}
+#endif
 
 	cpu = smp_processor_id();
 	if (cpumask_test_cpu(cpu, nohz_extended_mask)) {
-- 
1.7.5.4

Re: [PATCH 1/4] nohz: Force boot CPU outside full dynticks range

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> The timekeeping job must be able to run early on boot
> because there may be some pre-SMP (and thus pre-initcalls )
> components that rely on it. The IO-APIC is one such users
> as it tests the timer health by watching jiffies progression.

Btw., while I agree that a conservative mode is probably wise for bootup, 
that IO-APIC assumption could be fixed or even removed.

If the IO-APIC code wants to know whether an interrupt fired, it can take 
a look at the kstat_irqs numbers?

Also, could we restrict the boot CPU's mode only during the early bootup 
stage - i.e. until we are ready to execute user-space init?

Thaks,

	Ingo

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> +	cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
> +	pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);

I'd suggest removing the 'experimental' word. We are not sending anything 
experimental to Linus: we'll send something that is tested and that we 
expect to not break anything. (and which we'll fix if it does)

Thanks,

	Ingo

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> +config NO_HZ_EXTENDED_ALL
> +       bool "Full dynticks system on all CPUs"
> +       depends on NO_HZ_EXTENDED
> +       help
> +	 Force all CPUs to be full dynticks. The range specified in the
> +	 nohz_extended boot option will then be ignored.
> +
> +	 Note the boot CPU will still be kept outside the range to handle
> +	 the timekeeping duty.

In most cases this will be the first time users doing 'make oldconfig' 
will hear about this feature, so I'd not mention noh_extended in such a 
primary way at all.

Instead I'd suggest to explain, in a few simple sentences, what 'full 
dynticks' _is_. Then maybe mention this near the end of the next:

   ( Alternatively this feature can also be enabled via the 
     nohz_extended=<cpulist> boot option. If this kernel config option is 
     enabled then the nohz_extended boot parameter is ignored. )

Btw., while at it - I think the nohz_extended boot parameter should _not_ 
be ignored. The .config option should simply provide a default, for the 
case that the user does not have any boot option specified.

If the user specifies a boot option - say to work around a rare installer 
hang in a distro install kernel - then we very much _dont_ want to ignore 
it.

So please put some effort into how this feature is going to interface with 
and be represented to actual users and distro kernel maintainers.

Thanks,

	Ingo

Re: [PATCH 1/4] nohz: Force boot CPU outside full dynticks range

[Frederic Weisbecker]

2013/3/28 Ingo Molnar <mingo@kernel.org>:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
>> The timekeeping job must be able to run early on boot
>> because there may be some pre-SMP (and thus pre-initcalls )
>> components that rely on it. The IO-APIC is one such users
>> as it tests the timer health by watching jiffies progression.
>
> Btw., while I agree that a conservative mode is probably wise for bootup,
> that IO-APIC assumption could be fixed or even removed.
>
> If the IO-APIC code wants to know whether an interrupt fired, it can take
> a look at the kstat_irqs numbers?

Good point. Still I need to let timekeeping working early to avoid
more surprises.

>
> Also, could we restrict the boot CPU's mode only during the early bootup
> stage - i.e. until we are ready to execute user-space init?

That's a tricky issue. Let's consider the boot CPU is the timekeeper
in the beginning. Later reassigning the timekeeping duty to another
CPU require some careful treatment because we may do that remotely and
we want to keep tick_do_timer_cpu lockless. This may involve an IPI
coupled with proper memory ordering if we don't want to race with
dynticks idle (or even full dynticks).

It's on the long term plan but I thought about dealing with that later
once we get the basic feature working. But if you prefer I can work on
that now.

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Frederic Weisbecker]

2013/3/28 Ingo Molnar <mingo@kernel.org>:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
>> +     cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
>> +     pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
>
> I'd suggest removing the 'experimental' word. We are not sending anything
> experimental to Linus: we'll send something that is tested and that we
> expect to not break anything. (and which we'll fix if it does)

Ok. Just wanted to warn the user that feature is still young and
hasn't yet passed through many real world workloads. But the Kconfig
is probably enough for that.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Frederic Weisbecker]

2013/3/28 Ingo Molnar <mingo@kernel.org>:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
>> +config NO_HZ_EXTENDED_ALL
>> +       bool "Full dynticks system on all CPUs"
>> +       depends on NO_HZ_EXTENDED
>> +       help
>> +      Force all CPUs to be full dynticks. The range specified in the
>> +      nohz_extended boot option will then be ignored.
>> +
>> +      Note the boot CPU will still be kept outside the range to handle
>> +      the timekeeping duty.
>
> In most cases this will be the first time users doing 'make oldconfig'
> will hear about this feature, so I'd not mention noh_extended in such a
> primary way at all.
>
> Instead I'd suggest to explain, in a few simple sentences, what 'full
> dynticks' _is_. Then maybe mention this near the end of the next:
>
>    ( Alternatively this feature can also be enabled via the
>      nohz_extended=<cpulist> boot option. If this kernel config option is
>      enabled then the nohz_extended boot parameter is ignored. )

Ok. Hmm, concerning what full dynticks is, those who run make
oldconfig should have passed through CONFIG_NO_HZ_EXTENDED help first
so they should know what this is all about, right? I can improve its
help text if you think something need more details. Or you think I
should add some more reminder in the new config? Also nohz_extended is
not mentioned there so I indeed need to fix that. And then do a
reminder in the new config. I'll reuse a bit your above sentence.

>
> Btw., while at it - I think the nohz_extended boot parameter should _not_
> be ignored. The .config option should simply provide a default, for the
> case that the user does not have any boot option specified.

Agreed.

>
> If the user specifies a boot option - say to work around a rare installer
> hang in a distro install kernel - then we very much _dont_ want to ignore
> it.
>
> So please put some effort into how this feature is going to interface with
> and be represented to actual users and distro kernel maintainers.

Ok, will improve that.

Thanks.

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Paul Gortmaker]

[Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 08:40) Ingo Molnar wrote:

> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > +	cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
> > +	pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
> 
> I'd suggest removing the 'experimental' word. We are not sending anything 
> experimental to Linus: we'll send something that is tested and that we 
> expect to not break anything. (and which we'll fix if it does)

In that case, should we vector a patch through PaulM to clobber these too?

$ git grep xperiment |grep rcu
kernel/rcutree_plugin.h: printk(KERN_INFO "\tExperimental boot-time adjustment of leaf fanout to %d.\n", rcu_fanout_leaf);
kernel/rcutree_plugin.h: pr_info("\tExperimental no-CBs CPUs: %s.\n", nocb_buf);
kernel/rcutree_plugin.h: pr_info("\tExperimental polled no-CBs CPUs.\n");

Paul.

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Frederic Weisbecker]

2013/3/29 Paul Gortmaker <paul.gortmaker@windriver.com>:
> [Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 08:40) Ingo Molnar wrote:
>
>>
>> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>>
>> > +   cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
>> > +   pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
>>
>> I'd suggest removing the 'experimental' word. We are not sending anything
>> experimental to Linus: we'll send something that is tested and that we
>> expect to not break anything. (and which we'll fix if it does)
>
> In that case, should we vector a patch through PaulM to clobber these too?
>
> $ git grep xperiment |grep rcu
> kernel/rcutree_plugin.h: printk(KERN_INFO "\tExperimental boot-time adjustment of leaf fanout to %d.\n", rcu_fanout_leaf);
> kernel/rcutree_plugin.h: pr_info("\tExperimental no-CBs CPUs: %s.\n", nocb_buf);
> kernel/rcutree_plugin.h: pr_info("\tExperimental polled no-CBs CPUs.\n");

Hehe, that's indeed what I inspired from ;-)

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Paul E. McKenney]

On Fri, Mar 29, 2013 at 01:39:04AM +0100, Frederic Weisbecker wrote:
> 2013/3/29 Paul Gortmaker <paul.gortmaker@windriver.com>:
> > [Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 08:40) Ingo Molnar wrote:
> >
> >>
> >> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >>
> >> > +   cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
> >> > +   pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
> >>
> >> I'd suggest removing the 'experimental' word. We are not sending anything
> >> experimental to Linus: we'll send something that is tested and that we
> >> expect to not break anything. (and which we'll fix if it does)
> >
> > In that case, should we vector a patch through PaulM to clobber these too?
> >
> > $ git grep xperiment |grep rcu
> > kernel/rcutree_plugin.h: printk(KERN_INFO "\tExperimental boot-time adjustment of leaf fanout to %d.\n", rcu_fanout_leaf);

This one has been this way for quite some time.  I must confess that I am
more inclined to remove this Kconfig option entirely than I am to remove
the "Experimental".  ;-)

> > kernel/rcutree_plugin.h: pr_info("\tExperimental no-CBs CPUs: %s.\n", nocb_buf);
> > kernel/rcutree_plugin.h: pr_info("\tExperimental polled no-CBs CPUs.\n");
> 
> Hehe, that's indeed what I inspired from ;-)

I guess we can rerun the earlier "how to mark Kconfig options as not
yet being ready for 100 million unsuspecting victims^Wusers.  ;-)

Then again, I do have the warning at the bottom of the help info,
so might be OK removing the "Experimental".

							Thanx, Paul

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Paul Gortmaker]

[Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 29/03/2013 (Fri 01:39) Frederic Weisbecker wrote:

> 2013/3/29 Paul Gortmaker <paul.gortmaker@windriver.com>:
> > [Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 08:40) Ingo Molnar wrote:
> >
> >>
> >> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >>
> >> > +   cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
> >> > +   pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
> >>
> >> I'd suggest removing the 'experimental' word. We are not sending anything
> >> experimental to Linus: we'll send something that is tested and that we
> >> expect to not break anything. (and which we'll fix if it does)
> >
> > In that case, should we vector a patch through PaulM to clobber these too?
> >
> > $ git grep xperiment |grep rcu
> > kernel/rcutree_plugin.h: printk(KERN_INFO "\tExperimental boot-time adjustment of leaf fanout to %d.\n", rcu_fanout_leaf);
> > kernel/rcutree_plugin.h: pr_info("\tExperimental no-CBs CPUs: %s.\n", nocb_buf);
> > kernel/rcutree_plugin.h: pr_info("\tExperimental polled no-CBs CPUs.\n");
> 
> Hehe, that's indeed what I inspired from ;-)

I was guessing exactly that, which is why I mentioned it.  :)

P.
--

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Paul Gortmaker]

[Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 19:00) Paul E. McKenney wrote:

> On Fri, Mar 29, 2013 at 01:39:04AM +0100, Frederic Weisbecker wrote:
> > 2013/3/29 Paul Gortmaker <paul.gortmaker@windriver.com>:
> > > [Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 08:40) Ingo Molnar wrote:
> > >
> > >>
> > >> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > >>
> > >> > +   cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
> > >> > +   pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
> > >>
> > >> I'd suggest removing the 'experimental' word. We are not sending anything
> > >> experimental to Linus: we'll send something that is tested and that we
> > >> expect to not break anything. (and which we'll fix if it does)
> > >
> > > In that case, should we vector a patch through PaulM to clobber these too?
> > >
> > > $ git grep xperiment |grep rcu
> > > kernel/rcutree_plugin.h: printk(KERN_INFO "\tExperimental boot-time adjustment of leaf fanout to %d.\n", rcu_fanout_leaf);
> 
> This one has been this way for quite some time.  I must confess that I am
> more inclined to remove this Kconfig option entirely than I am to remove
> the "Experimental".  ;-)
> 
> > > kernel/rcutree_plugin.h: pr_info("\tExperimental no-CBs CPUs: %s.\n", nocb_buf);
> > > kernel/rcutree_plugin.h: pr_info("\tExperimental polled no-CBs CPUs.\n");
> > 
> > Hehe, that's indeed what I inspired from ;-)
> 
> I guess we can rerun the earlier "how to mark Kconfig options as not
> yet being ready for 100 million unsuspecting victims^Wusers.  ;-)

Perhaps consider "hiding" these kinds of choices behind CONFIG_EXPERT ?

P.
--

> 
> Then again, I do have the warning at the bottom of the help info,
> so might be OK removing the "Experimental".
> 
> 							Thanx, Paul
> 

Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot

[Paul E. McKenney]

On Thu, Mar 28, 2013 at 10:08:23PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 19:00) Paul E. McKenney wrote:
> 
> > On Fri, Mar 29, 2013 at 01:39:04AM +0100, Frederic Weisbecker wrote:
> > > 2013/3/29 Paul Gortmaker <paul.gortmaker@windriver.com>:
> > > > [Re: [PATCH 2/4] nohz: Print final full dynticks CPUs range on boot] On 28/03/2013 (Thu 08:40) Ingo Molnar wrote:
> > > >
> > > >>
> > > >> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > >>
> > > >> > +   cpulist_scnprintf(nohz_ext_buf, sizeof(nohz_ext_buf), nohz_extended_mask);
> > > >> > +   pr_info("NO_HZ: Experimental full dynticks CPUs: %s.\n", nohz_ext_buf);
> > > >>
> > > >> I'd suggest removing the 'experimental' word. We are not sending anything
> > > >> experimental to Linus: we'll send something that is tested and that we
> > > >> expect to not break anything. (and which we'll fix if it does)
> > > >
> > > > In that case, should we vector a patch through PaulM to clobber these too?
> > > >
> > > > $ git grep xperiment |grep rcu
> > > > kernel/rcutree_plugin.h: printk(KERN_INFO "\tExperimental boot-time adjustment of leaf fanout to %d.\n", rcu_fanout_leaf);
> > 
> > This one has been this way for quite some time.  I must confess that I am
> > more inclined to remove this Kconfig option entirely than I am to remove
> > the "Experimental".  ;-)
> > 
> > > > kernel/rcutree_plugin.h: pr_info("\tExperimental no-CBs CPUs: %s.\n", nocb_buf);
> > > > kernel/rcutree_plugin.h: pr_info("\tExperimental polled no-CBs CPUs.\n");
> > > 
> > > Hehe, that's indeed what I inspired from ;-)
> > 
> > I guess we can rerun the earlier "how to mark Kconfig options as not
> > yet being ready for 100 million unsuspecting victims^Wusers.  ;-)
> 
> Perhaps consider "hiding" these kinds of choices behind CONFIG_EXPERT ?

We tried that with CONFIG_EXPERIMENTAL (or something like that), and
what happened was that all distros eventually enabled CONFIG_EXPERIMENTAL.

But if I was -really- serious about keeping the testing population down,
I would take Matthew Garret's advice and force a splat during boot-up.
So I now have a commit for 3.11 queued to remove the "Experimental"
from these strings, with your Reported-by.

							Thanx, Paul

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> 2013/3/28 Ingo Molnar <mingo@kernel.org>:
> >
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >
> >> +config NO_HZ_EXTENDED_ALL
> >> +       bool "Full dynticks system on all CPUs"
> >> +       depends on NO_HZ_EXTENDED
> >> +       help
> >> +      Force all CPUs to be full dynticks. The range specified in the
> >> +      nohz_extended boot option will then be ignored.
> >> +
> >> +      Note the boot CPU will still be kept outside the range to handle
> >> +      the timekeeping duty.
> >
> > In most cases this will be the first time users doing 'make oldconfig'
> > will hear about this feature, so I'd not mention noh_extended in such a
> > primary way at all.
> >
> > Instead I'd suggest to explain, in a few simple sentences, what 'full
> > dynticks' _is_. Then maybe mention this near the end of the next:
> >
> >    ( Alternatively this feature can also be enabled via the
> >      nohz_extended=<cpulist> boot option. If this kernel config option is
> >      enabled then the nohz_extended boot parameter is ignored. )
> 
> Ok. Hmm, concerning what full dynticks is, those who run make oldconfig 
> should have passed through CONFIG_NO_HZ_EXTENDED help first so they 
> should know what this is all about, right? I can improve its help text 
> if you think something need more details. Or you think I should add some 
> more reminder in the new config? Also nohz_extended is not mentioned 
> there so I indeed need to fix that. And then do a reminder in the new 
> config. I'll reuse a bit your above sentence.

Kconfig option + boot parameter, or kconfig option + kconfig option, just 
to get a feature to activate, is usability madness.

I think we need _one_ config knob to configure, which, if selected, 
activates all of this feature. Preferably this would be similar to the 
preempt options, like CONFIG_PREEMPT is the most advanced preemption 
model:

          ( ) No Forced Preemption (Server)
          (X) Voluntary Kernel Preemption (Desktop)
          ( ) Preemptible Kernel (Low-Latency Desktop)

NO_HZ_EXTENDED or NO_HZ_FULL could be a third variant to:

          ( ) Periodic Timer Ticks
          (X) Tickless Idle (Dynamic Ticks)
          ( ) Full Tickless (Extended Dynamic Ticks)

or so - instead of this somewhat confusing hierarchy of config and boot 
options. Good help text would explain the differences between then.

Internally, the 'Tickless Idle' option would cause NO_HZ to be set, while 
'Full Tickless' would also set NO_HZ, but would also set NO_HZ_EXTENDED.

Furthermore, right now it's hard to enable NO_HZ_EXTENDED, because it 
depends on so many other options. The option is just hidden unless the 
user knows about it and enables 5 other kernel options. Instead the tick 
mode selected should enable (select) any required features - it should 
drive things, not the other way around.

(After all that is done, the boot option is something that augments this 
existing, kconfig driven mechanism.)

Thanks,

	Ingo

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Frederic Weisbecker]

2013/3/30 Ingo Molnar <mingo@kernel.org>:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
>> 2013/3/28 Ingo Molnar <mingo@kernel.org>:
>> >
>> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> >
>> >> +config NO_HZ_EXTENDED_ALL
>> >> +       bool "Full dynticks system on all CPUs"
>> >> +       depends on NO_HZ_EXTENDED
>> >> +       help
>> >> +      Force all CPUs to be full dynticks. The range specified in the
>> >> +      nohz_extended boot option will then be ignored.
>> >> +
>> >> +      Note the boot CPU will still be kept outside the range to handle
>> >> +      the timekeeping duty.
>> >
>> > In most cases this will be the first time users doing 'make oldconfig'
>> > will hear about this feature, so I'd not mention noh_extended in such a
>> > primary way at all.
>> >
>> > Instead I'd suggest to explain, in a few simple sentences, what 'full
>> > dynticks' _is_. Then maybe mention this near the end of the next:
>> >
>> >    ( Alternatively this feature can also be enabled via the
>> >      nohz_extended=<cpulist> boot option. If this kernel config option is
>> >      enabled then the nohz_extended boot parameter is ignored. )
>>
>> Ok. Hmm, concerning what full dynticks is, those who run make oldconfig
>> should have passed through CONFIG_NO_HZ_EXTENDED help first so they
>> should know what this is all about, right? I can improve its help text
>> if you think something need more details. Or you think I should add some
>> more reminder in the new config? Also nohz_extended is not mentioned
>> there so I indeed need to fix that. And then do a reminder in the new
>> config. I'll reuse a bit your above sentence.
>
> Kconfig option + boot parameter, or kconfig option + kconfig option, just
> to get a feature to activate, is usability madness.
>
> I think we need _one_ config knob to configure, which, if selected,
> activates all of this feature. Preferably this would be similar to the
> preempt options, like CONFIG_PREEMPT is the most advanced preemption
> model:
>
>           ( ) No Forced Preemption (Server)
>           (X) Voluntary Kernel Preemption (Desktop)
>           ( ) Preemptible Kernel (Low-Latency Desktop)
>
> NO_HZ_EXTENDED or NO_HZ_FULL could be a third variant to:
>
>           ( ) Periodic Timer Ticks
>           (X) Tickless Idle (Dynamic Ticks)
>           ( ) Full Tickless (Extended Dynamic Ticks)
>
> or so - instead of this somewhat confusing hierarchy of config and boot
> options. Good help text would explain the differences between then.
>
> Internally, the 'Tickless Idle' option would cause NO_HZ to be set, while
> 'Full Tickless' would also set NO_HZ, but would also set NO_HZ_EXTENDED.
>
> Furthermore, right now it's hard to enable NO_HZ_EXTENDED, because it
> depends on so many other options. The option is just hidden unless the
> user knows about it and enables 5 other kernel options. Instead the tick
> mode selected should enable (select) any required features - it should
> drive things, not the other way around.
>
> (After all that is done, the boot option is something that augments this
> existing, kconfig driven mechanism.)

All agreed, I'm fixing these issues.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Christoph Lameter]

I think we could simplify things quite a bit if we either

1. Add any cpus specified with nohz_full/extended=xxx to
rcu_nocb. No check is then necessary anymore.

or

2. Avoid the setting of cpus entirely? If full nohz mode is desired
then pick one cpu (f.e. the first one or the one that is used for xtime
updates) and then make all other cpus nohz. Set the affinity mask for the
rcuoXXX threads to that cpu.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Paul Gortmaker]

On 13-04-08 10:57 AM, Christoph Lameter wrote:
> I think we could simplify things quite a bit if we either
> 
> 1. Add any cpus specified with nohz_full/extended=xxx to
> rcu_nocb. No check is then necessary anymore.
> 
> or
> 
> 2. Avoid the setting of cpus entirely? If full nohz mode is desired
> then pick one cpu (f.e. the first one or the one that is used for xtime
> updates) and then make all other cpus nohz. Set the affinity mask for the
> rcuoXXX threads to that cpu.

I can imagine people with multi socket systems wanting to have
a system partitioned with one "normal" core per physical socket,
for timekeeping, RCU threads, etc, but #2 would prevent that.

Paul.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Christoph Lameter]

On Tue, 9 Apr 2013, Paul Gortmaker wrote:

> > 2. Avoid the setting of cpus entirely? If full nohz mode is desired
> > then pick one cpu (f.e. the first one or the one that is used for xtime
> > updates) and then make all other cpus nohz. Set the affinity mask for the
> > rcuoXXX threads to that cpu.
>
> I can imagine people with multi socket systems wanting to have
> a system partitioned with one "normal" core per physical socket,
> for timekeeping, RCU threads, etc, but #2 would prevent that.

That is a good point. The kernel needs to run per node threads for I/O and
reclaim which could have their home there. Just had some bad experience
with latency introduced by the block layer redirecting I/O to the first
processor of a node. Maybe we can adopt that convention and relocate
kernel processing as much as possible to the first processor of a
node?

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Frederic Weisbecker]

On Mon, Apr 08, 2013 at 02:57:18PM +0000, Christoph Lameter wrote:
> I think we could simplify things quite a bit if we either
> 
> 1. Add any cpus specified with nohz_full/extended=xxx to
> rcu_nocb. No check is then necessary anymore.

Yeah in the long term we probably want that indeed.

Thanks.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Paul E. McKenney]

On Thu, Apr 11, 2013 at 05:19:59PM +0200, Frederic Weisbecker wrote:
> On Mon, Apr 08, 2013 at 02:57:18PM +0000, Christoph Lameter wrote:
> > I think we could simplify things quite a bit if we either
> > 
> > 1. Add any cpus specified with nohz_full/extended=xxx to
> > rcu_nocb. No check is then necessary anymore.
> 
> Yeah in the long term we probably want that indeed.

Or just build with RCU_NOCB_CPU_ALL=y to unconditionally offload all
the CPUs when using nohz_extended.

							Thanx, Paul

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Frederic Weisbecker]

On Thu, Apr 11, 2013 at 08:37:21AM -0700, Paul E. McKenney wrote:
> On Thu, Apr 11, 2013 at 05:19:59PM +0200, Frederic Weisbecker wrote:
> > On Mon, Apr 08, 2013 at 02:57:18PM +0000, Christoph Lameter wrote:
> > > I think we could simplify things quite a bit if we either
> > > 
> > > 1. Add any cpus specified with nohz_full/extended=xxx to
> > > rcu_nocb. No check is then necessary anymore.
> > 
> > Yeah in the long term we probably want that indeed.
> 
> Or just build with RCU_NOCB_CPU_ALL=y to unconditionally offload all
> the CPUs when using nohz_extended.

If there is no performance issue with that I'm all for it.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Paul E. McKenney]

On Thu, Apr 11, 2013 at 05:53:53PM +0200, Frederic Weisbecker wrote:
> On Thu, Apr 11, 2013 at 08:37:21AM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 11, 2013 at 05:19:59PM +0200, Frederic Weisbecker wrote:
> > > On Mon, Apr 08, 2013 at 02:57:18PM +0000, Christoph Lameter wrote:
> > > > I think we could simplify things quite a bit if we either
> > > > 
> > > > 1. Add any cpus specified with nohz_full/extended=xxx to
> > > > rcu_nocb. No check is then necessary anymore.
> > > 
> > > Yeah in the long term we probably want that indeed.
> > 
> > Or just build with RCU_NOCB_CPU_ALL=y to unconditionally offload all
> > the CPUs when using nohz_extended.
> 
> If there is no performance issue with that I'm all for it.

We won't know until a bunch of people try it.  ;-)

							Thanx, Paul

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Christoph Lameter]

On Thu, 11 Apr 2013, Frederic Weisbecker wrote:

> If there is no performance issue with that I'm all for it.

Or have a

CONFIG_LOWLATENCY

that boots up a kernel with the proper configuration?

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Frederic Weisbecker]

On Thu, Apr 11, 2013 at 04:41:07PM +0000, Christoph Lameter wrote:
> On Thu, 11 Apr 2013, Frederic Weisbecker wrote:
> 
> > If there is no performance issue with that I'm all for it.
> 
> Or have a
> 
> CONFIG_LOWLATENCY
> 
> that boots up a kernel with the proper configuration?

It may be too general for a naming. But I don't mind just
selecting CONFIG_RCU_NOCBS_ALL unconditionally. It's easily
changed in the future if anybody complains.

Btw, if CONFIG_RCU_NOCBS_ALL is set, the rcu_nocbs= parameter
is ignored, right? If you want to keep that direction and not
override the Kconfig choice, may be warn the user about that
if the boot parameter is passed?

Thanks.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Paul E. McKenney]

On Thu, Apr 11, 2013 at 07:04:23PM +0200, Frederic Weisbecker wrote:
> On Thu, Apr 11, 2013 at 04:41:07PM +0000, Christoph Lameter wrote:
> > On Thu, 11 Apr 2013, Frederic Weisbecker wrote:
> > 
> > > If there is no performance issue with that I'm all for it.
> > 
> > Or have a
> > 
> > CONFIG_LOWLATENCY
> > 
> > that boots up a kernel with the proper configuration?
> 
> It may be too general for a naming. But I don't mind just
> selecting CONFIG_RCU_NOCBS_ALL unconditionally. It's easily
> changed in the future if anybody complains.
> 
> Btw, if CONFIG_RCU_NOCBS_ALL is set, the rcu_nocbs= parameter
> is ignored, right? If you want to keep that direction and not
> override the Kconfig choice, may be warn the user about that
> if the boot parameter is passed?

Fair point.  Let me think about how I should approach this.

							Thanx, Paul

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Frederic Weisbecker]

On Thu, Apr 11, 2013 at 10:11:53AM -0700, Paul E. McKenney wrote:
> On Thu, Apr 11, 2013 at 07:04:23PM +0200, Frederic Weisbecker wrote:
> > On Thu, Apr 11, 2013 at 04:41:07PM +0000, Christoph Lameter wrote:
> > > On Thu, 11 Apr 2013, Frederic Weisbecker wrote:
> > > 
> > > > If there is no performance issue with that I'm all for it.
> > > 
> > > Or have a
> > > 
> > > CONFIG_LOWLATENCY
> > > 
> > > that boots up a kernel with the proper configuration?
> > 
> > It may be too general for a naming. But I don't mind just
> > selecting CONFIG_RCU_NOCBS_ALL unconditionally. It's easily
> > changed in the future if anybody complains.
> > 
> > Btw, if CONFIG_RCU_NOCBS_ALL is set, the rcu_nocbs= parameter
> > is ignored, right? If you want to keep that direction and not
> > override the Kconfig choice, may be warn the user about that
> > if the boot parameter is passed?
> 
> Fair point.  Let me think about how I should approach Thanx.

An alternative is to treat the Kconfig choice as a default setting
that gets overriden by rcu_nocbs=

This would require to refactor the three way Kconfig layout you've set,
but that aligns with what Ingo is suggesting me to do with the full dynticks
range: having CONFIG_NO_HZ_EXTENDED_ALL that is overriden by nohz_extended=
if any (beware though, "extended" will soon be renamed to "full", lets hope
it's our last take ;)

That unifies both behaviours and it looks more flexible to me.

Of course that means selecting CONFIG_RCU_NOCBS_ALL won't be enough for
me to ensure my nohz range is also nocb, I'm fine with the cpumask check
on boot though.

Anyway, that's just a suggestion, the most important is that the
user is informed of what's happening and how to deal with it.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Paul E. McKenney]

On Thu, Apr 11, 2013 at 07:28:05PM +0200, Frederic Weisbecker wrote:
> On Thu, Apr 11, 2013 at 10:11:53AM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 11, 2013 at 07:04:23PM +0200, Frederic Weisbecker wrote:
> > > On Thu, Apr 11, 2013 at 04:41:07PM +0000, Christoph Lameter wrote:
> > > > On Thu, 11 Apr 2013, Frederic Weisbecker wrote:
> > > > 
> > > > > If there is no performance issue with that I'm all for it.
> > > > 
> > > > Or have a
> > > > 
> > > > CONFIG_LOWLATENCY
> > > > 
> > > > that boots up a kernel with the proper configuration?
> > > 
> > > It may be too general for a naming. But I don't mind just
> > > selecting CONFIG_RCU_NOCBS_ALL unconditionally. It's easily
> > > changed in the future if anybody complains.
> > > 
> > > Btw, if CONFIG_RCU_NOCBS_ALL is set, the rcu_nocbs= parameter
> > > is ignored, right? If you want to keep that direction and not
> > > override the Kconfig choice, may be warn the user about that
> > > if the boot parameter is passed?
> > 
> > Fair point.  Let me think about how I should approach Thanx.
> 
> An alternative is to treat the Kconfig choice as a default setting
> that gets overriden by rcu_nocbs=
> 
> This would require to refactor the three way Kconfig layout you've set,
> but that aligns with what Ingo is suggesting me to do with the full dynticks
> range: having CONFIG_NO_HZ_EXTENDED_ALL that is overriden by nohz_extended=
> if any (beware though, "extended" will soon be renamed to "full", lets hope
> it's our last take ;)

So the idea is that if CONFIG_NO_HZ_EXTENDED_ALL is specified, the list
passed via nohz_extended= is ANDed rather than ORed?  Interesting...

> That unifies both behaviours and it looks more flexible to me.
> 
> Of course that means selecting CONFIG_RCU_NOCBS_ALL won't be enough for
> me to ensure my nohz range is also nocb, I'm fine with the cpumask check
> on boot though.
> 
> Anyway, that's just a suggestion, the most important is that the
> user is informed of what's happening and how to deal with it.

I will hold off a bit and see how things settle out.

							Thanx, Paul

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Christoph Lameter]

On Thu, 11 Apr 2013, Frederic Weisbecker wrote:

> It may be too general for a naming. But I don't mind just
> selecting CONFIG_RCU_NOCBS_ALL unconditionally. It's easily
> changed in the future if anybody complains.


I like the general nature of that config option since it removes the need
to configure all the details. For an average user the current sets of
options must look pretty complicated.

> > Btw, if CONFIG_RCU_NOCBS_ALL isset, the rcu_nocbs=
parameter > is ignored, right? If you want to keep that direction and not
> override the Kconfig choice, may be warn the user about that
> if the boot parameter is passed?

Ok. But all these complicated things would go away if we had an option
CONFIG_LOWLATENCY and then everything would just follow the best setup
possible given the hardware. Would remove a lot of guesswork and a lot of
knobs.

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Ingo Molnar]

* Christoph Lameter <cl@linux.com> wrote:

> On Thu, 11 Apr 2013, Frederic Weisbecker wrote:
> 
> > It may be too general for a naming. But I don't mind just selecting 
> > CONFIG_RCU_NOCBS_ALL unconditionally. It's easily changed in the future if 
> > anybody complains.
> 
> 
> I like the general nature of that config option since it removes the need to 
> configure all the details. For an average user the current sets of options must 
> look pretty complicated.

Yes.

It's not just complicated but also fragile and time consuming: as new kernel 
options arrive you'd always have to be very careful with 'make oldconfig' and make 
sure you pick up the best options for latency.

Instead what we want is generally a high level knob that documents user preference 
and then the kernel config language can do the rest.

> > > Btw, if CONFIG_RCU_NOCBS_ALL isset, the rcu_nocbs= parameter is ignored, 
> > > right? If you want to keep that direction and not override the Kconfig 
> > > choice, may be warn the user about that if the boot parameter is passed?
> 
> Ok. But all these complicated things would go away if we had an option
> CONFIG_LOWLATENCY and then everything would just follow the best setup
> possible given the hardware. Would remove a lot of guesswork and a lot of
> knobs.

In that sense CONFIG_NO_HZ_FULL is such a flag as well, which, like 
CONFIG_PREEMPT_RT, tries to preconfigure the kernel correctly.

But we have to be careful not to use a too highlevel flag for that. If the user 
meant 'low latency' to mean 'low latency IRQ execution' - then enabling 
CONFIG_NO_HZ_FULL might achieve the opposite, it adds overhead to the IRQ paths.

Thanks,

	Ingo

Re: [PATCH 4/4] nohz: New option to force all CPUs in full dynticks range

[Christoph Lameter]

On Mon, 15 Apr 2013, Ingo Molnar wrote:

> > Ok. But all these complicated things would go away if we had an option
> > CONFIG_LOWLATENCY and then everything would just follow the best setup
> > possible given the hardware. Would remove a lot of guesswork and a lot of
> > knobs.
>
> In that sense CONFIG_NO_HZ_FULL is such a flag as well, which, like
> CONFIG_PREEMPT_RT, tries to preconfigure the kernel correctly.

It is too specific. Switching off HZ is one in a set of measures that one
woiuld take. F.e. the page allocator will want to do reclaim on the
sacrificial processor, the vmstatistics would run their monitoring thread
there etc etc.

> But we have to be careful not to use a too highlevel flag for that. If the user
> meant 'low latency' to mean 'low latency IRQ execution' - then enabling
> CONFIG_NO_HZ_FULL might achieve the opposite, it adds overhead to the IRQ paths.

Hmm.... Looks too specific for my taste. What the user wants is less OS
noise (or maybe some other terms that describes that the user wants
minimal disturbances for his code).

	CONFIG_OS_NOISE_REDUCTION

?