[PATCH 0/6] Limit console output by suppressing repetitious messages

[PATCH 0/6] Limit console output by suppressing repetitious messages

[Mike Travis]

Third version...

- Remove Processor Summary at end of bootup sequence and added '#'
  to CPU numbers.  This makes it a bit easier to pick up the context if
  an error message disrupts the single output line of cpus.

- Remove cacheline size printout changes (dealt with in other patches)

- Modify MCE handler to send msgs to kernel debug log buffer and to not
  output an extra '\n' per cpu.

- ACPI messages moved to separate thread since they need to be submitted
  via the acpi group.

-- 

[PATCH 1/6] x86: Limit the number of processor bootup messages

[Mike Travis]

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

When there are a large number of processors in a system, there
is an excessive amount of messages sent to the system console.
It's estimated that with 4096 processors in a system, and the
console baudrate set to 56K, the startup messages will take
about 84 minutes to clear the serial port.

This set of patches limits the number of repetitious messages
which contain no additional information.  Much of this information
is obtainable from the /proc and /sysfs.   Some of the messages
are also sent to the kernel log buffer as KERN_DEBUG messages so
dmesg can be used to examine more closely any details specific to
a problem.

The list of message transformations....

For system_state == SYSTEM_BOOTING:

Booting Node   0, Processors  #1 #2 #3 #4 #5 #6 #7 Ok.
Booting Node   1, Processors  #8 #9 #10 #11 #12 #13 #14 #15 Ok.
..
Booting Node   3, Processors  #56 #57 #58 #59 #60 #61 #62 #63 Ok.
Brought up 64 CPUs

The following lines have been removed:

	CPU: Physical Processor ID:
	CPU: Processor Core ID:
	CPU %d/0x%x -> Node %d

The following lines will only be printed if unusual (state):

	CPU %d is now offline  (system_state == RUNNING)

The following lines will only be printed in debug mode:

	Initializing CPU#%d

The following lines are only printed for the first (boot) cpu:

	CPU0: Hyper-Threading is disabled
	CPU0: Thermal monitoring enabled

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/cpu/addon_cpuid_features.c |    6 ----
 arch/x86/kernel/cpu/amd.c                  |    2 -
 arch/x86/kernel/cpu/common.c               |   20 ++++-----------
 arch/x86/kernel/cpu/intel.c                |    2 -
 arch/x86/kernel/cpu/mcheck/therm_throt.c   |    5 ++-
 arch/x86/kernel/smpboot.c                  |   37 ++++++++++++++++++-----------
 6 files changed, 33 insertions(+), 39 deletions(-)

--- linux.orig/arch/x86/kernel/cpu/addon_cpuid_features.c
+++ linux/arch/x86/kernel/cpu/addon_cpuid_features.c
@@ -127,12 +127,6 @@
 
 	c->x86_max_cores = (core_level_siblings / smp_num_siblings);
 
-
-	printk(KERN_INFO  "CPU: Physical Processor ID: %d\n",
-	       c->phys_proc_id);
-	if (c->x86_max_cores > 1)
-		printk(KERN_INFO  "CPU: Processor Core ID: %d\n",
-		       c->cpu_core_id);
 	return;
 #endif
 }
--- linux.orig/arch/x86/kernel/cpu/amd.c
+++ linux/arch/x86/kernel/cpu/amd.c
@@ -375,8 +375,6 @@
 			node = nearby_node(apicid);
 	}
 	numa_set_node(cpu, node);
-
-	printk(KERN_INFO "CPU %d/0x%x -> Node %d\n", cpu, apicid, node);
 #endif
 }
 
--- linux.orig/arch/x86/kernel/cpu/common.c
+++ linux/arch/x86/kernel/cpu/common.c
@@ -437,7 +437,7 @@
 		return;
 
 	if (cpu_has(c, X86_FEATURE_CMP_LEGACY))
-		goto out;
+		return;
 
 	if (cpu_has(c, X86_FEATURE_XTOPOLOGY))
 		return;
@@ -446,13 +446,13 @@
 
 	smp_num_siblings = (ebx & 0xff0000) >> 16;
 
-	if (smp_num_siblings == 1) {
-		printk(KERN_INFO  "CPU: Hyper-Threading is disabled\n");
-		goto out;
+	if (smp_num_siblings == 1 && c->cpu_index == 0) {
+		pr_info("CPU0: Hyper-Threading is disabled\n");
+		return;
 	}
 
 	if (smp_num_siblings <= 1)
-		goto out;
+		return;
 
 	if (smp_num_siblings > nr_cpu_ids) {
 		pr_warning("CPU: Unsupported number of siblings %d",
@@ -472,14 +472,6 @@
 
 	c->cpu_core_id = apic->phys_pkg_id(c->initial_apicid, index_msb) &
 				       ((1 << core_bits) - 1);
-
-out:
-	if ((c->x86_max_cores * smp_num_siblings) > 1) {
-		printk(KERN_INFO  "CPU: Physical Processor ID: %d\n",
-		       c->phys_proc_id);
-		printk(KERN_INFO  "CPU: Processor Core ID: %d\n",
-		       c->cpu_core_id);
-	}
 #endif
 }
 
@@ -1115,7 +1107,7 @@
 	if (cpumask_test_and_set_cpu(cpu, cpu_initialized_mask))
 		panic("CPU#%d already initialized!\n", cpu);
 
-	printk(KERN_INFO "Initializing CPU#%d\n", cpu);
+	pr_debug("Initializing CPU#%d\n", cpu);
 
 	clear_in_cr4(X86_CR4_VME|X86_CR4_PVI|X86_CR4_TSD|X86_CR4_DE);
 
--- linux.orig/arch/x86/kernel/cpu/intel.c
+++ linux/arch/x86/kernel/cpu/intel.c
@@ -266,8 +266,6 @@
 	if (node == NUMA_NO_NODE || !node_online(node))
 		node = first_node(node_online_map);
 	numa_set_node(cpu, node);
-
-	printk(KERN_INFO "CPU %d/0x%x -> Node %d\n", cpu, apicid, node);
 #endif
 }
 
--- linux.orig/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ linux/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -312,8 +312,9 @@
 	l = apic_read(APIC_LVTTHMR);
 	apic_write(APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
 
-	printk(KERN_INFO "CPU%d: Thermal monitoring enabled (%s)\n",
-	       cpu, tm2 ? "TM2" : "TM1");
+	if (cpu == 0)
+		printk(KERN_INFO "CPU0: Thermal monitoring enabled (%s)\n",
+		       tm2 ? "TM2" : "TM1");
 
 	/* enable thermal throttle processing */
 	atomic_set(&therm_throt_en, 1);
--- linux.orig/arch/x86/kernel/smpboot.c
+++ linux/arch/x86/kernel/smpboot.c
@@ -737,8 +737,21 @@
 	start_ip = setup_trampoline();
 
 	/* So we see what's up   */
-	printk(KERN_INFO "Booting processor %d APIC 0x%x ip 0x%lx\n",
-			  cpu, apicid, start_ip);
+#ifdef CONFIG_NUMA
+	if (system_state == SYSTEM_BOOTING) {
+		static int current_node = -1;
+		int node = cpu_to_node(cpu);
+
+		if (node != current_node) {
+			if (current_node > (-1))
+				pr_cont(" Ok.\n");
+			current_node = node;
+			pr_info("Booting Node %3d, Processors ", node);
+		}
+		pr_cont(" #%d%s", cpu, cpu == (nr_cpu_ids - 1) ? " Ok.\n" : "");
+	} else
+#endif
+		pr_info("Booting Processor %d APIC 0x%x\n", cpu, apicid);
 
 	/*
 	 * This grunge runs the startup process for
@@ -787,21 +800,17 @@
 			udelay(100);
 		}
 
-		if (cpumask_test_cpu(cpu, cpu_callin_mask)) {
-			/* number CPUs logically, starting from 1 (BSP is 0) */
-			pr_debug("OK.\n");
-			printk(KERN_INFO "CPU%d: ", cpu);
-			print_cpu_info(&cpu_data(cpu));
-			pr_debug("CPU has booted.\n");
-		} else {
+		if (cpumask_test_cpu(cpu, cpu_callin_mask))
+			pr_debug("CPU%d: has booted.\n", cpu);
+		else {
 			boot_error = 1;
 			if (*((volatile unsigned char *)trampoline_base)
 					== 0xA5)
 				/* trampoline started but...? */
-				printk(KERN_ERR "Stuck ??\n");
+				pr_err("CPU%d: Stuck ??\n", cpu);
 			else
 				/* trampoline code not run */
-				printk(KERN_ERR "Not responding.\n");
+				pr_err("CPU%d: Not responding.\n", cpu);
 			if (apic->inquire_remote_apic)
 				apic->inquire_remote_apic(apicid);
 		}
@@ -1300,14 +1309,16 @@
 	for (i = 0; i < 10; i++) {
 		/* They ack this in play_dead by setting CPU_DEAD */
 		if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
-			printk(KERN_INFO "CPU %d is now offline\n", cpu);
+			if (system_state == SYSTEM_RUNNING)
+				pr_info("CPU %u is now offline\n", cpu);
+
 			if (1 == num_online_cpus())
 				alternatives_smp_switch(0);
 			return;
 		}
 		msleep(100);
 	}
-	printk(KERN_ERR "CPU %u didn't die...\n", cpu);
+	pr_err("CPU %u didn't die...\n", cpu);
 }
 
 void play_dead_common(void)

-- 

[PATCH 2/6] x86: Limit the number of per cpu MCE bootup messages

[Mike Travis]

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

