Re: [PATCH v2] x86: Reduce clock calibration time during slave cpu startup

[Yinghai Lu <yhlu.kernel@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

On Fri, Aug 5, 2011 at 6:16 AM, Jack Steiner <steiner@sgi.com> wrote:
> Aside from the bogus comment (I'll send a V3 with a fixed comment), does the patch
> look ok.

Several months ago, Robin said that he will test updated version

[PATCH] x86: Make calibrate_delay run in parallel.

so any reason that you sgi guyes stop that path?

Please check attached patch for updated version to current tip.

Thanks

Yinghai Lu

[-- Attachment #2: Make-x86-calibrate_delay-run-in-parallel_v4.patch --]
[-- Type: text/x-patch, Size: 9170 bytes --]

[PATCH -v4] x86: Make calibrate_delay run in parallel.

On a 4096 cpu machine, we noticed that 318 seconds were taken for bringing
up the cpus.  By specifying lpj=<value>, we reduced that to 75 seconds.
Andi Kleen suggested we rework the calibrate_delay calls to run in
parallel.

-v2: from Yinghai
     two path: one for initial boot cpus. and one for hotplug cpus
	initial path:
	  after all cpu boot up, enter idle, use smp_call_function_many
	  let every ap call __calibrate_delay.
          We can not put that calibrate_delay after local_irq_enable
          in start_secondary(), at that time that cpu could be involed
	  with perf_event with nmi_watchdog enabling. that will cause
	  strange calibrating result.
        hotplug path:
	  need to add cpu notify block.
     add __calibrate_delay instead of changing calibrate_delay all over.
     use cpu_calibrated_delay_mask instead.
     use print_lpj to make print line complete.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Andi Kleen <andi@firstfloor.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/cpumask.h |    1 
 arch/x86/kernel/cpu/common.c   |    3 ++
 arch/x86/kernel/smpboot.c      |   58 ++++++++++++++++++++++++++++++++++-------
 include/linux/delay.h          |    1 
 init/calibrate.c               |   54 +++++++++++++++++++-------------------
 5 files changed, 81 insertions(+), 36 deletions(-)


--
Index: linux-2.6/arch/x86/include/asm/cpumask.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/cpumask.h
+++ linux-2.6/arch/x86/include/asm/cpumask.h
@@ -6,6 +6,7 @@
 extern cpumask_var_t cpu_callin_mask;
 extern cpumask_var_t cpu_callout_mask;
 extern cpumask_var_t cpu_initialized_mask;
+extern cpumask_var_t cpu_calibrated_delay_mask;
 extern cpumask_var_t cpu_sibling_setup_mask;
 
 extern void setup_cpu_local_masks(void);
Index: linux-2.6/arch/x86/kernel/cpu/common.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/common.c
+++ linux-2.6/arch/x86/kernel/cpu/common.c
@@ -45,6 +45,7 @@
 cpumask_var_t cpu_initialized_mask;
 cpumask_var_t cpu_callout_mask;
 cpumask_var_t cpu_callin_mask;
+cpumask_var_t cpu_calibrated_delay_mask;
 
 /* representing cpus for which sibling maps can be computed */
 cpumask_var_t cpu_sibling_setup_mask;
@@ -58,6 +59,8 @@ void __init setup_cpu_local_masks(void)
 	alloc_bootmem_cpumask_var(&cpu_callin_mask);
 	set_bootmem_name("cpu_callout_mask");
 	alloc_bootmem_cpumask_var(&cpu_callout_mask);
+	set_bootmem_name("cpu_calibrated_delay_mask");
+	alloc_bootmem_cpumask_var(&cpu_calibrated_delay_mask);
 	set_bootmem_name("cpu_sibling_setup_mask");
 	alloc_bootmem_cpumask_var(&cpu_sibling_setup_mask);
 }
Index: linux-2.6/arch/x86/kernel/smpboot.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/smpboot.c
+++ linux-2.6/arch/x86/kernel/smpboot.c
@@ -52,6 +52,7 @@
 #include <linux/gfp.h>
 
 #include <asm/acpi.h>
+#include <asm/cpumask.h>
 #include <asm/desc.h>
 #include <asm/nmi.h>
 #include <asm/irq.h>
@@ -210,15 +211,7 @@ static void __cpuinit smp_callin(void)
 	 * Need to setup vector mappings before we enable interrupts.
 	 */
 	setup_vector_irq(smp_processor_id());
-	/*
-	 * Get our bogomips.
-	 *
-	 * Need to enable IRQs because it can take longer and then
-	 * the NMI watchdog might kill us.
-	 */
-	local_irq_enable();
-	calibrate_delay();
-	local_irq_disable();
+
 	pr_debug("Stack at about %p\n", &cpuid);
 
 	/*
@@ -1038,6 +1031,8 @@ void __init native_smp_prepare_cpus(unsi
 	}
 	set_cpu_sibling_map(0);
 
+	/* already called earlier for boot cpu */
+	cpumask_set_cpu(0, cpu_calibrated_delay_mask);
 
 	if (smp_sanity_check(max_cpus) < 0) {
 		printk(KERN_INFO "SMP disabled\n");
@@ -1126,8 +1121,53 @@ void __init native_smp_prepare_boot_cpu(
 	per_cpu(cpu_state, me) = CPU_ONLINE;
 }
 
+static void __cpuinit calibrate_delay_fn(void *info)
+{
+	int cpu = smp_processor_id();
+
+	cpu_data(cpu).loops_per_jiffy = __calibrate_delay(cpu);
+	cpumask_set_cpu(cpu, cpu_calibrated_delay_mask);
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+static int __cpuinit
+cal_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+	int cpu = (unsigned long)hcpu;
+
+	switch (action) {
+	case CPU_ONLINE:
+	case CPU_ONLINE_FROZEN:
+		smp_call_function_single(cpu, calibrate_delay_fn, NULL, 1);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static __cpuinitdata struct notifier_block __cpuinitdata cal_cpu_nfb = {
+	.notifier_call = cal_cpu_callback
+};
+
+static void __init register_cal_cpu_nfb(void)
+{
+	register_cpu_notifier(&cal_cpu_nfb);
+}
+#else
+static void __init register_cal_cpu_nfb(void)
+{
+}
+#endif
+
 void __init native_smp_cpus_done(unsigned int max_cpus)
 {
+	smp_call_function_many(cpu_online_mask, calibrate_delay_fn, NULL, 0);
+	while (cpumask_weight(cpu_calibrated_delay_mask) != num_online_cpus()) {
+		cpu_relax();
+		touch_nmi_watchdog();
+	}
+	register_cal_cpu_nfb();
+
 	pr_debug("Boot done.\n");
 
 	impress_friends();
Index: linux-2.6/include/linux/delay.h
===================================================================
--- linux-2.6.orig/include/linux/delay.h
+++ linux-2.6/include/linux/delay.h
@@ -43,6 +43,7 @@ static inline void ndelay(unsigned long
 
 extern unsigned long lpj_fine;
 void calibrate_delay(void);
+unsigned long __calibrate_delay(int cpu);
 void msleep(unsigned int msecs);
 unsigned long msleep_interruptible(unsigned int msecs);
 void usleep_range(unsigned long min, unsigned long max);
Index: linux-2.6/init/calibrate.c
===================================================================
--- linux-2.6.orig/init/calibrate.c
+++ linux-2.6/init/calibrate.c
@@ -31,7 +31,7 @@ __setup("lpj=", lpj_setup);
 #define DELAY_CALIBRATION_TICKS			((HZ < 100) ? 1 : (HZ/100))
 #define MAX_DIRECT_CALIBRATION_RETRIES		5
 
-static unsigned long __cpuinit calibrate_delay_direct(void)
+static unsigned long __cpuinit calibrate_delay_direct(int cpu)
 {
 	unsigned long pre_start, start, post_start;
 	unsigned long pre_end, end, post_end;
@@ -134,15 +134,9 @@ static unsigned long __cpuinit calibrate
 		good_timer_count = 0;
 		if ((measured_times[max] - estimate) <
 				(estimate - measured_times[min])) {
-			printk(KERN_NOTICE "calibrate_delay_direct() dropping "
-					"min bogoMips estimate %d = %lu\n",
-				min, measured_times[min]);
 			measured_times[min] = 0;
 			min = max;
 		} else {
-			printk(KERN_NOTICE "calibrate_delay_direct() dropping "
-					"max bogoMips estimate %d = %lu\n",
-				max, measured_times[max]);
 			measured_times[max] = 0;
 			max = min;
 		}
@@ -160,9 +154,9 @@ static unsigned long __cpuinit calibrate
 
 	}
 
-	printk(KERN_NOTICE "calibrate_delay_direct() failed to get a good "
+	printk(KERN_NOTICE "CPU%d: calibrate_delay_direct() failed to get a good "
 	       "estimate for loops_per_jiffy.\nProbably due to long platform "
-		"interrupts. Consider using \"lpj=\" boot option.\n");
+		"interrupts. Consider using \"lpj=\" boot option.\n", cpu);
 	return 0;
 }
 #else
@@ -246,32 +240,38 @@ recalibrate:
 
 static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
 
-void __cpuinit calibrate_delay(void)
+static void __cpuinit print_lpj(int cpu, char *str, unsigned long lpj)
+{
+       pr_info("CPU%d: Calibrating delay%s"
+               "%lu.%02lu BogoMIPS (lpj=%lu)\n", cpu, str,
+               lpj/(500000/HZ), (lpj/(5000/HZ)) % 100, lpj);
+}
+
+unsigned long __cpuinit __calibrate_delay(int cpu)
 {
 	unsigned long lpj;
-	int this_cpu = smp_processor_id();
 
-	if (per_cpu(cpu_loops_per_jiffy, this_cpu)) {
-		lpj = per_cpu(cpu_loops_per_jiffy, this_cpu);
-		pr_info("Calibrating delay loop (skipped) "
-				"already calibrated this CPU");
+	if (per_cpu(cpu_loops_per_jiffy, cpu)) {
+		lpj = per_cpu(cpu_loops_per_jiffy, cpu);
+		print_lpj(cpu, " (skipped) already calibrated this CPU ", lpj);
 	} else if (preset_lpj) {
 		lpj = preset_lpj;
-		pr_info("Calibrating delay loop (skipped) preset value.. ");
-	} else if ((this_cpu == 0) && lpj_fine) {
+		print_lpj(cpu, " (skipped) preset value.. ", lpj);
+	} else if ((cpu == 0) && lpj_fine) {
 		lpj = lpj_fine;
-		pr_info("Calibrating delay loop (skipped), "
-			"value calculated using timer frequency.. ");
-	} else if ((lpj = calibrate_delay_direct()) != 0) {
-		pr_info("Calibrating delay using timer specific routine.. ");
+		print_lpj(cpu, " loop (skipped), value calculated using timer frequency.. ", lpj);
+	} else if ((lpj = calibrate_delay_direct(cpu)) != 0) {
+		print_lpj(cpu, " using timer specific routine.. ", lpj);
 	} else {
-		pr_info("Calibrating delay loop... ");
 		lpj = calibrate_delay_converge();
+		print_lpj(cpu, " delay loop... ", lpj);
 	}
-	per_cpu(cpu_loops_per_jiffy, this_cpu) = lpj;
-	pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
-			lpj/(500000/HZ),
-			(lpj/(5000/HZ)) % 100, lpj);
+	per_cpu(cpu_loops_per_jiffy, cpu) = lpj;
 
-	loops_per_jiffy = lpj;
+	return lpj;
+}
+
+void __cpuinit calibrate_delay(void)
+{
+	loops_per_jiffy = __calibrate_delay(smp_processor_id());
 }