Limit the number of per cpu MCE messages by using pr_debug.
This prevents filling up the console output with repetitious
messages when the number of cpus is large.

Remove the need for KERN_CONT so it does not add an extraneous
newline in the booting cpu sequence.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c       |    4 ++--
 arch/x86/kernel/cpu/mcheck/mce_intel.c |   20 ++++++++++++--------
 2 files changed, 14 insertions(+), 10 deletions(-)

--- linux.orig/arch/x86/kernel/cpu/mcheck/mce.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1215,10 +1215,10 @@
 
 	b = cap & MCG_BANKCNT_MASK;
 	if (!banks)
-		printk(KERN_INFO "mce: CPU supports %d MCE banks\n", b);
+		pr_debug("mce: CPU supports %d MCE banks\n", b);
 
 	if (b > MAX_NR_BANKS) {
-		printk(KERN_WARNING
+		pr_warning(
 		       "MCE: Using only %u machine check banks out of %u\n",
 			MAX_NR_BANKS, b);
 		b = MAX_NR_BANKS;
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -64,12 +64,15 @@
 	mce_notify_irq();
 }
 
-static void print_update(char *type, int *hdr, int num)
+static void print_update(char *type, int *hdr, int num, char *buf, int len)
 {
-	if (*hdr == 0)
-		printk(KERN_INFO "CPU %d MCA banks", smp_processor_id());
-	*hdr = 1;
-	printk(KERN_CONT " %s:%d", type, num);
+	int n = *hdr;
+
+	if (n == 0)
+		n = snprintf(buf, len, "CPU %d MCA banks", smp_processor_id());
+
+	n += snprintf(&buf[n], len - n, " %s:%d", type, num);
+	*hdr = n;
 }
 
 /*
@@ -83,6 +86,7 @@
 	unsigned long flags;
 	int hdr = 0;
 	int i;
+	char buf[120];
 
 	spin_lock_irqsave(&cmci_discover_lock, flags);
 	for (i = 0; i < banks; i++) {
@@ -96,7 +100,7 @@
 		/* Already owned by someone else? */
 		if (val & CMCI_EN) {
 			if (test_and_clear_bit(i, owned) || boot)
-				print_update("SHD", &hdr, i);
+				print_update("SHD", &hdr, i, buf, sizeof(buf));
 			__clear_bit(i, __get_cpu_var(mce_poll_banks));
 			continue;
 		}
@@ -108,7 +112,7 @@
 		/* Did the enable bit stick? -- the bank supports CMCI */
 		if (val & CMCI_EN) {
 			if (!test_and_set_bit(i, owned) || boot)
-				print_update("CMCI", &hdr, i);
+				print_update("CMCI", &hdr, i, buf, sizeof(buf));
 			__clear_bit(i, __get_cpu_var(mce_poll_banks));
 		} else {
 			WARN_ON(!test_bit(i, __get_cpu_var(mce_poll_banks)));
@@ -116,7 +120,7 @@
 	}
 	spin_unlock_irqrestore(&cmci_discover_lock, flags);
 	if (hdr)
-		printk(KERN_CONT "\n");
+		pr_debug("%s\n", buf);
 }
 
 /*

-- 

[PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Mike Travis]

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

Limit the number of per cpu calibration messages by only printing
out results for the boot cpu.

Don't print "CPUx is down" as this is expected, and we don't need
4096 reminders... ;-)

Signed-off-by: Mike Travis <travis@sgi.com>
---
 init/calibrate.c |   22 +++++++++++++---------
 kernel/cpu.c     |    5 ++---
 2 files changed, 15 insertions(+), 12 deletions(-)

--- linux.orig/init/calibrate.c
+++ linux/init/calibrate.c
@@ -123,23 +123,26 @@
 {
 	unsigned long ticks, loopbit;
 	int lps_precision = LPS_PREC;
+	bool boot_cpu = (smp_processor_id() == 0);
 
 	if (preset_lpj) {
 		loops_per_jiffy = preset_lpj;
-		printk(KERN_INFO
-			"Calibrating delay loop (skipped) preset value.. ");
-	} else if ((smp_processor_id() == 0) && lpj_fine) {
+		if (boot_cpu)
+			pr_info("Calibrating delay loop (skipped) "
+				"preset value.. ");
+	} else if ((boot_cpu) && lpj_fine) {
 		loops_per_jiffy = lpj_fine;
-		printk(KERN_INFO
-			"Calibrating delay loop (skipped), "
+		pr_info("Calibrating delay loop (skipped), "
 			"value calculated using timer frequency.. ");
 	} else if ((loops_per_jiffy = calibrate_delay_direct()) != 0) {
-		printk(KERN_INFO
-			"Calibrating delay using timer specific routine.. ");
+		if (boot_cpu)
+			pr_info("Calibrating delay using timer "
+				"specific routine.. ");
 	} else {
 		loops_per_jiffy = (1<<12);
 
-		printk(KERN_INFO "Calibrating delay loop... ");
+		if (boot_cpu)
+			pr_info("Calibrating delay loop... ");
 		while ((loops_per_jiffy <<= 1) != 0) {
 			/* wait for "start of" clock tick */
 			ticks = jiffies;
@@ -170,7 +173,8 @@
 				loops_per_jiffy &= ~loopbit;
 		}
 	}
-	printk(KERN_CONT "%lu.%02lu BogoMIPS (lpj=%lu)\n",
+	if (boot_cpu)
+		pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n",
 			loops_per_jiffy/(500000/HZ),
 			(loops_per_jiffy/(5000/HZ)) % 100, loops_per_jiffy);
 }
--- linux.orig/kernel/cpu.c
+++ linux/kernel/cpu.c
@@ -392,10 +392,9 @@
 		if (cpu == first_cpu)
 			continue;
 		error = _cpu_down(cpu, 1);
-		if (!error) {
+		if (!error)
 			cpumask_set_cpu(cpu, frozen_cpus);
-			printk("CPU%d is down\n", cpu);
-		} else {
+		else {
 			printk(KERN_ERR "Error taking CPU%d down: %d\n",
 				cpu, error);
 			break;

-- 

[PATCH 4/6] firmware: Limit the number of per cpu firmware messages during bootup

[Mike Travis]

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

Limit the number of per cpu firmware: messages by sending them
only to the kernel debug log buffer.

	[  170.643130] firmware: requesting intel-ucode/06-2e-0

Signed-off-by: Mike Travis <travis@sgi.com>
---
 drivers/base/firmware_class.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux.orig/drivers/base/firmware_class.c
+++ linux/drivers/base/firmware_class.c
@@ -495,7 +495,7 @@
 	}
 
 	if (uevent)
-		dev_info(device, "firmware: requesting %s\n", name);
+		dev_dbg(device, "firmware: requesting %s\n", name);
 
 	retval = fw_setup_device(firmware, &f_dev, name, device, uevent);
 	if (retval)

-- 

[PATCH 5/6] sched: Limit the number of scheduler debug messages

[Mike Travis]

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

Remove the verbose scheduler debug messages unless kernel parameter
"sched_debug" set.  /proc/sched_debug unchanged.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 Documentation/kernel-parameters.txt |    2 ++
 kernel/sched.c                      |   13 +++++++++++++
 2 files changed, 15 insertions(+)

--- linux.orig/Documentation/kernel-parameters.txt
+++ linux/Documentation/kernel-parameters.txt
@@ -2182,6 +2182,8 @@
 
 	sbni=		[NET] Granch SBNI12 leased line adapter
 
+	sched_debug	[KNL] Enables verbose scheduler debug messages.
+
 	sc1200wdt=	[HW,WDT] SC1200 WDT (watchdog) driver
 			Format: <io>[,<timeout>[,<isapnp>]]
 
--- linux.orig/kernel/sched.c
+++ linux/kernel/sched.c
@@ -7743,6 +7743,16 @@
 
 #ifdef CONFIG_SCHED_DEBUG
 
+static __read_mostly int sched_domain_debug_enabled;
+
+static int __init sched_domain_debug_setup(char *str)
+{
+	sched_domain_debug_enabled = 1;
+
+	return 0;
+}
+early_param("sched_debug", sched_domain_debug_setup);
+
 static int sched_domain_debug_one(struct sched_domain *sd, int cpu, int level,
 				  struct cpumask *groupmask)
 {
@@ -7829,6 +7839,9 @@
 	cpumask_var_t groupmask;
 	int level = 0;
 
+	if (!sched_domain_debug_enabled)
+		return;
+
 	if (!sd) {
 		printk(KERN_DEBUG "CPU%d attaching NULL sched-domain.\n", cpu);
 		return;

-- 

[PATCH 6/6] x86: Limit number of per cpu TSC sync messages

[Mike Travis]

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

Limit the number of per cpu TSC sync messages by only printing
to the console if an error occurs, otherwise print as a DEBUG
message.

The info message "Skipping synchronization ..." is only printed
after the last cpu has booted.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/tsc_sync.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

--- linux.orig/arch/x86/kernel/tsc_sync.c
+++ linux/arch/x86/kernel/tsc_sync.c
@@ -114,13 +114,12 @@
 		return;
 
 	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE)) {
-		printk_once(KERN_INFO "Skipping synchronization checks as TSC is reliable.\n");
+		if (cpu == (nr_cpu_ids-1) || system_state != SYSTEM_BOOTING)
+			pr_info(
+			"Skipped synchronization checks as TSC is reliable.\n");
 		return;
 	}
 
-	pr_info("checking TSC synchronization [CPU#%d -> CPU#%d]:",
-		smp_processor_id(), cpu);
-
 	/*
 	 * Reset it - in case this is a second bootup:
 	 */
@@ -142,12 +141,14 @@
 		cpu_relax();
 
 	if (nr_warps) {
-		printk("\n");
+		pr_warning("TSC synchronization [CPU#%d -> CPU#%d]:\n",
+			smp_processor_id(), cpu);
 		pr_warning("Measured %Ld cycles TSC warp between CPUs, "
 			   "turning off TSC clock.\n", max_warp);
 		mark_tsc_unstable("check_tsc_sync_source failed");
 	} else {
-		printk(" passed.\n");
+		pr_debug("TSC synchronization [CPU#%d -> CPU#%d]: passed\n",
+			smp_processor_id(), cpu);
 	}
 
 	/*

-- 

Re: [PATCH 1/6] x86: Limit the number of processor bootup messages

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> +	if (cpu == 0)
> +		printk(KERN_INFO "CPU0: Thermal monitoring enabled (%s)\n",
> +		       tm2 ? "TM2" : "TM1");

Hm, 'cpu==0 means boot cpu' assumptions are not particularly clean.

> +#ifdef CONFIG_NUMA
> +	if (system_state == SYSTEM_BOOTING) {
> +		static int current_node = -1;
> +		int node = cpu_to_node(cpu);
> +
> +		if (node != current_node) {
> +			if (current_node > (-1))
> +				pr_cont(" Ok.\n");
> +			current_node = node;
> +			pr_info("Booting Node %3d, Processors ", node);
> +		}
> +		pr_cont(" #%d%s", cpu, cpu == (nr_cpu_ids - 1) ? " Ok.\n" : "");
> +	} else
> +#endif
> +		pr_info("Booting Processor %d APIC 0x%x\n", cpu, apicid);

preprocessor directives cutting into if/else branches in an assymetric 
way is being frowned upon. I'd also suggest to put this into a helper 
inline.

Is the SYSTEM_BOOTING check there to not re-print this on CPU hotplug?

	Ingo

Re: [PATCH 2/6] x86: Limit the number of per cpu MCE bootup messages

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> @@ -83,6 +86,7 @@
>  	unsigned long flags;
>  	int hdr = 0;
>  	int i;
> +	char buf[120];

that constant is not particularly nice, is it?

	Ingo

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> --- linux.orig/init/calibrate.c
> +++ linux/init/calibrate.c
> @@ -123,23 +123,26 @@
>  {
>  	unsigned long ticks, loopbit;
>  	int lps_precision = LPS_PREC;
> +	bool boot_cpu = (smp_processor_id() == 0);

this code is shared by other architectures too - are you sure 
smp_processor_id()==0 is a proper 'I am the boot CPU' assumption 
everywhere?

	Ingo

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[H. Peter Anvin]

On 11/16/2009 01:24 PM, Ingo Molnar wrote:
> 
> * Mike Travis <travis@sgi.com> wrote:
> 
>> --- linux.orig/init/calibrate.c
>> +++ linux/init/calibrate.c
>> @@ -123,23 +123,26 @@
>>  {
>>  	unsigned long ticks, loopbit;
>>  	int lps_precision = LPS_PREC;
>> +	bool boot_cpu = (smp_processor_id() == 0);
> 
> this code is shared by other architectures too - are you sure 
> smp_processor_id()==0 is a proper 'I am the boot CPU' assumption 
> everywhere?
> 

It really shouldn't be for x86 either, although right now we play nasty
renumbering games to accommodate that assumption.  It seems like we
really should have a boot_cpu_id() or some such.

	-hpa

Re: [PATCH 1/6] x86: Limit the number of processor bootup messages

[Mike Travis]

Ingo Molnar wrote:
> * Mike Travis <travis@sgi.com> wrote:
> 
>> +	if (cpu == 0)
>> +		printk(KERN_INFO "CPU0: Thermal monitoring enabled (%s)\n",
>> +		       tm2 ? "TM2" : "TM1");
> 
> Hm, 'cpu==0 means boot cpu' assumptions are not particularly clean.

Yes.  It appears that boot_cpu_id is only defined for x86.  I don't know
any other way to identify the boot cpu.  (Hmm, in this case it *is* an
x86 specific function, so I'll change it.)

> 
>> +#ifdef CONFIG_NUMA
>> +	if (system_state == SYSTEM_BOOTING) {
>> +		static int current_node = -1;
>> +		int node = cpu_to_node(cpu);
>> +
>> +		if (node != current_node) {
>> +			if (current_node > (-1))
>> +				pr_cont(" Ok.\n");
>> +			current_node = node;
>> +			pr_info("Booting Node %3d, Processors ", node);
>> +		}
>> +		pr_cont(" #%d%s", cpu, cpu == (nr_cpu_ids - 1) ? " Ok.\n" : "");
>> +	} else
>> +#endif
>> +		pr_info("Booting Processor %d APIC 0x%x\n", cpu, apicid);
> 
> preprocessor directives cutting into if/else branches in an assymetric 
> way is being frowned upon. I'd also suggest to put this into a helper 
> inline.

ok.

> 
> Is the SYSTEM_BOOTING check there to not re-print this on CPU hotplug?

Actually you get the old style "Booting Processor" message for hotplug off/on.

newton:~ # cd /sys/devices/system/cpu
newton:/sys/devices/system/cpu # echo 0 > cpu6/online
[ 3037.593411] CPU 6 is now offline

newton:/sys/devices/system/cpu # echo 1 > cpu6/online
[ 3045.843469] Booting Processor 6 APIC 0x3

I can't test going into/out of the sleep modes, so it may be better to
summarize for those operations as well.

Thanks,
Mike

Re: [PATCH 2/6] x86: Limit the number of per cpu MCE bootup messages

[Mike Travis]

Ingo Molnar wrote:
> * Mike Travis <travis@sgi.com> wrote:
> 
>> @@ -83,6 +86,7 @@
>>  	unsigned long flags;
>>  	int hdr = 0;
>>  	int i;
>> +	char buf[120];
> 
> that constant is not particularly nice, is it?
> 
> 	Ingo

I'm up for suggestions.  I just noticed that during testing, the
MCE Banks messages overflowed 80 chars but I didn't actually
check to see what the longest might be.

Should I trim it to 80?  Or use a different constant?

Thanks,
Mike

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Cyrill Gorcunov]

On Mon, Nov 16, 2009 at 01:27:55PM -0800, H. Peter Anvin wrote:
> On 11/16/2009 01:24 PM, Ingo Molnar wrote:
> > 
> > * Mike Travis <travis@sgi.com> wrote:
> > 
> >> --- linux.orig/init/calibrate.c
> >> +++ linux/init/calibrate.c
> >> @@ -123,23 +123,26 @@
> >>  {
> >>  	unsigned long ticks, loopbit;
> >>  	int lps_precision = LPS_PREC;
> >> +	bool boot_cpu = (smp_processor_id() == 0);
> > 
> > this code is shared by other architectures too - are you sure 
> > smp_processor_id()==0 is a proper 'I am the boot CPU' assumption 
> > everywhere?
> > 
> 
> It really shouldn't be for x86 either, although right now we play nasty
> renumbering games to accommodate that assumption.  It seems like we
> really should have a boot_cpu_id() or some such.
> 
> 	-hpa
> 

It seems we have one

	arch/x86/kernel/setup.c:125:unsigned int boot_cpu_id __read_mostly;

	-- Cyrill

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Mike Travis]

Ingo Molnar wrote:
> * Mike Travis <travis@sgi.com> wrote:
> 
>> --- linux.orig/init/calibrate.c
>> +++ linux/init/calibrate.c
>> @@ -123,23 +123,26 @@
>>  {
>>  	unsigned long ticks, loopbit;
>>  	int lps_precision = LPS_PREC;
>> +	bool boot_cpu = (smp_processor_id() == 0);
> 
> this code is shared by other architectures too - are you sure 
> smp_processor_id()==0 is a proper 'I am the boot CPU' assumption 
> everywhere?
> 
> 	Ingo

This was where having the boot_cpu_id would have been handy.

I could add something like:

--- linux.orig/init/main.c
+++ linux/init/main.c
@@ -539,13 +539,15 @@
  *     Activate the first processor.
  */

+int boot_cpu_id __read_mostly;
+
 static void __init boot_cpu_init(void)
 {
-       int cpu = smp_processor_id();
+       int boot_cpu_id = smp_processor_id();
        /* Mark the boot cpu "present", "online" etc for SMP and UP case */
-       set_cpu_online(cpu, true);
-       set_cpu_present(cpu, true);
-       set_cpu_possible(cpu, true);
+       set_cpu_online(boot_cpu_id, true);
+       set_cpu_present(boot_cpu_id, true);
+       set_cpu_possible(boot_cpu_id, true);
 }

 void __init __weak smp_setup_processor_id(void)

and remove boot_cpu_id from arch/x86 ... ?

Thanks,
Mike

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[H. Peter Anvin]

On 11/16/2009 01:43 PM, Cyrill Gorcunov wrote:
> 
> It seems we have one
> 
> 	arch/x86/kernel/setup.c:125:unsigned int boot_cpu_id __read_mostly;
> 
> 	-- Cyrill

We probably should make it an inline function so that if other arches
want to define it to be a constant or some other kind of special thing
they can.

	-hpa

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[H. Peter Anvin]

On 11/16/2009 01:45 PM, Mike Travis wrote:
> 
> 
> Ingo Molnar wrote:
>> * Mike Travis <travis@sgi.com> wrote:
>>
>>> --- linux.orig/init/calibrate.c
>>> +++ linux/init/calibrate.c
>>> @@ -123,23 +123,26 @@
>>>  {
>>>  	unsigned long ticks, loopbit;
>>>  	int lps_precision = LPS_PREC;
>>> +	bool boot_cpu = (smp_processor_id() == 0);
>>
>> this code is shared by other architectures too - are you sure 
>> smp_processor_id()==0 is a proper 'I am the boot CPU' assumption 
>> everywhere?
>>
>> 	Ingo
> 
> This was where having the boot_cpu_id would have been handy.
> 
> I could add something like:
> 
> --- linux.orig/init/main.c
> +++ linux/init/main.c
> @@ -539,13 +539,15 @@
>   *     Activate the first processor.
>   */
> 
> +int boot_cpu_id __read_mostly;
   ^^^^^^^^^^^^^^^
> +
>  static void __init boot_cpu_init(void)
>  {
> -       int cpu = smp_processor_id();
> +       int boot_cpu_id = smp_processor_id();
          ^^^^^^^^^^^^^^^
>         /* Mark the boot cpu "present", "online" etc for SMP and UP case */

Doesn't really work, does it?

I also still think that wrapping it in an inline would be good.

	-hpa

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Cyrill Gorcunov]

On Mon, Nov 16, 2009 at 01:46:07PM -0800, H. Peter Anvin wrote:
> On 11/16/2009 01:43 PM, Cyrill Gorcunov wrote:
> > 
> > It seems we have one
> > 
> > 	arch/x86/kernel/setup.c:125:unsigned int boot_cpu_id __read_mostly;
> > 
> > 	-- Cyrill
> 
> We probably should make it an inline function so that if other arches
> want to define it to be a constant or some other kind of special thing
> they can.
> 
> 	-hpa
> 

IA-64 and SPARC already has this variable. But boot_cpu_id() as an
inline function seem to be more natural/portable ineed.

	-- Cyrill

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Mike Travis]

H. Peter Anvin wrote:
> On 11/16/2009 01:45 PM, Mike Travis wrote:
>>
>> Ingo Molnar wrote:
>>> * Mike Travis <travis@sgi.com> wrote:
>>>
>>>> --- linux.orig/init/calibrate.c
>>>> +++ linux/init/calibrate.c
>>>> @@ -123,23 +123,26 @@
>>>>  {
>>>>  	unsigned long ticks, loopbit;
>>>>  	int lps_precision = LPS_PREC;
>>>> +	bool boot_cpu = (smp_processor_id() == 0);
>>> this code is shared by other architectures too - are you sure 
>>> smp_processor_id()==0 is a proper 'I am the boot CPU' assumption 
>>> everywhere?
>>>
>>> 	Ingo
>> This was where having the boot_cpu_id would have been handy.
>>
>> I could add something like:
>>
>> --- linux.orig/init/main.c
>> +++ linux/init/main.c
>> @@ -539,13 +539,15 @@
>>   *     Activate the first processor.
>>   */
>>
>> +int boot_cpu_id __read_mostly;
>    ^^^^^^^^^^^^^^^
>> +
>>  static void __init boot_cpu_init(void)
>>  {
>> -       int cpu = smp_processor_id();
>> +       int boot_cpu_id = smp_processor_id();
>           ^^^^^^^^^^^^^^^
>>         /* Mark the boot cpu "present", "online" etc for SMP and UP case */
> 
> Doesn't really work, does it?
> 
> I also still think that wrapping it in an inline would be good.
> 
> 	-hpa

Yes, a bit quick on the reply button.  But the proposal was clear enough?

I've just looked at this, and there are many, many references in arch-specific
code to a scalar "boot_cpu_id".  Maybe a better approach would be to add an
inline named something else?  get_boot_cpu_id() perhaps?  Or an inline
function "is_boot_cpu()" ... ?

There's also some confusion on whether the boot_cpu_id is the APIC id (ia64)
or the cpu_index (x86) or some other number (others).   (cpu_index 0 by
definition will always be the boot cpu.)

Here's an initial stab (by no means complete) at adding boot_cpu_id().  As
you can see the reach is quite extensive.


INIT: Identify processor id of boot cpu

Provide an architecture-independent means of identifying
the boot cpu's processor id.

Signed-of-by: Mike Travis <travis@sgi.com>
---
 arch/alpha/kernel/smp.c        |    2 +-
 arch/sparc/kernel/smp_32.c     |    1 -
 arch/x86/include/asm/cpu.h     |    2 --
 arch/x86/kernel/apic/io_apic.c |   10 +++++-----
 arch/x86/kernel/cpu/amd.c      |    2 +-
 arch/x86/kernel/cpu/common.c   |    4 ++--
 arch/x86/kernel/cpu/intel.c    |    2 +-
 arch/x86/kernel/setup.c        |    2 --
 arch/x86/kernel/setup_percpu.c |    4 ++--
 include/linux/smp.h            |   13 +++++++++++++
 init/main.c                    |   12 +++++++-----
 11 files changed, 32 insertions(+), 22 deletions(-)

--- linux.orig/arch/alpha/kernel/smp.c
+++ linux/arch/alpha/kernel/smp.c
@@ -633,7 +633,7 @@
 	cpumask_t to_whom = cpu_possible_map;
 	cpu_clear(smp_processor_id(), to_whom);
 #ifdef DEBUG_IPI_MSG
-	if (hard_smp_processor_id() != boot_cpu_id)
+	if (hard_smp_processor_id() != boot_cpu_id())
 		printk(KERN_WARNING "smp_send_stop: Not on boot cpu.\n");
 #endif
 	send_ipi_message(&to_whom, IPI_CPU_STOP);
--- linux.orig/arch/sparc/kernel/smp_32.c
+++ linux/arch/sparc/kernel/smp_32.c
@@ -36,7 +36,6 @@
 #include "irq.h"
 
 volatile unsigned long cpu_callin_map[NR_CPUS] __cpuinitdata = {0,};
-unsigned char boot_cpu_id = 0;
 unsigned char boot_cpu_id4 = 0; /* boot_cpu_id << 2 */
 
 cpumask_t smp_commenced_mask = CPU_MASK_NONE;
--- linux.orig/arch/x86/include/asm/cpu.h
+++ linux/arch/x86/include/asm/cpu.h
@@ -32,6 +32,4 @@
 
 DECLARE_PER_CPU(int, cpu_state);
 
-extern unsigned int boot_cpu_id;
-
 #endif /* _ASM_X86_CPU_H */
--- linux.orig/arch/x86/kernel/apic/io_apic.c
+++ linux/arch/x86/kernel/apic/io_apic.c
@@ -197,7 +197,7 @@
 
 	cfg = irq_cfgx;
 	count = ARRAY_SIZE(irq_cfgx);
-	node= cpu_to_node(boot_cpu_id);
+	node= cpu_to_node(boot_cpu_id());
 
 	for (i = 0; i < count; i++) {
 		desc = irq_to_desc(i);
@@ -1496,7 +1496,7 @@
 	int notcon = 0;
 	struct irq_desc *desc;
 	struct irq_cfg *cfg;
-	int node = cpu_to_node(boot_cpu_id);
+	int node = cpu_to_node(boot_cpu_id());
 
 	apic_printk(APIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");
 
@@ -2828,7 +2828,7 @@
 {
 	struct irq_desc *desc = irq_to_desc(0);
 	struct irq_cfg *cfg = desc->chip_data;
-	int node = cpu_to_node(boot_cpu_id);
+	int node = cpu_to_node(boot_cpu_id());
 	int apic1, pin1, apic2, pin2;
 	unsigned long flags;
 	int no_pin1 = 0;
@@ -3184,7 +3184,7 @@
 
 int create_irq(void)
 {
-	int node = cpu_to_node(boot_cpu_id);
+	int node = cpu_to_node(boot_cpu_id());
 	unsigned int irq_want;
 	int irq;
 
@@ -3861,7 +3861,7 @@
 	if (dev)
 		node = dev_to_node(dev);
 	else
-		node = cpu_to_node(boot_cpu_id);
+		node = cpu_to_node(boot_cpu_id());
 
 	desc = irq_to_desc_alloc_node(irq, node);
 	if (!desc) {
--- linux.orig/arch/x86/kernel/cpu/amd.c
+++ linux/arch/x86/kernel/cpu/amd.c
@@ -148,7 +148,7 @@
 {
 #ifdef CONFIG_SMP
 	/* calling is from identify_secondary_cpu() ? */
-	if (c->cpu_index == boot_cpu_id)
+	if (raw_smp_processor_id() == boot_cpu_id())
 		return;
 
 	/*
--- linux.orig/arch/x86/kernel/cpu/common.c
+++ linux/arch/x86/kernel/cpu/common.c
@@ -649,7 +649,7 @@
 		this_cpu->c_early_init(c);
 
 #ifdef CONFIG_SMP
-	c->cpu_index = boot_cpu_id;
+	c->cpu_index = boot_cpu_id();
 #endif
 	filter_cpuid_features(c, false);
 }
@@ -1251,7 +1251,7 @@
 	/*
 	 * Boot processor to setup the FP and extended state context info.
 	 */
-	if (smp_processor_id() == boot_cpu_id)
+	if (smp_processor_id() == boot_cpu_id())
 		init_thread_xstate();
 
 	xsave_init();
--- linux.orig/arch/x86/kernel/cpu/intel.c
+++ linux/arch/x86/kernel/cpu/intel.c
@@ -149,7 +149,7 @@
 {
 #ifdef CONFIG_SMP
 	/* calling is from identify_secondary_cpu() ? */
-	if (c->cpu_index == boot_cpu_id)
+	if (raw_smp_processor_id() == boot_cpu_id())
 		return;
 
 	/*
--- linux.orig/arch/x86/kernel/setup.c
+++ linux/arch/x86/kernel/setup.c
@@ -120,8 +120,6 @@
 
 RESERVE_BRK(dmi_alloc, 65536);
 
-unsigned int boot_cpu_id __read_mostly;
-
 static __initdata unsigned long _brk_start = (unsigned long)__brk_base;
 unsigned long _brk_end = (unsigned long)__brk_base;
 
--- linux.orig/arch/x86/kernel/setup_percpu.c
+++ linux/arch/x86/kernel/setup_percpu.c
@@ -243,7 +243,7 @@
 		 * Up to this point, the boot CPU has been using .data.init
 		 * area.  Reload any changed state for the boot CPU.
 		 */
-		if (cpu == boot_cpu_id)
+		if (cpu == boot_cpu_id())
 			switch_to_new_gdt(cpu);
 	}
 
@@ -261,7 +261,7 @@
 	 * make sure boot cpu node_number is right, when boot cpu is on the
 	 * node that doesn't have mem installed
 	 */
-	per_cpu(node_number, boot_cpu_id) = cpu_to_node(boot_cpu_id);
+	per_cpu(node_number, boot_cpu_id()) = cpu_to_node(boot_cpu_id());
 #endif
 
 	/* Setup node to cpumask map */
--- linux.orig/include/linux/smp.h
+++ linux/include/linux/smp.h
@@ -109,6 +109,17 @@
  */
 void smp_prepare_boot_cpu(void);
 
+/*
+ * Identify processor ID of boot cpu
+ */
+extern int _boot_cpu_id;
+
+static inline int boot_cpu_id(void)
+{
+	return _boot_cpu_id;
+}
+
+
 extern unsigned int setup_max_cpus;
 
 #else /* !SMP */
@@ -119,6 +130,8 @@
  *	These macros fold the SMP functionality into a single CPU system
  */
 #define raw_smp_processor_id()			0
+#define	boot_cpu_id()				0
+
 static inline int up_smp_call_function(void (*func)(void *), void *info)
 {
 	return 0;
--- linux.orig/init/main.c
+++ linux/init/main.c
@@ -538,14 +538,16 @@
 /*
  *	Activate the first processor.
  */
+int _boot_cpu_id __read_mostly;
+EXPORT_SYMBOL(_boot_cpu_id);
 
-static void __init boot_cpu_init(void)
+void __init __weak boot_cpu_init(void)
 {
-	int cpu = smp_processor_id();
 	/* Mark the boot cpu "present", "online" etc for SMP and UP case */
-	set_cpu_online(cpu, true);
-	set_cpu_present(cpu, true);
-	set_cpu_possible(cpu, true);
+	_boot_cpu_id = raw_smp_processor_id();
+	set_cpu_online(_boot_cpu_id, true);
+	set_cpu_present(_boot_cpu_id, true);
+	set_cpu_possible(_boot_cpu_id, true);
 }
 
 void __init __weak smp_setup_processor_id(void)

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[H. Peter Anvin]

On 11/16/2009 02:51 PM, Mike Travis wrote:
> 
> There's also some confusion on whether the boot_cpu_id is the APIC id (ia64)
> or the cpu_index (x86) or some other number (others).   (cpu_index 0 by
> definition will always be the boot cpu.)
> 

This is part of what I said... the whole cpu_index thing is a rather
awkward extra layer of indirection.

But regardless... we have a panarchitectural notion of a cpu ID, and
that is what boot_cpu_id() [or whatever we call it] should refer to.  On
some architectures that will be constant.

	-hpa

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[David Miller]

From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Tue, 17 Nov 2009 00:50:52 +0300

> On Mon, Nov 16, 2009 at 01:46:07PM -0800, H. Peter Anvin wrote:
>> On 11/16/2009 01:43 PM, Cyrill Gorcunov wrote:
>> > 
>> > It seems we have one
>> > 
>> > 	arch/x86/kernel/setup.c:125:unsigned int boot_cpu_id __read_mostly;
>> > 
>> > 	-- Cyrill
>> 
>> We probably should make it an inline function so that if other arches
>> want to define it to be a constant or some other kind of special thing
>> they can.
>
> IA-64 and SPARC already has this variable. But boot_cpu_id() as an
> inline function seem to be more natural/portable ineed.

Only 32-bit SPARC actually has it.  On sparc64 we have no reason to
remember which processor was the boot cpu, and remembering it merely
for the sake of only printing out the bogomips message once seems a
bit excessive?

How about:

	static bool printed;

	if (!printed) {
		printk(...);
		printed = true;
	}

Or, alternatively, use an atomic_t instead of a bool if you think
races matter this early in the boot process.

Re: [PATCH 2/6] x86: Limit the number of per cpu MCE bootup messages

[Hidetoshi Seto]

Mike Travis wrote:
> 
> 
> Ingo Molnar wrote:
>> * Mike Travis <travis@sgi.com> wrote:
>>
>>> @@ -83,6 +86,7 @@
>>>      unsigned long flags;
>>>      int hdr = 0;
>>>      int i;
>>> +    char buf[120];
>>
>> that constant is not particularly nice, is it?
>>
>>     Ingo
> 
> I'm up for suggestions.  I just noticed that during testing, the
> MCE Banks messages overflowed 80 chars but I didn't actually
> check to see what the longest might be.
> 
> Should I trim it to 80?  Or use a different constant?

I think you could calculate the size using MAX_NR_BANKS.

But I'd like to change the format to shorter one at same time, 
So how about the following?


Thanks,
H.Seto

===

[PATCH] x86, mce: rework output of MCE banks ownership information

The output of MCE banks ownership information on boot tend
to be long on new processor which has many banks:

  CPU 1 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18 SHD:19 SHD:20 SHD:21

This message can fill up the console output when the number
of cpus is large.

This patch suppress this info message on boot, and introduce
debug message in shorter format instead, like:

  CPU 1 MCE banks map: ssCC PCss ssPP ssss ssss ss

  where: s: shared, C: checked by cmci, P: checked by poll.

This patch still keep the info when ownership is updated.
E.g. if a cpu take over the ownership from hot-removed cpu,
both message will be shown:

  CPU 1 MCE banks map updated: CMCI:6 CMCI:7 CMCI:10 CMCI:11
  CPU 1 MCE banks map: ssCC PCCC ssPP ssCC ssss ss

v2:
  - stop changing the level of message on update
  - change the number of banks message on boot to debug level

Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c       |    6 +++---
 arch/x86/kernel/cpu/mcheck/mce_intel.c |   29 +++++++++++++++++++++++------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 5f277ca..8627976 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1229,11 +1229,11 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
 
 	b = cap & MCG_BANKCNT_MASK;
 	if (!banks)
-		printk(KERN_INFO "mce: CPU supports %d MCE banks\n", b);
+		pr_debug("mce: CPU supports %d MCE banks\n", b);
 
 	if (b > MAX_NR_BANKS) {
-		printk(KERN_WARNING
-		       "MCE: Using only %u machine check banks out of %u\n",
+		pr_warning(
+			"MCE: Using only %u machine check banks out of %u\n",
 			MAX_NR_BANKS, b);
 		b = MAX_NR_BANKS;
 	}
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 7c78563..448a38b 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -64,12 +64,25 @@ static void intel_threshold_interrupt(void)
 	mce_notify_irq();
 }
 
+static void print_banks_map(int banks)
+{
+	int i;
+
+	pr_debug("CPU %d MCE banks map:", smp_processor_id());
+	for (i = 0; i < banks; i++) {
+		pr_cont("%s%s", (i % 4) ? "" : " ",
+			test_bit(i, __get_cpu_var(mce_banks_owned)) ? "C" :
+			test_bit(i, __get_cpu_var(mce_poll_banks)) ? "P" : "s");
+	}
+	pr_cont("\n");
+}
+
 static void print_update(char *type, int *hdr, int num)
 {
 	if (*hdr == 0)
-		printk(KERN_INFO "CPU %d MCA banks", smp_processor_id());
+		pr_info("CPU %d MCE banks map updated:", smp_processor_id());
 	*hdr = 1;
-	printk(KERN_CONT " %s:%d", type, num);
+	pr_cont(" %s:%d", type, num);
 }
 
 /*
@@ -85,6 +98,7 @@ static void cmci_discover(int banks, int boot)
 	int i;
 
 	spin_lock_irqsave(&cmci_discover_lock, flags);
+
 	for (i = 0; i < banks; i++) {
 		u64 val;
 
@@ -95,7 +109,7 @@ static void cmci_discover(int banks, int boot)
 
 		/* Already owned by someone else? */
 		if (val & CMCI_EN) {
-			if (test_and_clear_bit(i, owned) || boot)
+			if (test_and_clear_bit(i, owned) && !boot)
 				print_update("SHD", &hdr, i);
 			__clear_bit(i, __get_cpu_var(mce_poll_banks));
 			continue;
@@ -107,16 +121,19 @@ static void cmci_discover(int banks, int boot)
 
 		/* Did the enable bit stick? -- the bank supports CMCI */
 		if (val & CMCI_EN) {
-			if (!test_and_set_bit(i, owned) || boot)
+			if (!test_and_set_bit(i, owned) && !boot)
 				print_update("CMCI", &hdr, i);
 			__clear_bit(i, __get_cpu_var(mce_poll_banks));
 		} else {
 			WARN_ON(!test_bit(i, __get_cpu_var(mce_poll_banks)));
 		}
 	}
-	spin_unlock_irqrestore(&cmci_discover_lock, flags);
 	if (hdr)
-		printk(KERN_CONT "\n");
+		pr_cont("\n");
+	if (hdr || boot)
+		print_banks_map(banks);
+
+	spin_unlock_irqrestore(&cmci_discover_lock, flags);
 }
 
 /*
-- 
1.6.5.2

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Cyrill Gorcunov]

On Mon, Nov 16, 2009 at 07:09:22PM -0800, David Miller wrote:
...
> >
> > IA-64 and SPARC already has this variable. But boot_cpu_id() as an
> > inline function seem to be more natural/portable ineed.
> 
> Only 32-bit SPARC actually has it.  On sparc64 we have no reason to
> remember which processor was the boot cpu, and remembering it merely
> for the sake of only printing out the bogomips message once seems a
> bit excessive?
> 
> How about:
> 
> 	static bool printed;
> 
> 	if (!printed) {
> 		printk(...);
> 		printed = true;
> 	}
> 
> Or, alternatively, use an atomic_t instead of a bool if you think
> races matter this early in the boot process.
> 

Hi David,

I must admit I know *nothing* about SPARC code. I was rather pointing
out that some archs already use this variable.  IOW it means
that per-platform boot_cpu_id() helper implementation may be not
that simple. Perhaps for other archs like SPARC64, where as you
said no need to remember boot cpu id at all, we should define
some __weak per-kernel global helper which would return 0
and every arch would implement own helper boot_cpu_id().

	-- Cyrill

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[David Miller]

From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Tue, 17 Nov 2009 18:59:46 +0300

> Perhaps for other archs like SPARC64, where as you said no need to
> remember boot cpu id at all, we should define some __weak per-kernel
> global helper which would return 0 and every arch would implement
> own helper boot_cpu_id().

On many of my machines none of my cpus are numbered "0", so that
wouldn't be a legitimate implementation on sparc64.

I see no reason for a platform the be required to remember the boot
cpu ID, there is nothing special about that processor generically.

And if we do need it generically, it's available there as
hard_smp_processor_id() when start_kernel() is called.  So init/main.c
could remember that value in an __initdata annotated static variable.

But just using a boolean for this "did I print the bogomips message
already?" thing seems more than sufficient.

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Mike Travis]

David Miller wrote:
> From: Cyrill Gorcunov <gorcunov@gmail.com>
> Date: Tue, 17 Nov 2009 00:50:52 +0300
> 
>> On Mon, Nov 16, 2009 at 01:46:07PM -0800, H. Peter Anvin wrote:
>>> On 11/16/2009 01:43 PM, Cyrill Gorcunov wrote:
>>>> It seems we have one
>>>>
>>>> 	arch/x86/kernel/setup.c:125:unsigned int boot_cpu_id __read_mostly;
>>>>
>>>> 	-- Cyrill
>>> We probably should make it an inline function so that if other arches
>>> want to define it to be a constant or some other kind of special thing
>>> they can.
>> IA-64 and SPARC already has this variable. But boot_cpu_id() as an
>> inline function seem to be more natural/portable ineed.
> 
> Only 32-bit SPARC actually has it.  On sparc64 we have no reason to
> remember which processor was the boot cpu, and remembering it merely
> for the sake of only printing out the bogomips message once seems a
> bit excessive?
> 
> How about:
> 
> 	static bool printed;
> 
> 	if (!printed) {
> 		printk(...);
> 		printed = true;
> 	}
> 
> Or, alternatively, use an atomic_t instead of a bool if you think
> races matter this early in the boot process.

Yeah, I was thinking along these same lines.  Thanks for the feedback!

Mike

Re: [PATCH 2/6] x86: Limit the number of per cpu MCE bootup messages

[Mike Travis]

Hidetoshi Seto wrote:
> Mike Travis wrote:
>>
>> Ingo Molnar wrote:
>>> * Mike Travis <travis@sgi.com> wrote:
>>>
>>>> @@ -83,6 +86,7 @@
>>>>      unsigned long flags;
>>>>      int hdr = 0;
>>>>      int i;
>>>> +    char buf[120];
>>> that constant is not particularly nice, is it?
>>>
>>>     Ingo
>> I'm up for suggestions.  I just noticed that during testing, the
>> MCE Banks messages overflowed 80 chars but I didn't actually
>> check to see what the longest might be.
>>
>> Should I trim it to 80?  Or use a different constant?
> 
> I think you could calculate the size using MAX_NR_BANKS.
> 
> But I'd like to change the format to shorter one at same time, 
> So how about the following?
> 
> 
> Thanks,
> H.Seto
> 
> ===
> 
> [PATCH] x86, mce: rework output of MCE banks ownership information
> 
> The output of MCE banks ownership information on boot tend
> to be long on new processor which has many banks:
> 
>   CPU 1 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18 SHD:19 SHD:20 SHD:21
> 
> This message can fill up the console output when the number
> of cpus is large.
> 
> This patch suppress this info message on boot, and introduce
> debug message in shorter format instead, like:
> 
>   CPU 1 MCE banks map: ssCC PCss ssPP ssss ssss ss
> 
>   where: s: shared, C: checked by cmci, P: checked by poll.
> 
> This patch still keep the info when ownership is updated.
> E.g. if a cpu take over the ownership from hot-removed cpu,
> both message will be shown:
> 
>   CPU 1 MCE banks map updated: CMCI:6 CMCI:7 CMCI:10 CMCI:11
>   CPU 1 MCE banks map: ssCC PCCC ssPP ssCC ssss ss
> 
> v2:
>   - stop changing the level of message on update
>   - change the number of banks message on boot to debug level
> 
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> ---
>  arch/x86/kernel/cpu/mcheck/mce.c       |    6 +++---
>  arch/x86/kernel/cpu/mcheck/mce_intel.c |   29 +++++++++++++++++++++++------
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index 5f277ca..8627976 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -1229,11 +1229,11 @@ static int __cpuinit __mcheck_cpu_cap_init(void)
>  
>  	b = cap & MCG_BANKCNT_MASK;
>  	if (!banks)
> -		printk(KERN_INFO "mce: CPU supports %d MCE banks\n", b);
> +		pr_debug("mce: CPU supports %d MCE banks\n", b);
>  
>  	if (b > MAX_NR_BANKS) {
> -		printk(KERN_WARNING
> -		       "MCE: Using only %u machine check banks out of %u\n",
> +		pr_warning(
> +			"MCE: Using only %u machine check banks out of %u\n",
>  			MAX_NR_BANKS, b);
>  		b = MAX_NR_BANKS;
>  	}
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
> index 7c78563..448a38b 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
> @@ -64,12 +64,25 @@ static void intel_threshold_interrupt(void)
>  	mce_notify_irq();
>  }
>  
> +static void print_banks_map(int banks)
> +{
> +	int i;
> +
> +	pr_debug("CPU %d MCE banks map:", smp_processor_id());
> +	for (i = 0; i < banks; i++) {
> +		pr_cont("%s%s", (i % 4) ? "" : " ",
> +			test_bit(i, __get_cpu_var(mce_banks_owned)) ? "C" :
> +			test_bit(i, __get_cpu_var(mce_poll_banks)) ? "P" : "s");
> +	}
> +	pr_cont("\n");

The problem here is that if pr_debug is not in effect, then the pr_cont("\n")
outputs a newline in the middle of other messages.  I had introduced a new
macro (pr_debug_cont) but it was just as easy to buffer the message and then
print the entire thing with pr_debug().

I think pr_cont() should be reserved for when you actually want a partial
line printed before the end of the line, and not a means to stitch together
a complete line, but I may be in the minority.

This might also be another candidate for printing the needed information via
/sys or /proc ... in which case as Ingo has pointed out, the debug bootup
messages should be simple even to the point of voluminous, and the reports
compacted. 

> +}
> +
>  static void print_update(char *type, int *hdr, int num)
>  {
>  	if (*hdr == 0)
> -		printk(KERN_INFO "CPU %d MCA banks", smp_processor_id());
> +		pr_info("CPU %d MCE banks map updated:", smp_processor_id());
>  	*hdr = 1;
> -	printk(KERN_CONT " %s:%d", type, num);
> +	pr_cont(" %s:%d", type, num);
>  }
>  
>  /*
> @@ -85,6 +98,7 @@ static void cmci_discover(int banks, int boot)
>  	int i;
>  
>  	spin_lock_irqsave(&cmci_discover_lock, flags);
> +
>  	for (i = 0; i < banks; i++) {
>  		u64 val;
>  
> @@ -95,7 +109,7 @@ static void cmci_discover(int banks, int boot)
>  
>  		/* Already owned by someone else? */
>  		if (val & CMCI_EN) {
> -			if (test_and_clear_bit(i, owned) || boot)
> +			if (test_and_clear_bit(i, owned) && !boot)
>  				print_update("SHD", &hdr, i);
>  			__clear_bit(i, __get_cpu_var(mce_poll_banks));
>  			continue;
> @@ -107,16 +121,19 @@ static void cmci_discover(int banks, int boot)
>  
>  		/* Did the enable bit stick? -- the bank supports CMCI */
>  		if (val & CMCI_EN) {
> -			if (!test_and_set_bit(i, owned) || boot)
> +			if (!test_and_set_bit(i, owned) && !boot)
>  				print_update("CMCI", &hdr, i);
>  			__clear_bit(i, __get_cpu_var(mce_poll_banks));
>  		} else {
>  			WARN_ON(!test_bit(i, __get_cpu_var(mce_poll_banks)));
>  		}
>  	}
> -	spin_unlock_irqrestore(&cmci_discover_lock, flags);
>  	if (hdr)
> -		printk(KERN_CONT "\n");
> +		pr_cont("\n");

Here again an extraneous newline will be printed in the middle of other messages.

> +	if (hdr || boot)
> +		print_banks_map(banks);
> +
> +	spin_unlock_irqrestore(&cmci_discover_lock, flags);
>  }
>  
>  /*

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Cyrill Gorcunov]

On Tue, Nov 17, 2009 at 08:29:28AM -0800, David Miller wrote:
> From: Cyrill Gorcunov <gorcunov@gmail.com>
> Date: Tue, 17 Nov 2009 18:59:46 +0300
> 
> > Perhaps for other archs like SPARC64, where as you said no need to
> > remember boot cpu id at all, we should define some __weak per-kernel
> > global helper which would return 0 and every arch would implement
> > own helper boot_cpu_id().
> 
> On many of my machines none of my cpus are numbered "0", so that
> wouldn't be a legitimate implementation on sparc64.
> 
> I see no reason for a platform the be required to remember the boot
> cpu ID, there is nothing special about that processor generically.

I fear we still need it and it's special due to code structure at
least. For SMP compiled kernel say callin() do change its behaviour
depending on which cpu it's called.

Also iy seems a differ techhique used to find out on which cpu
the code is running: there is smp_processor_id() == 0 and raw version
and boot_cpu_id and cpu == 0, so having one general boot_cpu_id() would
be more clear (though we will need to clean code up then :)

So plain hard_smp_processor_id() wouldn't help since it doesn't
say if this is a boot cpu or not.

> 
> And if we do need it generically, it's available there as
> hard_smp_processor_id() when start_kernel() is called.  So init/main.c
> could remember that value in an __initdata annotated static variable.
> 
> But just using a boolean for this "did I print the bogomips message
> already?" thing seems more than sufficient.
> 

Yes. As I see Mike already pick it up. Thanks David!

	-- Cyrill

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Mike Travis]

Cyrill Gorcunov wrote:
> On Tue, Nov 17, 2009 at 08:29:28AM -0800, David Miller wrote:
>> From: Cyrill Gorcunov <gorcunov@gmail.com>
>> Date: Tue, 17 Nov 2009 18:59:46 +0300
>>
>>> Perhaps for other archs like SPARC64, where as you said no need to
>>> remember boot cpu id at all, we should define some __weak per-kernel
>>> global helper which would return 0 and every arch would implement
>>> own helper boot_cpu_id().
>> On many of my machines none of my cpus are numbered "0", so that
>> wouldn't be a legitimate implementation on sparc64.
>>
>> I see no reason for a platform the be required to remember the boot
>> cpu ID, there is nothing special about that processor generically.
> 
> I fear we still need it and it's special due to code structure at
> least. For SMP compiled kernel say callin() do change its behaviour
> depending on which cpu it's called.
> 
> Also iy seems a differ techhique used to find out on which cpu
> the code is running: there is smp_processor_id() == 0 and raw version
> and boot_cpu_id and cpu == 0, so having one general boot_cpu_id() would
> be more clear (though we will need to clean code up then :)
> 
> So plain hard_smp_processor_id() wouldn't help since it doesn't
> say if this is a boot cpu or not.
> 
>> And if we do need it generically, it's available there as
>> hard_smp_processor_id() when start_kernel() is called.  So init/main.c
>> could remember that value in an __initdata annotated static variable.
>>
>> But just using a boolean for this "did I print the bogomips message
>> already?" thing seems more than sufficient.
>>
> 
> Yes. As I see Mike already pick it up. Thanks David!
> 
> 	-- Cyrill

I'd like to say that, but Peter wanted it to become an inlined function
return value, and there are too many references in too many arches to
a scalar value, so that moves it out of the scope of this patch set.

Thanks,
Mike

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[H. Peter Anvin]

On 11/17/2009 09:49 AM, Mike Travis wrote:
> 
> I'd like to say that, but Peter wanted it to become an inlined function
> return value, and there are too many references in too many arches to
> a scalar value, so that moves it out of the scope of this patch set.
> 

Another thing: if we do a lot of testing for "are we running on the boot
CPU", an is_boot_cpu() or is_boot_cpu(cpu) function might be a nice
piece of syntactic sugar (and more immediately implementable.)

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 3/6] INIT: Limit the number of per cpu calibration bootup messages

[Cyrill Gorcunov]

On Tue, Nov 17, 2009 at 09:49:54AM -0800, Mike Travis wrote:
...
>>> And if we do need it generically, it's available there as
>>> hard_smp_processor_id() when start_kernel() is called.  So init/main.c
>>> could remember that value in an __initdata annotated static variable.
>>>
>>> But just using a boolean for this "did I print the bogomips message
>>> already?" thing seems more than sufficient.
>>>
>>
>> Yes. As I see Mike already pick it up. Thanks David!
>>
>> 	-- Cyrill
>
> I'd like to say that, but Peter wanted it to become an inlined function
> return value, and there are too many references in too many arches to
> a scalar value, so that moves it out of the scope of this patch set.

I meant only "local static variable being used for printing to suppress recursion"
idea rather then boot_cpu_id() inliner at moment [btw there was some thread
someday about printk_once (or something like that) though I'm not sure if patches
were merged] :) As only the inliner will be ready we will just switch to use it
eliminating this static variable. That is how I imagine it.

> Thanks,
> Mike
>
	-- Cyrill

[PATCH] x86, mce: rework output of MCE banks ownership information

[Mike Travis]

Author: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>

The output of MCE banks ownership information on boot tend
to be long on new processor which has many banks:

  CPU 1 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18 SHD:19 SHD:20 SHD:21

This message can fill up the console output when the number
of cpus is large.

This patch suppress this info message on boot, and introduce
debug message in shorter format instead, like:

  CPU 1 MCE banks map: ssCC PCss ssPP ssss ssss ss

  where: s: shared, C: checked by cmci, P: checked by poll.

This patch still keep the info when ownership is updated.
E.g. if a cpu take over the ownership from hot-removed cpu,
both message will be shown:

  CPU 1 MCE banks map updated: CMCI:6 CMCI:7 CMCI:10 CMCI:11
  CPU 1 MCE banks map: ssCC PCCC ssPP ssCC ssss ss

v2:
  - stop changing the level of message on update
  - change the number of banks message on boot to debug level

Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>

  - Modified to not use pr_cont().

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c       |    6 +--
 arch/x86/kernel/cpu/mcheck/mce_intel.c |   63 +++++++++++++++++++++++++++------
 2 files changed, 55 insertions(+), 14 deletions(-)

--- linux.orig/arch/x86/kernel/cpu/mcheck/mce.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1215,11 +1215,11 @@
 
 	b = cap & MCG_BANKCNT_MASK;
 	if (!banks)
-		printk(KERN_INFO "mce: CPU supports %d MCE banks\n", b);
+		pr_debug("mce: CPU supports %d MCE banks\n", b);
 
 	if (b > MAX_NR_BANKS) {
-		printk(KERN_WARNING
-		       "MCE: Using only %u machine check banks out of %u\n",
+		pr_warning(
+			"MCE: Using only %u machine check banks out of %u\n",
 			MAX_NR_BANKS, b);
 		b = MAX_NR_BANKS;
 	}
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -64,14 +64,50 @@
 	mce_notify_irq();
 }
 
-static void print_update(char *type, int *hdr, int num)
+#define	MCE_MSG_LEN	120
+
+#ifdef DEBUG_KERNEL
+static void print_banks_map(int banks, char *buf)
+{
+	int i, n;
+
+	n = snprintf(buf, MCE_MSG_LEN, "CPU %d MCE banks map:",
+							smp_processor_id());
+	for (i = 0; i < banks; i++) {
+		n += snprintf(&buf[n], MCE_MSG_LEN - n,
+			"%s%s", (i % 4) ? "" : " ",
+			test_bit(i, __get_cpu_var(mce_banks_owned)) ? "C" :
+			test_bit(i, __get_cpu_var(mce_poll_banks)) ? "P" : "s");
+	}
+
+	/* (indicate if message buffer overflowed) */
+	pr_debug("%s%s\n", buf, n < MCE_MSG_LEN ? "" : "..." );
+}
+
+static void print_update(char *type, int *hdr, int num, char *buf)
+{
+	int n = *hdr;
+
+	if (n == 0)
+		n = snprintf(buf, MCE_MSG_LEN,
+			"CPU %d MCE banks map updated:", smp_processor_id());
+
+	n += snprintf(&buf[n], MCE_MSG_LEN - n, " %s:%d", type, num);
+	*hdr = n;
+}
+
+#else /* !DEBUG_KERNEL */
+
+static inline void print_banks_map(int banks, char *buf)
+{
+}
+
+static inline void print_update(char *type, int *hdr, int num, char *buf)
 {
-	if (*hdr == 0)
-		printk(KERN_INFO "CPU %d MCA banks", smp_processor_id());
-	*hdr = 1;
-	printk(KERN_CONT " %s:%d", type, num);
 }
 
+#endif
+
 /*
  * Enable CMCI (Corrected Machine Check Interrupt) for available MCE banks
  * on this CPU. Use the algorithm recommended in the SDM to discover shared
@@ -83,8 +119,10 @@
 	unsigned long flags;
 	int hdr = 0;
 	int i;
+	char buf[MCE_MSG_LEN];
 
 	spin_lock_irqsave(&cmci_discover_lock, flags);
+
 	for (i = 0; i < banks; i++) {
 		u64 val;
 
@@ -95,8 +133,8 @@
 
 		/* Already owned by someone else? */
 		if (val & CMCI_EN) {
-			if (test_and_clear_bit(i, owned) || boot)
-				print_update("SHD", &hdr, i);
+			if (test_and_clear_bit(i, owned) && !boot)
+				print_update("SHD", &hdr, i, buf);
 			__clear_bit(i, __get_cpu_var(mce_poll_banks));
 			continue;
 		}
@@ -107,16 +145,19 @@
 
 		/* Did the enable bit stick? -- the bank supports CMCI */
 		if (val & CMCI_EN) {
-			if (!test_and_set_bit(i, owned) || boot)
-				print_update("CMCI", &hdr, i);
+			if (!test_and_set_bit(i, owned) && !boot)
+				print_update("CMCI", &hdr, i, buf);
 			__clear_bit(i, __get_cpu_var(mce_poll_banks));
 		} else {
 			WARN_ON(!test_bit(i, __get_cpu_var(mce_poll_banks)));
 		}
 	}
-	spin_unlock_irqrestore(&cmci_discover_lock, flags);
 	if (hdr)
-		printk(KERN_CONT "\n");
+		pr_debug("%s%s\n", buf, hdr < MCE_MSG_LEN ? "" : "...");
+	if (hdr || boot)
+		print_banks_map(banks, buf);
+
+	spin_unlock_irqrestore(&cmci_discover_lock, flags);
 }
 
 /*

Re: [PATCH] x86, mce: rework output of MCE banks ownership information

[Mike Travis]

Hi Ingo,

When running the latest kernel, I still find these in the output:


[    0.722553] Booting Node   0, Processors  #1
[    0.811625] CPU 1 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18 SHD:19 SHD:20 SHD:21
[    0.812071]  #2
[    0.907468] CPU 2 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18 SHD:19 SHD:20 SHD:21
[    0.907918]  #3
[    1.003311] CPU 3 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18 SHD:19 SHD:20 SHD:21
[    1.003750]  #4

Was there anything else needed for this patch to be accepted? 
If it's not acceptable, would simply printing the above as DEBUG
messages be acceptable?  (I'm aware you don't like printing
summaries during init.)

Thanks,
Mike

Mike Travis wrote:
> Author: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> 
> The output of MCE banks ownership information on boot tend
> to be long on new processor which has many banks:
> 
>  CPU 1 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:7 SHD:8 
> SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18 SHD:19 SHD:20 SHD:21
> 
> This message can fill up the console output when the number
> of cpus is large.
> 
> This patch suppress this info message on boot, and introduce
> debug message in shorter format instead, like:
> 
>  CPU 1 MCE banks map: ssCC PCss ssPP ssss ssss ss
> 
>  where: s: shared, C: checked by cmci, P: checked by poll.
> 
> This patch still keep the info when ownership is updated.
> E.g. if a cpu take over the ownership from hot-removed cpu,
> both message will be shown:
> 
>  CPU 1 MCE banks map updated: CMCI:6 CMCI:7 CMCI:10 CMCI:11
>  CPU 1 MCE banks map: ssCC PCCC ssPP ssCC ssss ss
> 
> v2:
>  - stop changing the level of message on update
>  - change the number of banks message on boot to debug level
> 
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
> 
>  - Modified to not use pr_cont().
> 
> Signed-off-by: Mike Travis <travis@sgi.com>
> ---
> arch/x86/kernel/cpu/mcheck/mce.c       |    6 +--
> arch/x86/kernel/cpu/mcheck/mce_intel.c |   63 
> +++++++++++++++++++++++++++------
> 2 files changed, 55 insertions(+), 14 deletions(-)
> 
> --- linux.orig/arch/x86/kernel/cpu/mcheck/mce.c
> +++ linux/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -1215,11 +1215,11 @@
> 
>     b = cap & MCG_BANKCNT_MASK;
>     if (!banks)
> -        printk(KERN_INFO "mce: CPU supports %d MCE banks\n", b);
> +        pr_debug("mce: CPU supports %d MCE banks\n", b);
> 
>     if (b > MAX_NR_BANKS) {
> -        printk(KERN_WARNING
> -               "MCE: Using only %u machine check banks out of %u\n",
> +        pr_warning(
> +            "MCE: Using only %u machine check banks out of %u\n",
>             MAX_NR_BANKS, b);
>         b = MAX_NR_BANKS;
>     }
> --- linux.orig/arch/x86/kernel/cpu/mcheck/mce_intel.c
> +++ linux/arch/x86/kernel/cpu/mcheck/mce_intel.c
> @@ -64,14 +64,50 @@
>     mce_notify_irq();
> }
> 
> -static void print_update(char *type, int *hdr, int num)
> +#define    MCE_MSG_LEN    120
> +
> +#ifdef DEBUG_KERNEL
> +static void print_banks_map(int banks, char *buf)
> +{
> +    int i, n;
> +
> +    n = snprintf(buf, MCE_MSG_LEN, "CPU %d MCE banks map:",
> +                            smp_processor_id());
> +    for (i = 0; i < banks; i++) {
> +        n += snprintf(&buf[n], MCE_MSG_LEN - n,
> +            "%s%s", (i % 4) ? "" : " ",
> +            test_bit(i, __get_cpu_var(mce_banks_owned)) ? "C" :
> +            test_bit(i, __get_cpu_var(mce_poll_banks)) ? "P" : "s");
> +    }
> +
> +    /* (indicate if message buffer overflowed) */
> +    pr_debug("%s%s\n", buf, n < MCE_MSG_LEN ? "" : "..." );
> +}
> +
> +static void print_update(char *type, int *hdr, int num, char *buf)
> +{
> +    int n = *hdr;
> +
> +    if (n == 0)
> +        n = snprintf(buf, MCE_MSG_LEN,
> +            "CPU %d MCE banks map updated:", smp_processor_id());
> +
> +    n += snprintf(&buf[n], MCE_MSG_LEN - n, " %s:%d", type, num);
> +    *hdr = n;
> +}
> +
> +#else /* !DEBUG_KERNEL */
> +
> +static inline void print_banks_map(int banks, char *buf)
> +{
> +}
> +
> +static inline void print_update(char *type, int *hdr, int num, char *buf)
> {
> -    if (*hdr == 0)
> -        printk(KERN_INFO "CPU %d MCA banks", smp_processor_id());
> -    *hdr = 1;
> -    printk(KERN_CONT " %s:%d", type, num);
> }
> 
> +#endif
> +
> /*
>  * Enable CMCI (Corrected Machine Check Interrupt) for available MCE banks
>  * on this CPU. Use the algorithm recommended in the SDM to discover shared
> @@ -83,8 +119,10 @@
>     unsigned long flags;
>     int hdr = 0;
>     int i;
> +    char buf[MCE_MSG_LEN];
> 
>     spin_lock_irqsave(&cmci_discover_lock, flags);
> +
>     for (i = 0; i < banks; i++) {
>         u64 val;
> 
> @@ -95,8 +133,8 @@
> 
>         /* Already owned by someone else? */
>         if (val & CMCI_EN) {
> -            if (test_and_clear_bit(i, owned) || boot)
> -                print_update("SHD", &hdr, i);
> +            if (test_and_clear_bit(i, owned) && !boot)
> +                print_update("SHD", &hdr, i, buf);
>             __clear_bit(i, __get_cpu_var(mce_poll_banks));
>             continue;
>         }
> @@ -107,16 +145,19 @@
> 
>         /* Did the enable bit stick? -- the bank supports CMCI */
>         if (val & CMCI_EN) {
> -            if (!test_and_set_bit(i, owned) || boot)
> -                print_update("CMCI", &hdr, i);
> +            if (!test_and_set_bit(i, owned) && !boot)
> +                print_update("CMCI", &hdr, i, buf);
>             __clear_bit(i, __get_cpu_var(mce_poll_banks));
>         } else {
>             WARN_ON(!test_bit(i, __get_cpu_var(mce_poll_banks)));
>         }
>     }
> -    spin_unlock_irqrestore(&cmci_discover_lock, flags);
>     if (hdr)
> -        printk(KERN_CONT "\n");
> +        pr_debug("%s%s\n", buf, hdr < MCE_MSG_LEN ? "" : "...");
> +    if (hdr || boot)
> +        print_banks_map(banks, buf);
> +
> +    spin_unlock_irqrestore(&cmci_discover_lock, flags);
> }
> 
> /*

Re: [PATCH] x86, mce: rework output of MCE banks ownership information

[Hidetoshi Seto]

(2009/12/15 6:46), Mike Travis wrote:
> Hi Ingo,
> 
> When running the latest kernel, I still find these in the output:
> 
> 
> [    0.722553] Booting Node   0, Processors  #1
> [    0.811625] CPU 1 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6
> SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18
> SHD:19 SHD:20 SHD:21
> [    0.812071]  #2
> [    0.907468] CPU 2 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6
> SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18
> SHD:19 SHD:20 SHD:21
> [    0.907918]  #3
> [    1.003311] CPU 3 MCA banks SHD:0 SHD:1 CMCI:2 CMCI:3 CMCI:5 SHD:6
> SHD:7 SHD:8 SHD:9 SHD:12 SHD:13 SHD:14 SHD:15 SHD:16 SHD:17 SHD:18
> SHD:19 SHD:20 SHD:21
> [    1.003750]  #4
> 
> Was there anything else needed for this patch to be accepted? If it's
> not acceptable, would simply printing the above as DEBUG
> messages be acceptable?  (I'm aware you don't like printing
> summaries during init.)
> 
> Thanks,
> Mike

I have an updated patch, so let's continue on the new thread if needed.


Thanks,
H.Seto