[PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[Mike Travis]

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

Limit number of ACPI messages of the form:

[    0.000000] ACPI: LSAPIC (acpi_id[0x00] lsapic_id[0x00] lsapic_eid[0x00] enabled)

[   99.638655] processor ACPI0007:00: registered as cooling_device0

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Myron Stowe <myron.stowe@hp.com>
Cc: Feng Tang <feng.tang@intel.com>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yhlu.kernel@gmail.com>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Mike Travis <travis@sgi.com>
---
 drivers/acpi/fan.c            |    7 ++++++-
 drivers/acpi/processor_core.c |    8 ++++++--
 drivers/acpi/tables.c         |   15 ++++++++++-----
 3 files changed, 22 insertions(+), 8 deletions(-)

--- linux.orig/drivers/acpi/fan.c
+++ linux/drivers/acpi/fan.c
@@ -243,6 +243,7 @@
 	int result = 0;
 	int state = 0;
 	struct thermal_cooling_device *cdev;
+	static int msgcnt;
 
 	if (!device)
 		return -EINVAL;
@@ -267,7 +268,11 @@
 		goto end;
 	}
 
-	dev_info(&device->dev, "registered as cooling_device%d\n", cdev->id);
+	if (msgcnt < 4 || !limit_console_output(false)) {
+		dev_info(&device->dev,
+			"registered as cooling_device%d\n", cdev->id);
+		msgcnt++;
+	}
 
 	device->driver_data = cdev;
 	result = sysfs_create_link(&device->dev.kobj,
--- linux.orig/drivers/acpi/processor_core.c
+++ linux/drivers/acpi/processor_core.c
@@ -775,6 +775,7 @@
 	struct acpi_processor *pr = NULL;
 	int result = 0;
 	struct sys_device *sysdev;
+	static int msgcnt;
 
 	pr = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
 	if (!pr)
@@ -845,8 +846,11 @@
 		goto err_power_exit;
 	}
 
-	dev_info(&device->dev, "registered as cooling_device%d\n",
-		 pr->cdev->id);
+	if (msgcnt < 4 || !limit_console_output(false)) {
+		dev_info(&device->dev, "registered as cooling_device%d\n",
+			 pr->cdev->id);
+		msgcnt++;
+	}
 
 	result = sysfs_create_link(&device->dev.kobj,
 				   &pr->cdev->device.kobj,
--- linux.orig/drivers/acpi/tables.c
+++ linux/drivers/acpi/tables.c
@@ -170,11 +170,16 @@
 	case ACPI_MADT_TYPE_LOCAL_SAPIC:
 		{
 			struct acpi_madt_local_sapic *p =
-			    (struct acpi_madt_local_sapic *)header;
-			printk(KERN_INFO PREFIX
-			       "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
-			       p->processor_id, p->id, p->eid,
-			       (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
+				(struct acpi_madt_local_sapic *)header;
+
+			if (p->eid < 8 || !limit_console_output(false))
+				printk(KERN_INFO PREFIX
+					"LSAPIC (acpi_id[0x%02x] "
+						"lsapic_id[0x%02x] "
+						"lsapic_eid[0x%02x] %s)\n",
+					p->processor_id, p->id, p->eid,
+					(p->lapic_flags & ACPI_MADT_ENABLED) ?
+						"enabled" : "disabled");
 		}
 		break;
 

-- 

Re: [PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[Bjorn Helgaas]

On Fri, 2009-10-23 at 18:37 -0500, Mike Travis wrote:
> plain text document attachment (limit_acpi)
> Limit number of ACPI messages of the form:
> 
> [    0.000000] ACPI: LSAPIC (acpi_id[0x00] lsapic_id[0x00] lsapic_eid[0x00] enabled)
> 
> [   99.638655] processor ACPI0007:00: registered as cooling_device0
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Thomas Renninger <trenn@suse.de>
> Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Myron Stowe <myron.stowe@hp.com>
> Cc: Feng Tang <feng.tang@intel.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Yinghai Lu <yhlu.kernel@gmail.com>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Mike Travis <travis@sgi.com>
> ---
>  drivers/acpi/fan.c            |    7 ++++++-
>  drivers/acpi/processor_core.c |    8 ++++++--
>  drivers/acpi/tables.c         |   15 ++++++++++-----
>  3 files changed, 22 insertions(+), 8 deletions(-)
> 
> --- linux.orig/drivers/acpi/fan.c
> +++ linux/drivers/acpi/fan.c
> @@ -243,6 +243,7 @@
>  	int result = 0;
>  	int state = 0;
>  	struct thermal_cooling_device *cdev;
> +	static int msgcnt;
>  
>  	if (!device)
>  		return -EINVAL;
> @@ -267,7 +268,11 @@
>  		goto end;
>  	}
>  
> -	dev_info(&device->dev, "registered as cooling_device%d\n", cdev->id);
> +	if (msgcnt < 4 || !limit_console_output(false)) {
> +		dev_info(&device->dev,
> +			"registered as cooling_device%d\n", cdev->id);
> +		msgcnt++;
> +	}

I'm personally not in favor of printing some, but not all, of these
messages.  That leads to questions when analyzing a dmesg log, such as
"Hmm, I see I have 64 CPUs, but only 0-3 are registered as cooling
devices.  Does that mean something is wrong?"

But I would be glad to see this particular message removed completely.

>  	device->driver_data = cdev;
>  	result = sysfs_create_link(&device->dev.kobj,
> --- linux.orig/drivers/acpi/processor_core.c
> +++ linux/drivers/acpi/processor_core.c
> @@ -775,6 +775,7 @@
>  	struct acpi_processor *pr = NULL;
>  	int result = 0;
>  	struct sys_device *sysdev;
> +	static int msgcnt;
>  
>  	pr = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
>  	if (!pr)
> @@ -845,8 +846,11 @@
>  		goto err_power_exit;
>  	}
>  
> -	dev_info(&device->dev, "registered as cooling_device%d\n",
> -		 pr->cdev->id);
> +	if (msgcnt < 4 || !limit_console_output(false)) {
> +		dev_info(&device->dev, "registered as cooling_device%d\n",
> +			 pr->cdev->id);
> +		msgcnt++;
> +	}
>  
>  	result = sysfs_create_link(&device->dev.kobj,
>  				   &pr->cdev->device.kobj,
> --- linux.orig/drivers/acpi/tables.c
> +++ linux/drivers/acpi/tables.c
> @@ -170,11 +170,16 @@
>  	case ACPI_MADT_TYPE_LOCAL_SAPIC:
>  		{
>  			struct acpi_madt_local_sapic *p =
> -			    (struct acpi_madt_local_sapic *)header;
> -			printk(KERN_INFO PREFIX
> -			       "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
> -			       p->processor_id, p->id, p->eid,
> -			       (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
> +				(struct acpi_madt_local_sapic *)header;
> +
> +			if (p->eid < 8 || !limit_console_output(false))
> +				printk(KERN_INFO PREFIX
> +					"LSAPIC (acpi_id[0x%02x] "
> +						"lsapic_id[0x%02x] "
> +						"lsapic_eid[0x%02x] %s)\n",
> +					p->processor_id, p->id, p->eid,
> +					(p->lapic_flags & ACPI_MADT_ENABLED) ?
> +						"enabled" : "disabled");

I know we print way too much stuff for every processor, but again, I'd
rather see all CPUs or none.  I think there's a little more value in
this one than the cooling device one (probably because I do a lot of
platform bringup), but it could certainly be made KERN_DEBUG and/or
combined with another processor discovery line.

Bjorn

>  		}
>  		break;
>  
> 

Re: [PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[Mike Travis]

Bjorn Helgaas wrote:
> On Fri, 2009-10-23 at 18:37 -0500, Mike Travis wrote:
>> plain text document attachment (limit_acpi)
>> Limit number of ACPI messages of the form:
>>
>> [    0.000000] ACPI: LSAPIC (acpi_id[0x00] lsapic_id[0x00] lsapic_eid[0x00] enabled)
>>
>> [   99.638655] processor ACPI0007:00: registered as cooling_device0
>>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: Thomas Renninger <trenn@suse.de>
>> Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
>> Cc: Alexey Dobriyan <adobriyan@gmail.com>
>> Cc: Myron Stowe <myron.stowe@hp.com>
>> Cc: Feng Tang <feng.tang@intel.com>
>> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
>> Cc: Yinghai Lu <yhlu.kernel@gmail.com>
>> Cc: linux-acpi@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Mike Travis <travis@sgi.com>
>> ---
>>  drivers/acpi/fan.c            |    7 ++++++-
>>  drivers/acpi/processor_core.c |    8 ++++++--
>>  drivers/acpi/tables.c         |   15 ++++++++++-----
>>  3 files changed, 22 insertions(+), 8 deletions(-)
>>
>> --- linux.orig/drivers/acpi/fan.c
>> +++ linux/drivers/acpi/fan.c
>> @@ -243,6 +243,7 @@
>>  	int result = 0;
>>  	int state = 0;
>>  	struct thermal_cooling_device *cdev;
>> +	static int msgcnt;
>>  
>>  	if (!device)
>>  		return -EINVAL;
>> @@ -267,7 +268,11 @@
>>  		goto end;
>>  	}
>>  
>> -	dev_info(&device->dev, "registered as cooling_device%d\n", cdev->id);
>> +	if (msgcnt < 4 || !limit_console_output(false)) {
>> +		dev_info(&device->dev,
>> +			"registered as cooling_device%d\n", cdev->id);
>> +		msgcnt++;
>> +	}
> 
> I'm personally not in favor of printing some, but not all, of these
> messages.  That leads to questions when analyzing a dmesg log, such as
> "Hmm, I see I have 64 CPUs, but only 0-3 are registered as cooling
> devices.  Does that mean something is wrong?"
> 
> But I would be glad to see this particular message removed completely.

I didn't want to make the decision to remove messages as the original
authors might have very good reasons for including them.

Note that the dmesg log (kernel log buffer) still does have every one of
the messages, only the prints to the console output (which usually is a
serial connection [or IPMI] on servers) are limited.

> 
>>  	device->driver_data = cdev;
>>  	result = sysfs_create_link(&device->dev.kobj,
>> --- linux.orig/drivers/acpi/processor_core.c
>> +++ linux/drivers/acpi/processor_core.c
>> @@ -775,6 +775,7 @@
>>  	struct acpi_processor *pr = NULL;
>>  	int result = 0;
>>  	struct sys_device *sysdev;
>> +	static int msgcnt;
>>  
>>  	pr = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
>>  	if (!pr)
>> @@ -845,8 +846,11 @@
>>  		goto err_power_exit;
>>  	}
>>  
>> -	dev_info(&device->dev, "registered as cooling_device%d\n",
>> -		 pr->cdev->id);
>> +	if (msgcnt < 4 || !limit_console_output(false)) {
>> +		dev_info(&device->dev, "registered as cooling_device%d\n",
>> +			 pr->cdev->id);
>> +		msgcnt++;
>> +	}
>>  
>>  	result = sysfs_create_link(&device->dev.kobj,
>>  				   &pr->cdev->device.kobj,
>> --- linux.orig/drivers/acpi/tables.c
>> +++ linux/drivers/acpi/tables.c
>> @@ -170,11 +170,16 @@
>>  	case ACPI_MADT_TYPE_LOCAL_SAPIC:
>>  		{
>>  			struct acpi_madt_local_sapic *p =
>> -			    (struct acpi_madt_local_sapic *)header;
>> -			printk(KERN_INFO PREFIX
>> -			       "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
>> -			       p->processor_id, p->id, p->eid,
>> -			       (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
>> +				(struct acpi_madt_local_sapic *)header;
>> +
>> +			if (p->eid < 8 || !limit_console_output(false))
>> +				printk(KERN_INFO PREFIX
>> +					"LSAPIC (acpi_id[0x%02x] "
>> +						"lsapic_id[0x%02x] "
>> +						"lsapic_eid[0x%02x] %s)\n",
>> +					p->processor_id, p->id, p->eid,
>> +					(p->lapic_flags & ACPI_MADT_ENABLED) ?
>> +						"enabled" : "disabled");
> 
> I know we print way too much stuff for every processor, but again, I'd
> rather see all CPUs or none.  I think there's a little more value in
> this one than the cooling device one (probably because I do a lot of
> platform bringup), but it could certainly be made KERN_DEBUG and/or
> combined with another processor discovery line.

This was the major reason why I left the default as it currently is, and
made it a startup option that a site can choose to use or not.

The intent of printing a few messages was to give context to the last line
in this sequence:

[   99.638655] processor ACPI0007:00: registered as cooling_device0
[   99.648277] processor ACPI0007:01: registered as cooling_device1
[   99.657976] processor ACPI0007:02: registered as cooling_device2
[   99.667229] processor ACPI0007:03: registered as cooling_device3
[   99.676517] printk: further related messages suppressed

Thanks,
Mike

> 
> Bjorn
> 
>>  		}
>>  		break;
>>  
>>

Re: [PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[Thomas Renninger]

On Saturday 24 October 2009 05:29:47 am Bjorn Helgaas wrote:
> On Fri, 2009-10-23 at 18:37 -0500, Mike Travis wrote:
> > plain text document attachment (limit_acpi)
> > Limit number of ACPI messages of the form:
> >
> > [    0.000000] ACPI: LSAPIC (acpi_id[0x00] lsapic_id[0x00]
> > lsapic_eid[0x00] enabled)
> >
> > [   99.638655] processor ACPI0007:00: registered as cooling_device0
> >
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: Thomas Renninger <trenn@suse.de>
> > Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > Cc: Myron Stowe <myron.stowe@hp.com>
> > Cc: Feng Tang <feng.tang@intel.com>
> > Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: Yinghai Lu <yhlu.kernel@gmail.com>
> > Cc: linux-acpi@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Mike Travis <travis@sgi.com>
> > ---
> >  drivers/acpi/fan.c            |    7 ++++++-
> >  drivers/acpi/processor_core.c |    8 ++++++--
> >  drivers/acpi/tables.c         |   15 ++++++++++-----
> >  3 files changed, 22 insertions(+), 8 deletions(-)
> >
> > --- linux.orig/drivers/acpi/fan.c
> > +++ linux/drivers/acpi/fan.c
> > @@ -243,6 +243,7 @@
> >  	int result = 0;
> >  	int state = 0;
> >  	struct thermal_cooling_device *cdev;
> > +	static int msgcnt;
> >
> >  	if (!device)
> >  		return -EINVAL;
> > @@ -267,7 +268,11 @@
> >  		goto end;
> >  	}
> >
> > -	dev_info(&device->dev, "registered as cooling_device%d\n", cdev->id);
> > +	if (msgcnt < 4 || !limit_console_output(false)) {
> > +		dev_info(&device->dev,
> > +			"registered as cooling_device%d\n", cdev->id);
> > +		msgcnt++;
> > +	}
>
> I'm personally not in favor of printing some, but not all, of these
> messages.  That leads to questions when analyzing a dmesg log, such as
> "Hmm, I see I have 64 CPUs, but only 0-3 are registered as cooling
> devices.  Does that mean something is wrong?"
>
> But I would be glad to see this particular message removed completely.
>
> >  	device->driver_data = cdev;
> >  	result = sysfs_create_link(&device->dev.kobj,
> > --- linux.orig/drivers/acpi/processor_core.c
> > +++ linux/drivers/acpi/processor_core.c
> > @@ -775,6 +775,7 @@
> >  	struct acpi_processor *pr = NULL;
> >  	int result = 0;
> >  	struct sys_device *sysdev;
> > +	static int msgcnt;
> >
> >  	pr = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
> >  	if (!pr)
> > @@ -845,8 +846,11 @@
> >  		goto err_power_exit;
> >  	}
> >
> > -	dev_info(&device->dev, "registered as cooling_device%d\n",
> > -		 pr->cdev->id);
> > +	if (msgcnt < 4 || !limit_console_output(false)) {
> > +		dev_info(&device->dev, "registered as cooling_device%d\n",
> > +			 pr->cdev->id);
> > +		msgcnt++;
> > +	}
If Zhang Rui does not complain you can change these:
..registered as cooling_device..
into dev_dbg() without any condition.
This isn't critical.

Or why not use the more fine grained
ACPI debug facility and change it into:
ACPI_DEBUG_PRINT((ACPI_DB_INFO "..."));
(compare with Documentation/acpi/debug.txt and other
occurences in the same file)
You have to pass:
acpi_dbg_layer=0x20000000
to see it then.
> >
> >  	result = sysfs_create_link(&device->dev.kobj,
> >  				   &pr->cdev->device.kobj,
> > --- linux.orig/drivers/acpi/tables.c
> > +++ linux/drivers/acpi/tables.c
> > @@ -170,11 +170,16 @@
> >  	case ACPI_MADT_TYPE_LOCAL_SAPIC:
> >  		{
> >  			struct acpi_madt_local_sapic *p =
> > -			    (struct acpi_madt_local_sapic *)header;
> > -			printk(KERN_INFO PREFIX
> > -			       "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x]
> > %s)\n", -			       p->processor_id, p->id, p->eid,
> > -			       (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" :
> > "disabled"); +				(struct acpi_madt_local_sapic *)header;
> > +
> > +			if (p->eid < 8 || !limit_console_output(false))
I can't find limit_console_output(), I expect it got introduced by another one 
of your patch series, not send to the acpi list?
Still shouldn't this be:
limit_console_output(true)
instead of:
!limit_console_output(false)

       Thomas

> > +				printk(KERN_INFO PREFIX
> > +					"LSAPIC (acpi_id[0x%02x] "
> > +						"lsapic_id[0x%02x] "
> > +						"lsapic_eid[0x%02x] %s)\n",
> > +					p->processor_id, p->id, p->eid,
> > +					(p->lapic_flags & ACPI_MADT_ENABLED) ?
> > +						"enabled" : "disabled");
>
> I know we print way too much stuff for every processor, but again, I'd
> rather see all CPUs or none.  I think there's a little more value in
> this one than the cooling device one (probably because I do a lot of
> platform bringup), but it could certainly be made KERN_DEBUG and/or
> combined with another processor discovery line.

Re: [PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[Mike Travis]

Thomas Renninger wrote:
> On Saturday 24 October 2009 05:29:47 am Bjorn Helgaas wrote:
>> On Fri, 2009-10-23 at 18:37 -0500, Mike Travis wrote:
>>> plain text document attachment (limit_acpi)
>>> Limit number of ACPI messages of the form:
>>>
>>> [    0.000000] ACPI: LSAPIC (acpi_id[0x00] lsapic_id[0x00]
>>> lsapic_eid[0x00] enabled)
>>>
>>> [   99.638655] processor ACPI0007:00: registered as cooling_device0
>>>
>>> Cc: Zhang Rui <rui.zhang@intel.com>
>>> Cc: Len Brown <lenb@kernel.org>
>>> Cc: Thomas Renninger <trenn@suse.de>
>>> Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
>>> Cc: Alexey Dobriyan <adobriyan@gmail.com>
>>> Cc: Myron Stowe <myron.stowe@hp.com>
>>> Cc: Feng Tang <feng.tang@intel.com>
>>> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
>>> Cc: Yinghai Lu <yhlu.kernel@gmail.com>
>>> Cc: linux-acpi@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Mike Travis <travis@sgi.com>
>>> ---
>>>  drivers/acpi/fan.c            |    7 ++++++-
>>>  drivers/acpi/processor_core.c |    8 ++++++--
>>>  drivers/acpi/tables.c         |   15 ++++++++++-----
>>>  3 files changed, 22 insertions(+), 8 deletions(-)
>>>
>>> --- linux.orig/drivers/acpi/fan.c
>>> +++ linux/drivers/acpi/fan.c
>>> @@ -243,6 +243,7 @@
>>>  	int result = 0;
>>>  	int state = 0;
>>>  	struct thermal_cooling_device *cdev;
>>> +	static int msgcnt;
>>>
>>>  	if (!device)
>>>  		return -EINVAL;
>>> @@ -267,7 +268,11 @@
>>>  		goto end;
>>>  	}
>>>
>>> -	dev_info(&device->dev, "registered as cooling_device%d\n", cdev->id);
>>> +	if (msgcnt < 4 || !limit_console_output(false)) {
>>> +		dev_info(&device->dev,
>>> +			"registered as cooling_device%d\n", cdev->id);
>>> +		msgcnt++;
>>> +	}
>> I'm personally not in favor of printing some, but not all, of these
>> messages.  That leads to questions when analyzing a dmesg log, such as
>> "Hmm, I see I have 64 CPUs, but only 0-3 are registered as cooling
>> devices.  Does that mean something is wrong?"
>>
>> But I would be glad to see this particular message removed completely.
>>
>>>  	device->driver_data = cdev;
>>>  	result = sysfs_create_link(&device->dev.kobj,
>>> --- linux.orig/drivers/acpi/processor_core.c
>>> +++ linux/drivers/acpi/processor_core.c
>>> @@ -775,6 +775,7 @@
>>>  	struct acpi_processor *pr = NULL;
>>>  	int result = 0;
>>>  	struct sys_device *sysdev;
>>> +	static int msgcnt;
>>>
>>>  	pr = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
>>>  	if (!pr)
>>> @@ -845,8 +846,11 @@
>>>  		goto err_power_exit;
>>>  	}
>>>
>>> -	dev_info(&device->dev, "registered as cooling_device%d\n",
>>> -		 pr->cdev->id);
>>> +	if (msgcnt < 4 || !limit_console_output(false)) {
>>> +		dev_info(&device->dev, "registered as cooling_device%d\n",
>>> +			 pr->cdev->id);
>>> +		msgcnt++;
>>> +	}
> If Zhang Rui does not complain you can change these:
> ..registered as cooling_device..
> into dev_dbg() without any condition.
> This isn't critical.
> 
> Or why not use the more fine grained
> ACPI debug facility and change it into:
> ACPI_DEBUG_PRINT((ACPI_DB_INFO "..."));
> (compare with Documentation/acpi/debug.txt and other
> occurences in the same file)
> You have to pass:
> acpi_dbg_layer=0x20000000
> to see it then.

Ok.
>>>  	result = sysfs_create_link(&device->dev.kobj,
>>>  				   &pr->cdev->device.kobj,
>>> --- linux.orig/drivers/acpi/tables.c
>>> +++ linux/drivers/acpi/tables.c
>>> @@ -170,11 +170,16 @@
>>>  	case ACPI_MADT_TYPE_LOCAL_SAPIC:
>>>  		{
>>>  			struct acpi_madt_local_sapic *p =
>>> -			    (struct acpi_madt_local_sapic *)header;
>>> -			printk(KERN_INFO PREFIX
>>> -			       "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x]
>>> %s)\n", -			       p->processor_id, p->id, p->eid,
>>> -			       (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" :
>>> "disabled"); +				(struct acpi_madt_local_sapic *)header;
>>> +
>>> +			if (p->eid < 8 || !limit_console_output(false))
> I can't find limit_console_output(), I expect it got introduced by another one 
> of your patch series, not send to the acpi list?
> Still shouldn't this be:
> limit_console_output(true)
> instead of:
> !limit_console_output(false)
> 
>        Thomas

Sorry, I used a semi-auto method of calling get_maintainer which filled each patch
with specific Cc's.  I did send the first one to everyone in hopes that that would
help find the others.

See http://marc.info/?l=linux-kernel&m=125634109621411&w=4   (the argument specifies
whether to reduce the console loglevel.  It's currently only used to suppress the
cpu bootup messages.)

Thanks,
Mike

> 
>>> +				printk(KERN_INFO PREFIX
>>> +					"LSAPIC (acpi_id[0x%02x] "
>>> +						"lsapic_id[0x%02x] "
>>> +						"lsapic_eid[0x%02x] %s)\n",
>>> +					p->processor_id, p->id, p->eid,
>>> +					(p->lapic_flags & ACPI_MADT_ENABLED) ?
>>> +						"enabled" : "disabled");
>> I know we print way too much stuff for every processor, but again, I'd
>> rather see all CPUs or none.  I think there's a little more value in
>> this one than the cooling device one (probably because I do a lot of
>> platform bringup), but it could certainly be made KERN_DEBUG and/or
>> combined with another processor discovery line.

Re: [PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[Mike Travis]

Bjorn Helgaas wrote:
...
> 
> I know we print way too much stuff for every processor, but again, I'd
> rather see all CPUs or none.  I think there's a little more value in
> this one than the cooling device one (probably because I do a lot of
> platform bringup), but it could certainly be made KERN_DEBUG and/or
> combined with another processor discovery line.

Is this more acceptable?

Thanks,
Mike
--- 

SGI x86_64 UV: Limit the number of ACPI messages

Limit number of ACPI messages of the form:

[    0.000000] ACPI: LSAPIC (acpi_id[0x00] lsapic_id[0x00] lsapic_eid[0x00] enabled)

[   99.638655] processor ACPI0007:00: registered as cooling_device0

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Myron Stowe <myron.stowe@hp.com>
Cc: Feng Tang <feng.tang@intel.com>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yhlu.kernel@gmail.com>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Mike Travis <travis@sgi.com>
---
 drivers/acpi/fan.c            |    2 +-
 drivers/acpi/processor_core.c |    3 +--
 drivers/acpi/tables.c         |   13 ++++++++-----
 3 files changed, 10 insertions(+), 8 deletions(-)

--- linux.orig/drivers/acpi/fan.c
+++ linux/drivers/acpi/fan.c
@@ -267,7 +267,7 @@
 		goto end;
 	}
 
-	dev_info(&device->dev, "registered as cooling_device%d\n", cdev->id);
+	dev_dbg(&device->dev, "registered as cooling_device%d\n", cdev->id);
 
 	device->driver_data = cdev;
 	result = sysfs_create_link(&device->dev.kobj,
--- linux.orig/drivers/acpi/processor_core.c
+++ linux/drivers/acpi/processor_core.c
@@ -845,8 +845,7 @@
 		goto err_power_exit;
 	}
 
-	dev_info(&device->dev, "registered as cooling_device%d\n",
-		 pr->cdev->id);
+	dev_dbg(&device->dev, "registered as cooling_device%d\n", pr->cdev->id);
 
 	result = sysfs_create_link(&device->dev.kobj,
 				   &pr->cdev->device.kobj,
--- linux.orig/drivers/acpi/tables.c
+++ linux/drivers/acpi/tables.c
@@ -170,11 +170,14 @@
 	case ACPI_MADT_TYPE_LOCAL_SAPIC:
 		{
 			struct acpi_madt_local_sapic *p =
-			    (struct acpi_madt_local_sapic *)header;
-			printk(KERN_INFO PREFIX
-			       "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
-			       p->processor_id, p->id, p->eid,
-			       (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
+				(struct acpi_madt_local_sapic *)header;
+
+			printk(KERN_DEBUG PREFIX
+				"LSAPIC (acpi_id[0x%02x] "
+				"lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
+				p->processor_id, p->id, p->eid,
+				(p->lapic_flags & ACPI_MADT_ENABLED) ?
+					"enabled" : "disabled");
 		}
 		break;
 

Re: [PATCH 4/8] SGI x86_64 UV: Limit the number of ACPI messages

[Bjorn Helgaas]

On Tuesday 27 October 2009 09:27:35 am Mike Travis wrote:
> Bjorn Helgaas wrote:
> ...
> > 
> > I know we print way too much stuff for every processor, but again, I'd
> > rather see all CPUs or none.  I think there's a little more value in
> > this one than the cooling device one (probably because I do a lot of
> > platform bringup), but it could certainly be made KERN_DEBUG and/or
> > combined with another processor discovery line.
> 
> Is this more acceptable?
> 
> Thanks,
> Mike
> --- 
> 
> SGI x86_64 UV: Limit the number of ACPI messages
> 
> Limit number of ACPI messages of the form:
> 
> [    0.000000] ACPI: LSAPIC (acpi_id[0x00] lsapic_id[0x00] lsapic_eid[0x00] enabled)
> 
> [   99.638655] processor ACPI0007:00: registered as cooling_device0
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Thomas Renninger <trenn@suse.de>
> Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Myron Stowe <myron.stowe@hp.com>
> Cc: Feng Tang <feng.tang@intel.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Yinghai Lu <yhlu.kernel@gmail.com>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Mike Travis <travis@sgi.com>
> ---
>  drivers/acpi/fan.c            |    2 +-
>  drivers/acpi/processor_core.c |    3 +--
>  drivers/acpi/tables.c         |   13 ++++++++-----
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> --- linux.orig/drivers/acpi/fan.c
> +++ linux/drivers/acpi/fan.c
> @@ -267,7 +267,7 @@
>  		goto end;
>  	}
>  
> -	dev_info(&device->dev, "registered as cooling_device%d\n", cdev->id);
> +	dev_dbg(&device->dev, "registered as cooling_device%d\n", cdev->id);
>  
>  	device->driver_data = cdev;
>  	result = sysfs_create_link(&device->dev.kobj,
> --- linux.orig/drivers/acpi/processor_core.c
> +++ linux/drivers/acpi/processor_core.c
> @@ -845,8 +845,7 @@
>  		goto err_power_exit;
>  	}
>  
> -	dev_info(&device->dev, "registered as cooling_device%d\n",
> -		 pr->cdev->id);
> +	dev_dbg(&device->dev, "registered as cooling_device%d\n", pr->cdev->id);

I still think you should just remove these messages completely.

If you do keep them, note that dev_dbg() is not the same as
dev_prinkt(KERN_DEBUG) -- dev_dbg() compiles to nothing at all
unless "DEBUG" is defined.

>  	result = sysfs_create_link(&device->dev.kobj,
>  				   &pr->cdev->device.kobj,
> --- linux.orig/drivers/acpi/tables.c
> +++ linux/drivers/acpi/tables.c
> @@ -170,11 +170,14 @@
>  	case ACPI_MADT_TYPE_LOCAL_SAPIC:
>  		{
>  			struct acpi_madt_local_sapic *p =
> -			    (struct acpi_madt_local_sapic *)header;
> -			printk(KERN_INFO PREFIX
> -			       "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
> -			       p->processor_id, p->id, p->eid,
> -			       (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
> +				(struct acpi_madt_local_sapic *)header;
> +
> +			printk(KERN_DEBUG PREFIX
> +				"LSAPIC (acpi_id[0x%02x] "
> +				"lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
> +				p->processor_id, p->id, p->eid,
> +				(p->lapic_flags & ACPI_MADT_ENABLED) ?
> +					"enabled" : "disabled");

I don't object to this.

I do think it'd be much better to combine this with the other
per-processor startup messages, of which we have an absolute over-
abundance:

	CPU0: Intel(R) Xeon(TM) CPU 2.80GHz stepping 09
	Booting processor 1 APIC 0x6 ip 0x6000
	Initializing CPU#1
	Calibrating delay using timer specific routine.. 5597.14 BogoMIPS (lpj=2798571)
	CPU: Trace cache: 12K uops, L1 D cache: 8K
	CPU: L2 cache: 512K
	CPU: Physical Processor ID: 3
	CPU: Processor Core ID: 0

But that's a bigger project.

Bjorn

Re: [PATCH 3/8] SGI x86_64 UV: Limit the number of number of SRAT messages

[David Rientjes]

On Tue, 27 Oct 2009, Mike Travis wrote:

> --- linux.orig/arch/x86/mm/srat_64.c
> +++ linux/arch/x86/mm/srat_64.c
> @@ -115,6 +115,7 @@
> {
> 	int pxm, node;
> 	int apic_id;
> +	static int last_node = -1;
> 
> 	if (srat_disabled())
> 		return;
> @@ -136,8 +137,11 @@
> 	apicid_to_node[apic_id] = node;
> 	node_set(node, cpu_nodes_parsed);
> 	acpi_numa = 1;
> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> -	       pxm, apic_id, node);
> +	if (node > last_node) {
> +		printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> +		       pxm, apic_id, node);
> +		last_node = node;
> +	}
> }
> 
> /* Callback for Proximity Domain -> LAPIC mapping */
> @@ -146,6 +150,7 @@
> {
> 	int pxm, node;
> 	int apic_id;
> +	static int last_node = -1;
> 
> 	if (srat_disabled())
> 		return;
> @@ -170,8 +175,11 @@
> 	apicid_to_node[apic_id] = node;
> 	node_set(node, cpu_nodes_parsed);
> 	acpi_numa = 1;
> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> -	       pxm, apic_id, node);
> +	if (node > last_node) {
> +		printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> +		       pxm, apic_id, node);
> +		last_node = node;
> +	}
> }
> 
> #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> 

So on my Opteron I'll be getting this:

	SRAT: PXM 0 -> APIC 0 -> Node 0
	SRAT: PXM 1 -> APIC 2 -> Node 1

instead of this:

	SRAT: PXM 0 -> APIC 0 -> Node 0
	SRAT: PXM 0 -> APIC 1 -> Node 0
	SRAT: PXM 1 -> APIC 2 -> Node 1
	SRAT: PXM 1 -> APIC 3 -> Node 1

Do I need to infer what apic 1 or 3 map to with your patch (or whether 
they are even valid)?

It would seem much better to print this information once the SRAT parsing 
in acpi_numa_init() is complete and apicid_to_node[] is populated.  This 
leads to the ideal beheavior, which is:

	SRAT: PXM 0 -> APIC {0-1} -> Node 0
	SRAT: PXM 1 -> APIC {2-3} -> Node 1

Something like the following patch?  (Regardless, we need to cc 
linux-acpi@vger.kernel.org.  I've added it.)



x86: reduce srat verbosity in the kernel log

It's possible to reduce the number of SRAT messages emitted to the kernel
log by printing each valid pxm once and then creating bitmaps to represent
the apicids that map to the same node.

This reduces lines such as

	SRAT: PXM 0 -> APIC 0 -> Node 0
	SRAT: PXM 0 -> APIC 1 -> Node 0
	SRAT: PXM 1 -> APIC 2 -> Node 1
	SRAT: PXM 1 -> APIC 3 -> Node 1

to

	SRAT: PXM 0 -> APIC {0-1} -> Node 0
	SRAT: PXM 1 -> APIC {2-3} -> Node 1

Signed-off-by: David Rientjes <rientjes@google.com>
---
 arch/x86/mm/srat_64.c |   31 +++++++++++++++++++++++++++----
 drivers/acpi/numa.c   |    5 +++++
 include/linux/acpi.h  |    3 ++-
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
--- a/arch/x86/mm/srat_64.c
+++ b/arch/x86/mm/srat_64.c
@@ -136,8 +136,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 	apicid_to_node[apic_id] = node;
 	node_set(node, cpu_nodes_parsed);
 	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
-	       pxm, apic_id, node);
 }
 
 /* Callback for Proximity Domain -> LAPIC mapping */
@@ -170,8 +168,33 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
 	apicid_to_node[apic_id] = node;
 	node_set(node, cpu_nodes_parsed);
 	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
-	       pxm, apic_id, node);
+}
+
+void __init acpi_numa_print_srat_mapping(void)
+{
+	DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC);
+	char apicid_list[MAX_LOCAL_APIC];
+	int i, j;
+
+	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
+		int nid;
+
+		nid = pxm_to_node(i);
+		if (nid == NUMA_NO_NODE)
+			continue;
+
+		bitmap_zero(apicid_map, MAX_LOCAL_APIC);
+		for (j = 0; j < MAX_LOCAL_APIC; j++)
+			if (apicid_to_node[j] == nid)
+				set_bit(j, apicid_map);
+
+		if (bitmap_empty(apicid_map, MAX_LOCAL_APIC))
+			continue;
+		bitmap_scnlistprintf(apicid_list, MAX_LOCAL_APIC,
+				     apicid_map, MAX_LOCAL_APIC);
+		pr_info("SRAT: PXM %u -> APIC {%s} -> Node %u\n",
+			i, apicid_list, nid);
+	}
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -281,6 +281,10 @@ acpi_table_parse_srat(enum acpi_srat_type id,
 					    handler, max_entries);
 }
 
+void __init __attribute__((weak)) acpi_numa_print_srat_mapping(void)
+{
+}
+
 int __init acpi_numa_init(void)
 {
 	/* SRAT: Static Resource Affinity Table */
@@ -292,6 +296,7 @@ int __init acpi_numa_init(void)
 		acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 				      acpi_parse_memory_affinity,
 				      NR_NODE_MEMBLKS);
+		acpi_numa_print_srat_mapping();
 	}
 
 	/* SLIT: System Locality Information Table */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -92,12 +92,13 @@ int acpi_table_parse_madt (enum acpi_madt_type id, acpi_table_entry_handler hand
 int acpi_parse_mcfg (struct acpi_table_header *header);
 void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
 
-/* the following four functions are architecture-dependent */
+/* the following six functions are architecture-dependent */
 void acpi_numa_slit_init (struct acpi_table_slit *slit);
 void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
 void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
 void acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
 void acpi_numa_arch_fixup(void);
+void acpi_numa_print_srat_mapping(void);
 
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */

Re: [PATCH 3/8] SGI x86_64 UV: Limit the number of number of SRAT messages

[Mike Travis]

Hi David,

Very Cool, I'll try it out and let you know how it goes.

Note that it would be better to declare the BITMAP in the
static initdata section so it doesn't grow the stack by 4k
bytes.  (And it's thrown away after the kernel starts.)

Thanks,
Mike


David Rientjes wrote:
> On Tue, 27 Oct 2009, Mike Travis wrote:
> 
>> --- linux.orig/arch/x86/mm/srat_64.c
>> +++ linux/arch/x86/mm/srat_64.c
>> @@ -115,6 +115,7 @@
>> {
>> 	int pxm, node;
>> 	int apic_id;
>> +	static int last_node = -1;
>>
>> 	if (srat_disabled())
>> 		return;
>> @@ -136,8 +137,11 @@
>> 	apicid_to_node[apic_id] = node;
>> 	node_set(node, cpu_nodes_parsed);
>> 	acpi_numa = 1;
>> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
>> -	       pxm, apic_id, node);
>> +	if (node > last_node) {
>> +		printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
>> +		       pxm, apic_id, node);
>> +		last_node = node;
>> +	}
>> }
>>
>> /* Callback for Proximity Domain -> LAPIC mapping */
>> @@ -146,6 +150,7 @@
>> {
>> 	int pxm, node;
>> 	int apic_id;
>> +	static int last_node = -1;
>>
>> 	if (srat_disabled())
>> 		return;
>> @@ -170,8 +175,11 @@
>> 	apicid_to_node[apic_id] = node;
>> 	node_set(node, cpu_nodes_parsed);
>> 	acpi_numa = 1;
>> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
>> -	       pxm, apic_id, node);
>> +	if (node > last_node) {
>> +		printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
>> +		       pxm, apic_id, node);
>> +		last_node = node;
>> +	}
>> }
>>
>> #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
>>
> 
> So on my Opteron I'll be getting this:
> 
> 	SRAT: PXM 0 -> APIC 0 -> Node 0
> 	SRAT: PXM 1 -> APIC 2 -> Node 1
> 
> instead of this:
> 
> 	SRAT: PXM 0 -> APIC 0 -> Node 0
> 	SRAT: PXM 0 -> APIC 1 -> Node 0
> 	SRAT: PXM 1 -> APIC 2 -> Node 1
> 	SRAT: PXM 1 -> APIC 3 -> Node 1
> 
> Do I need to infer what apic 1 or 3 map to with your patch (or whether 
> they are even valid)?
> 
> It would seem much better to print this information once the SRAT parsing 
> in acpi_numa_init() is complete and apicid_to_node[] is populated.  This 
> leads to the ideal beheavior, which is:
> 
> 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
> 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
> 
> Something like the following patch?  (Regardless, we need to cc 
> linux-acpi@vger.kernel.org.  I've added it.)
> 
> 
> 
> x86: reduce srat verbosity in the kernel log
> 
> It's possible to reduce the number of SRAT messages emitted to the kernel
> log by printing each valid pxm once and then creating bitmaps to represent
> the apicids that map to the same node.
> 
> This reduces lines such as
> 
> 	SRAT: PXM 0 -> APIC 0 -> Node 0
> 	SRAT: PXM 0 -> APIC 1 -> Node 0
> 	SRAT: PXM 1 -> APIC 2 -> Node 1
> 	SRAT: PXM 1 -> APIC 3 -> Node 1
> 
> to
> 
> 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
> 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
> 
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  arch/x86/mm/srat_64.c |   31 +++++++++++++++++++++++++++----
>  drivers/acpi/numa.c   |    5 +++++
>  include/linux/acpi.h  |    3 ++-
>  3 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
> --- a/arch/x86/mm/srat_64.c
> +++ b/arch/x86/mm/srat_64.c
> @@ -136,8 +136,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
>  	apicid_to_node[apic_id] = node;
>  	node_set(node, cpu_nodes_parsed);
>  	acpi_numa = 1;
> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> -	       pxm, apic_id, node);
>  }
>  
>  /* Callback for Proximity Domain -> LAPIC mapping */
> @@ -170,8 +168,33 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>  	apicid_to_node[apic_id] = node;
>  	node_set(node, cpu_nodes_parsed);
>  	acpi_numa = 1;
> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> -	       pxm, apic_id, node);
> +}
> +
> +void __init acpi_numa_print_srat_mapping(void)
> +{
> +	DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC);
> +	char apicid_list[MAX_LOCAL_APIC];
> +	int i, j;
> +
> +	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
> +		int nid;
> +
> +		nid = pxm_to_node(i);
> +		if (nid == NUMA_NO_NODE)
> +			continue;
> +
> +		bitmap_zero(apicid_map, MAX_LOCAL_APIC);
> +		for (j = 0; j < MAX_LOCAL_APIC; j++)
> +			if (apicid_to_node[j] == nid)
> +				set_bit(j, apicid_map);
> +
> +		if (bitmap_empty(apicid_map, MAX_LOCAL_APIC))
> +			continue;
> +		bitmap_scnlistprintf(apicid_list, MAX_LOCAL_APIC,
> +				     apicid_map, MAX_LOCAL_APIC);
> +		pr_info("SRAT: PXM %u -> APIC {%s} -> Node %u\n",
> +			i, apicid_list, nid);
> +	}
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -281,6 +281,10 @@ acpi_table_parse_srat(enum acpi_srat_type id,
>  					    handler, max_entries);
>  }
>  
> +void __init __attribute__((weak)) acpi_numa_print_srat_mapping(void)
> +{
> +}
> +
>  int __init acpi_numa_init(void)
>  {
>  	/* SRAT: Static Resource Affinity Table */
> @@ -292,6 +296,7 @@ int __init acpi_numa_init(void)
>  		acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
>  				      acpi_parse_memory_affinity,
>  				      NR_NODE_MEMBLKS);
> +		acpi_numa_print_srat_mapping();
>  	}
>  
>  	/* SLIT: System Locality Information Table */
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -92,12 +92,13 @@ int acpi_table_parse_madt (enum acpi_madt_type id, acpi_table_entry_handler hand
>  int acpi_parse_mcfg (struct acpi_table_header *header);
>  void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
>  
> -/* the following four functions are architecture-dependent */
> +/* the following six functions are architecture-dependent */
>  void acpi_numa_slit_init (struct acpi_table_slit *slit);
>  void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
>  void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
>  void acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
>  void acpi_numa_arch_fixup(void);
> +void acpi_numa_print_srat_mapping(void);
>  
>  #ifdef CONFIG_ACPI_HOTPLUG_CPU
>  /* Arch dependent functions for cpu hotplug support */

[patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Tue, 27 Oct 2009, Mike Travis wrote:

> Hi David,
> 
> Very Cool, I'll try it out and let you know how it goes.
> 
> Note that it would be better to declare the BITMAP in the
> static initdata section so it doesn't grow the stack by 4k
> bytes.  (And it's thrown away after the kernel starts.)
> 

Right, here's an updated version.  I was thinking of MAX_PXM_DOMAINS being 
256 instead of MAX_LOCAL_APIC :)

Here's an updated version.  apicid_map and apicid_list don't need to be 
synchronized because there're no concurrency issues here on init.



x86: reduce srat verbosity in the kernel log

It's possible to reduce the number of SRAT messages emitted to the kernel
log by printing each valid pxm once and then creating bitmaps to represent
the apicids that map to the same node.

This reduces lines such as

	SRAT: PXM 0 -> APIC 0 -> Node 0
	SRAT: PXM 0 -> APIC 1 -> Node 0
	SRAT: PXM 1 -> APIC 2 -> Node 1
	SRAT: PXM 1 -> APIC 3 -> Node 1

to

	SRAT: PXM 0 -> APIC {0-1} -> Node 0
	SRAT: PXM 1 -> APIC {2-3} -> Node 1

Signed-off-by: David Rientjes <rientjes@google.com>
---
 arch/x86/mm/srat_64.c |   32 ++++++++++++++++++++++++++++----
 drivers/acpi/numa.c   |    5 +++++
 include/linux/acpi.h  |    3 ++-
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
--- a/arch/x86/mm/srat_64.c
+++ b/arch/x86/mm/srat_64.c
@@ -36,6 +36,9 @@ static int num_node_memblks __initdata;
 static struct bootnode node_memblk_range[NR_NODE_MEMBLKS] __initdata;
 static int memblk_nodeid[NR_NODE_MEMBLKS] __initdata;
 
+static DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC) __initdata;
+static char apicid_list[MAX_LOCAL_APIC] __initdata;
+
 static __init int setup_node(int pxm)
 {
 	return acpi_map_pxm_to_node(pxm);
@@ -136,8 +139,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 	apicid_to_node[apic_id] = node;
 	node_set(node, cpu_nodes_parsed);
 	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
-	       pxm, apic_id, node);
 }
 
 /* Callback for Proximity Domain -> LAPIC mapping */
@@ -170,8 +171,31 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
 	apicid_to_node[apic_id] = node;
 	node_set(node, cpu_nodes_parsed);
 	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
-	       pxm, apic_id, node);
+}
+
+void __init acpi_numa_print_srat_mapping(void)
+{
+	int i, j;
+
+	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
+		int nid;
+
+		nid = pxm_to_node(i);
+		if (nid == NUMA_NO_NODE)
+			continue;
+
+		bitmap_zero(apicid_map, MAX_LOCAL_APIC);
+		for (j = 0; j < MAX_LOCAL_APIC; j++)
+			if (apicid_to_node[j] == nid)
+				set_bit(j, apicid_map);
+
+		if (bitmap_empty(apicid_map, MAX_LOCAL_APIC))
+			continue;
+		bitmap_scnlistprintf(apicid_list, MAX_LOCAL_APIC,
+				     apicid_map, MAX_LOCAL_APIC);
+		pr_info("SRAT: PXM %u -> APIC {%s} -> Node %u\n",
+			i, apicid_list, nid);
+	}
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -281,6 +281,10 @@ acpi_table_parse_srat(enum acpi_srat_type id,
 					    handler, max_entries);
 }
 
+void __init __attribute__((weak)) acpi_numa_print_srat_mapping(void)
+{
+}
+
 int __init acpi_numa_init(void)
 {
 	/* SRAT: Static Resource Affinity Table */
@@ -292,6 +296,7 @@ int __init acpi_numa_init(void)
 		acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 				      acpi_parse_memory_affinity,
 				      NR_NODE_MEMBLKS);
+		acpi_numa_print_srat_mapping();
 	}
 
 	/* SLIT: System Locality Information Table */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -92,12 +92,13 @@ int acpi_table_parse_madt (enum acpi_madt_type id, acpi_table_entry_handler hand
 int acpi_parse_mcfg (struct acpi_table_header *header);
 void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
 
-/* the following four functions are architecture-dependent */
+/* the following six functions are architecture-dependent */
 void acpi_numa_slit_init (struct acpi_table_slit *slit);
 void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
 void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
 void acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
 void acpi_numa_arch_fixup(void);
+void acpi_numa_print_srat_mapping(void);
 
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */

Re: [patch] x86: reduce srat verbosity in the kernel log

[Mike Travis]

I applied your previous patch with the change to use static and
here's the console output from a live system:


[    0.000000] SRAT: PXM 0 -> APIC {0-7,16-23} -> Node 0
[    0.000000] SRAT: PXM 1 -> APIC {32-39,48-55} -> Node 1
[    0.000000] SRAT: PXM 2 -> APIC {64-71,80-87} -> Node 2
[    0.000000] SRAT: PXM 3 -> APIC {96-103,112-119} -> Node 3
[    0.000000] SRAT: PXM 4 -> APIC {128-135,144-151} -> Node 4
[    0.000000] SRAT: PXM 5 -> APIC {160-167,176-183} -> Node 5
[    0.000000] SRAT: PXM 6 -> APIC {192-199,208-215} -> Node 6
[    0.000000] SRAT: PXM 7 -> APIC {224-231,240-247} -> Node 7
[    0.000000] SRAT: PXM 8 -> APIC {256-263,272-279} -> Node 8
[    0.000000] SRAT: PXM 9 -> APIC {288-295,304-311} -> Node 9
[    0.000000] SRAT: PXM 10 -> APIC {320-327,336-343} -> Node 10
[    0.000000] SRAT: PXM 11 -> APIC {352-359,368-375} -> Node 11
[    0.000000] SRAT: PXM 12 -> APIC {384-391,400-407} -> Node 12
[    0.000000] SRAT: PXM 13 -> APIC {416-423,432-439} -> Node 13
[    0.000000] SRAT: PXM 14 -> APIC {448-455,464-471} -> Node 14
[    0.000000] SRAT: PXM 15 -> APIC {480-487,496-503} -> Node 15
[    0.000000] SRAT: PXM 16 -> APIC {512-519,528-535} -> Node 16
[    0.000000] SRAT: PXM 17 -> APIC {544-551,560-567} -> Node 17
[    0.000000] SRAT: PXM 18 -> APIC {576-583,592-599} -> Node 18
[    0.000000] SRAT: PXM 19 -> APIC {608-615,624-631} -> Node 19
[    0.000000] SRAT: PXM 20 -> APIC {640-647,656-663} -> Node 20
[    0.000000] SRAT: PXM 21 -> APIC {672-679,688-695} -> Node 21
[    0.000000] SRAT: PXM 22 -> APIC {704-711,720-727} -> Node 22
[    0.000000] SRAT: PXM 23 -> APIC {736-743,752-759} -> Node 23   

                                                                                              
David Rientjes wrote:
> On Tue, 27 Oct 2009, Mike Travis wrote:
> 
>> Hi David,
>>
>> Very Cool, I'll try it out and let you know how it goes.
>>
>> Note that it would be better to declare the BITMAP in the
>> static initdata section so it doesn't grow the stack by 4k
>> bytes.  (And it's thrown away after the kernel starts.)
>>
> 
> Right, here's an updated version.  I was thinking of MAX_PXM_DOMAINS being 
> 256 instead of MAX_LOCAL_APIC :)
> 
> Here's an updated version.  apicid_map and apicid_list don't need to be 
> synchronized because there're no concurrency issues here on init.
> 
> 
> 
> x86: reduce srat verbosity in the kernel log
> 
> It's possible to reduce the number of SRAT messages emitted to the kernel
> log by printing each valid pxm once and then creating bitmaps to represent
> the apicids that map to the same node.
> 
> This reduces lines such as
> 
> 	SRAT: PXM 0 -> APIC 0 -> Node 0
> 	SRAT: PXM 0 -> APIC 1 -> Node 0
> 	SRAT: PXM 1 -> APIC 2 -> Node 1
> 	SRAT: PXM 1 -> APIC 3 -> Node 1
> 
> to
> 
> 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
> 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
> 
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  arch/x86/mm/srat_64.c |   32 ++++++++++++++++++++++++++++----
>  drivers/acpi/numa.c   |    5 +++++
>  include/linux/acpi.h  |    3 ++-
>  3 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
> --- a/arch/x86/mm/srat_64.c
> +++ b/arch/x86/mm/srat_64.c
> @@ -36,6 +36,9 @@ static int num_node_memblks __initdata;
>  static struct bootnode node_memblk_range[NR_NODE_MEMBLKS] __initdata;
>  static int memblk_nodeid[NR_NODE_MEMBLKS] __initdata;
>  
> +static DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC) __initdata;
> +static char apicid_list[MAX_LOCAL_APIC] __initdata;
> +
>  static __init int setup_node(int pxm)
>  {
>  	return acpi_map_pxm_to_node(pxm);
> @@ -136,8 +139,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
>  	apicid_to_node[apic_id] = node;
>  	node_set(node, cpu_nodes_parsed);
>  	acpi_numa = 1;
> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> -	       pxm, apic_id, node);
>  }
>  
>  /* Callback for Proximity Domain -> LAPIC mapping */
> @@ -170,8 +171,31 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
>  	apicid_to_node[apic_id] = node;
>  	node_set(node, cpu_nodes_parsed);
>  	acpi_numa = 1;
> -	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
> -	       pxm, apic_id, node);
> +}
> +
> +void __init acpi_numa_print_srat_mapping(void)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
> +		int nid;
> +
> +		nid = pxm_to_node(i);
> +		if (nid == NUMA_NO_NODE)
> +			continue;
> +
> +		bitmap_zero(apicid_map, MAX_LOCAL_APIC);
> +		for (j = 0; j < MAX_LOCAL_APIC; j++)
> +			if (apicid_to_node[j] == nid)
> +				set_bit(j, apicid_map);
> +
> +		if (bitmap_empty(apicid_map, MAX_LOCAL_APIC))
> +			continue;
> +		bitmap_scnlistprintf(apicid_list, MAX_LOCAL_APIC,
> +				     apicid_map, MAX_LOCAL_APIC);
> +		pr_info("SRAT: PXM %u -> APIC {%s} -> Node %u\n",
> +			i, apicid_list, nid);
> +	}
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -281,6 +281,10 @@ acpi_table_parse_srat(enum acpi_srat_type id,
>  					    handler, max_entries);
>  }
>  
> +void __init __attribute__((weak)) acpi_numa_print_srat_mapping(void)
> +{
> +}
> +
>  int __init acpi_numa_init(void)
>  {
>  	/* SRAT: Static Resource Affinity Table */
> @@ -292,6 +296,7 @@ int __init acpi_numa_init(void)
>  		acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
>  				      acpi_parse_memory_affinity,
>  				      NR_NODE_MEMBLKS);
> +		acpi_numa_print_srat_mapping();
>  	}
>  
>  	/* SLIT: System Locality Information Table */
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -92,12 +92,13 @@ int acpi_table_parse_madt (enum acpi_madt_type id, acpi_table_entry_handler hand
>  int acpi_parse_mcfg (struct acpi_table_header *header);
>  void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
>  
> -/* the following four functions are architecture-dependent */
> +/* the following six functions are architecture-dependent */
>  void acpi_numa_slit_init (struct acpi_table_slit *slit);
>  void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
>  void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
>  void acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
>  void acpi_numa_arch_fixup(void);
> +void acpi_numa_print_srat_mapping(void);
>  
>  #ifdef CONFIG_ACPI_HOTPLUG_CPU
>  /* Arch dependent functions for cpu hotplug support */

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Tue, 27 Oct 2009, Mike Travis wrote:

> I applied your previous patch with the change to use static and
> here's the console output from a live system:
> 
> 
> [    0.000000] SRAT: PXM 0 -> APIC {0-7,16-23} -> Node 0
> [    0.000000] SRAT: PXM 1 -> APIC {32-39,48-55} -> Node 1
> [    0.000000] SRAT: PXM 2 -> APIC {64-71,80-87} -> Node 2
> [    0.000000] SRAT: PXM 3 -> APIC {96-103,112-119} -> Node 3
> [    0.000000] SRAT: PXM 4 -> APIC {128-135,144-151} -> Node 4
> [    0.000000] SRAT: PXM 5 -> APIC {160-167,176-183} -> Node 5
> [    0.000000] SRAT: PXM 6 -> APIC {192-199,208-215} -> Node 6
> [    0.000000] SRAT: PXM 7 -> APIC {224-231,240-247} -> Node 7
> [    0.000000] SRAT: PXM 8 -> APIC {256-263,272-279} -> Node 8
> [    0.000000] SRAT: PXM 9 -> APIC {288-295,304-311} -> Node 9
> [    0.000000] SRAT: PXM 10 -> APIC {320-327,336-343} -> Node 10
> [    0.000000] SRAT: PXM 11 -> APIC {352-359,368-375} -> Node 11
> [    0.000000] SRAT: PXM 12 -> APIC {384-391,400-407} -> Node 12
> [    0.000000] SRAT: PXM 13 -> APIC {416-423,432-439} -> Node 13
> [    0.000000] SRAT: PXM 14 -> APIC {448-455,464-471} -> Node 14
> [    0.000000] SRAT: PXM 15 -> APIC {480-487,496-503} -> Node 15
> [    0.000000] SRAT: PXM 16 -> APIC {512-519,528-535} -> Node 16
> [    0.000000] SRAT: PXM 17 -> APIC {544-551,560-567} -> Node 17
> [    0.000000] SRAT: PXM 18 -> APIC {576-583,592-599} -> Node 18
> [    0.000000] SRAT: PXM 19 -> APIC {608-615,624-631} -> Node 19
> [    0.000000] SRAT: PXM 20 -> APIC {640-647,656-663} -> Node 20
> [    0.000000] SRAT: PXM 21 -> APIC {672-679,688-695} -> Node 21
> [    0.000000] SRAT: PXM 22 -> APIC {704-711,720-727} -> Node 22
> [    0.000000] SRAT: PXM 23 -> APIC {736-743,752-759} -> Node 23   
> 

Quite the system you have there :)  What was once 760 lines has been 
reduced to 24 without removing any information.

This seems to be the most we can reduce this particular output since we 
don't support mapping multiple pxms to a single node.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Mike Travis]

David Rientjes wrote:
> On Tue, 27 Oct 2009, Mike Travis wrote:
> 
>> I applied your previous patch with the change to use static and
>> here's the console output from a live system:
>>
>>
>> [    0.000000] SRAT: PXM 0 -> APIC {0-7,16-23} -> Node 0
>> [    0.000000] SRAT: PXM 1 -> APIC {32-39,48-55} -> Node 1
>> [    0.000000] SRAT: PXM 2 -> APIC {64-71,80-87} -> Node 2
>> [    0.000000] SRAT: PXM 3 -> APIC {96-103,112-119} -> Node 3
>> [    0.000000] SRAT: PXM 4 -> APIC {128-135,144-151} -> Node 4
>> [    0.000000] SRAT: PXM 5 -> APIC {160-167,176-183} -> Node 5
>> [    0.000000] SRAT: PXM 6 -> APIC {192-199,208-215} -> Node 6
>> [    0.000000] SRAT: PXM 7 -> APIC {224-231,240-247} -> Node 7
>> [    0.000000] SRAT: PXM 8 -> APIC {256-263,272-279} -> Node 8
>> [    0.000000] SRAT: PXM 9 -> APIC {288-295,304-311} -> Node 9
>> [    0.000000] SRAT: PXM 10 -> APIC {320-327,336-343} -> Node 10
>> [    0.000000] SRAT: PXM 11 -> APIC {352-359,368-375} -> Node 11
>> [    0.000000] SRAT: PXM 12 -> APIC {384-391,400-407} -> Node 12
>> [    0.000000] SRAT: PXM 13 -> APIC {416-423,432-439} -> Node 13
>> [    0.000000] SRAT: PXM 14 -> APIC {448-455,464-471} -> Node 14
>> [    0.000000] SRAT: PXM 15 -> APIC {480-487,496-503} -> Node 15
>> [    0.000000] SRAT: PXM 16 -> APIC {512-519,528-535} -> Node 16
>> [    0.000000] SRAT: PXM 17 -> APIC {544-551,560-567} -> Node 17
>> [    0.000000] SRAT: PXM 18 -> APIC {576-583,592-599} -> Node 18
>> [    0.000000] SRAT: PXM 19 -> APIC {608-615,624-631} -> Node 19
>> [    0.000000] SRAT: PXM 20 -> APIC {640-647,656-663} -> Node 20
>> [    0.000000] SRAT: PXM 21 -> APIC {672-679,688-695} -> Node 21
>> [    0.000000] SRAT: PXM 22 -> APIC {704-711,720-727} -> Node 22
>> [    0.000000] SRAT: PXM 23 -> APIC {736-743,752-759} -> Node 23   
>>
> 
> Quite the system you have there :)  What was once 760 lines has been 
> reduced to 24 without removing any information.
> 
> This seems to be the most we can reduce this particular output since we 
> don't support mapping multiple pxms to a single node.

Yes, thanks very much for the optimization.

(And you can add my Acked-by or whatever you need.)

Tomorrow I will have more time on the system and will try out all the
new changes together, mostly with summarizing the Processor stats.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Andi Kleen]

>> Quite the system you have there :)  What was once 760 lines has been 
>> reduced to 24 without removing any information.
>>
>> This seems to be the most we can reduce this particular output since we 
>> don't support mapping multiple pxms to a single node.
>
> Yes, thanks very much for the optimization.
>
> (And you can add my Acked-by or whatever you need.)

Looks also good to me, thanks. Also Acked-by.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Andi Kleen wrote:

> >> Quite the system you have there :)  What was once 760 lines has been 
> >> reduced to 24 without removing any information.
> >>
> >> This seems to be the most we can reduce this particular output since we 
> >> don't support mapping multiple pxms to a single node.
> >
> > Yes, thanks very much for the optimization.
> >
> > (And you can add my Acked-by or whatever you need.)
> 
> Looks also good to me, thanks. Also Acked-by.
> 

Thanks Andi.  I'm hoping Ingo will pick this up and not have a problem 
with the use of NUMA_NO_NODE vs. NID_INVAL since there's a patch pending 
in -mm that removes the former and this saves a Linus build error when he 
pushes for 2.6.33 (and they are both defined the same anyway).

Re: [patch] x86: reduce srat verbosity in the kernel log

[Yinghai Lu]

David Rientjes wrote:
> On Tue, 27 Oct 2009, Mike Travis wrote:
> 
>> I applied your previous patch with the change to use static and
>> here's the console output from a live system:
>>
>>
>> [    0.000000] SRAT: PXM 0 -> APIC {0-7,16-23} -> Node 0
>> [    0.000000] SRAT: PXM 1 -> APIC {32-39,48-55} -> Node 1
>> [    0.000000] SRAT: PXM 2 -> APIC {64-71,80-87} -> Node 2
>> [    0.000000] SRAT: PXM 3 -> APIC {96-103,112-119} -> Node 3
>> [    0.000000] SRAT: PXM 4 -> APIC {128-135,144-151} -> Node 4
>> [    0.000000] SRAT: PXM 5 -> APIC {160-167,176-183} -> Node 5
>> [    0.000000] SRAT: PXM 6 -> APIC {192-199,208-215} -> Node 6
>> [    0.000000] SRAT: PXM 7 -> APIC {224-231,240-247} -> Node 7
>> [    0.000000] SRAT: PXM 8 -> APIC {256-263,272-279} -> Node 8
>> [    0.000000] SRAT: PXM 9 -> APIC {288-295,304-311} -> Node 9
>> [    0.000000] SRAT: PXM 10 -> APIC {320-327,336-343} -> Node 10
>> [    0.000000] SRAT: PXM 11 -> APIC {352-359,368-375} -> Node 11
>> [    0.000000] SRAT: PXM 12 -> APIC {384-391,400-407} -> Node 12
>> [    0.000000] SRAT: PXM 13 -> APIC {416-423,432-439} -> Node 13
>> [    0.000000] SRAT: PXM 14 -> APIC {448-455,464-471} -> Node 14
>> [    0.000000] SRAT: PXM 15 -> APIC {480-487,496-503} -> Node 15
>> [    0.000000] SRAT: PXM 16 -> APIC {512-519,528-535} -> Node 16
>> [    0.000000] SRAT: PXM 17 -> APIC {544-551,560-567} -> Node 17
>> [    0.000000] SRAT: PXM 18 -> APIC {576-583,592-599} -> Node 18
>> [    0.000000] SRAT: PXM 19 -> APIC {608-615,624-631} -> Node 19
>> [    0.000000] SRAT: PXM 20 -> APIC {640-647,656-663} -> Node 20
>> [    0.000000] SRAT: PXM 21 -> APIC {672-679,688-695} -> Node 21
>> [    0.000000] SRAT: PXM 22 -> APIC {704-711,720-727} -> Node 22
>> [    0.000000] SRAT: PXM 23 -> APIC {736-743,752-759} -> Node 23   
>>
> 
> Quite the system you have there :)  What was once 760 lines has been 
> reduced to 24 without removing any information.
> 

can you change the apic to hex print?

YH

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Tue, 27 Oct 2009, Yinghai Lu wrote:

> can you change the apic to hex print?
> 

That would be an extension made on top of my patch (which may be difficult 
without adding an additional library function to be used in this case 
since it relies on bitmap_scnlistprintf()).  It's been printed as an 
unsigned int for well over four years so I don't see any specific urgency, 
anyway.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Cyrill Gorcunov]

[David Rientjes - Tue, Oct 27, 2009 at 01:25:51PM -0700]
| On Tue, 27 Oct 2009, Mike Travis wrote:
| 
...
| +
| +void __init acpi_numa_print_srat_mapping(void)
| +{
| +	int i, j;
| +
| +	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
| +		int nid;
| +
| +		nid = pxm_to_node(i);
| +		if (nid == NUMA_NO_NODE)

Btw, David, while you at it, I just curious -- shouldn't we test it
with NID_INVAL (as pxm_to_node_map initially defined to)? Not a big
deal at all (since they are both = -1) but for the record.
Or perhaps I miss something?

| +			continue;
...

	-- Cyrill

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Tue, 27 Oct 2009, Cyrill Gorcunov wrote:

> | +void __init acpi_numa_print_srat_mapping(void)
> | +{
> | +	int i, j;
> | +
> | +	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
> | +		int nid;
> | +
> | +		nid = pxm_to_node(i);
> | +		if (nid == NUMA_NO_NODE)
> 
> Btw, David, while you at it, I just curious -- shouldn't we test it
> with NID_INVAL (as pxm_to_node_map initially defined to)? Not a big
> deal at all (since they are both = -1) but for the record.
> Or perhaps I miss something?
> 

I don't think we need to address that since NID_INVAL is going away and 
will be replaced by NUMA_NO_NODE since Lee has exposed it globally in his 
mempolicy patchset, and as you mention they are the same anyway.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Cyrill Gorcunov]

[David Rientjes - Tue, Oct 27, 2009 at 02:06:21PM -0700]
| On Tue, 27 Oct 2009, Cyrill Gorcunov wrote:
| 
| > | +void __init acpi_numa_print_srat_mapping(void)
| > | +{
| > | +	int i, j;
| > | +
| > | +	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
| > | +		int nid;
| > | +
| > | +		nid = pxm_to_node(i);
| > | +		if (nid == NUMA_NO_NODE)
| > 
| > Btw, David, while you at it, I just curious -- shouldn't we test it
| > with NID_INVAL (as pxm_to_node_map initially defined to)? Not a big
| > deal at all (since they are both = -1) but for the record.
| > Or perhaps I miss something?
| > 
| 
| I don't think we need to address that since NID_INVAL is going away and 
| will be replaced by NUMA_NO_NODE since Lee has exposed it globally in his 
| mempolicy patchset, and as you mention they are the same anyway.
| 

I see. Thanks!

	-- Cyrill

Re: [patch] x86: reduce srat verbosity in the kernel log

[Andi Kleen]

> +static DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC) __initdata;
> +static char apicid_list[MAX_LOCAL_APIC] __initdata;

Is MAX_LOCAL_APIC really big enough to print them all in ASCII?

It would be better to not use that large a buffer, but print
in smaller pieces (I realize this would enlarge your patch,
but then it would also save a lot of BSS)

-Andi

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Andi Kleen wrote:

> > +static DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC) __initdata;
> > +static char apicid_list[MAX_LOCAL_APIC] __initdata;
> 
> Is MAX_LOCAL_APIC really big enough to print them all in ASCII?
> 
> It would be better to not use that large a buffer, but print
> in smaller pieces (I realize this would enlarge your patch,
> but then it would also save a lot of BSS)
> 

MAX_LOCAL_APIC was definitely an arbitrary choice here and has very little 
relevance.  scnlistprintf will protect against overflow, but we still need 
to decide upon a constant that will emit the most information possible 
while not overly polluting the printk and saving on bss, as you mentioned.  
I suspect we could agree on a value as little as 128 and it would work for 
the overwhelming majority (all?) of users.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Andi Kleen]

> > 
> 
> MAX_LOCAL_APIC was definitely an arbitrary choice here and has very little 
> relevance.  scnlistprintf will protect against overflow, but we still need 
> to decide upon a constant that will emit the most information possible 
> while not overly polluting the printk and saving on bss, as you mentioned.  
> I suspect we could agree on a value as little as 128 and it would work for 
> the overwhelming majority (all?) of users.

For now at least seems reasonable to limit to 128 or so yes (and go
back to the stack). if we ever have sparse apic ids for nodes 
then that might change; but in this case could still just do
a acpidump or teach the printer to be more clever and support
strides.

It would be just good to have some indication in the output
if there was a overflow.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

[patch v2] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Andi Kleen wrote:

> For now at least seems reasonable to limit to 128 or so yes (and go
> back to the stack). if we ever have sparse apic ids for nodes 
> then that might change; but in this case could still just do
> a acpidump or teach the printer to be more clever and support
> strides.
> 

It'll support sparse apicids, which was shown in Mike's example, although 
it only becomes a problem when they cannot be represented in a 128 
character buffer.  I doubt there are many machines where that happens 
given the way they are formed.

> It would be just good to have some indication in the output
> if there was a overflow.
> 

Agreed, a trailing "..." would be appropriate.



x86: reduce srat verbosity in the kernel log

It's possible to reduce the number of SRAT messages emitted to the kernel
log by printing each valid pxm once and then creating bitmaps to represent
the apic ids that map to the same node.

This reduces lines such as

	SRAT: PXM 0 -> APIC 0 -> Node 0
	SRAT: PXM 0 -> APIC 1 -> Node 0
	SRAT: PXM 1 -> APIC 2 -> Node 1
	SRAT: PXM 1 -> APIC 3 -> Node 1

to

	SRAT: PXM 0 -> APIC {0-1} -> Node 0
	SRAT: PXM 1 -> APIC {2-3} -> Node 1

The buffer used to store the apic id list is 128 characters in length.
If that is too small to represent all the apic id ranges that are bound
to a single pxm, a trailing "..." is added.  APICID_LIST_LEN should be
manually increased for such configurations.

Acked-by: Mike Travis <travis@sgi.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 arch/x86/mm/srat_64.c |   41 +++++++++++++++++++++++++++++++++++++----
 drivers/acpi/numa.c   |    5 +++++
 include/linux/acpi.h  |    3 ++-
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
--- a/arch/x86/mm/srat_64.c
+++ b/arch/x86/mm/srat_64.c
@@ -36,6 +36,9 @@ static int num_node_memblks __initdata;
 static struct bootnode node_memblk_range[NR_NODE_MEMBLKS] __initdata;
 static int memblk_nodeid[NR_NODE_MEMBLKS] __initdata;
 
+static DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC) __initdata;
+#define APICID_LIST_LEN	(128)
+
 static __init int setup_node(int pxm)
 {
 	return acpi_map_pxm_to_node(pxm);
@@ -136,8 +139,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 	apicid_to_node[apic_id] = node;
 	node_set(node, cpu_nodes_parsed);
 	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
-	       pxm, apic_id, node);
 }
 
 /* Callback for Proximity Domain -> LAPIC mapping */
@@ -170,8 +171,40 @@ acpi_numa_processor_affinity_init(struct acpi_srat_cpu_affinity *pa)
 	apicid_to_node[apic_id] = node;
 	node_set(node, cpu_nodes_parsed);
 	acpi_numa = 1;
-	printk(KERN_INFO "SRAT: PXM %u -> APIC %u -> Node %u\n",
-	       pxm, apic_id, node);
+}
+
+void __init acpi_numa_print_srat_mapping(void)
+{
+	char apicid_list[APICID_LIST_LEN];
+	int i, j;
+
+	for (i = 0; i < MAX_PXM_DOMAINS; i++) {
+		int len;
+		int nid;
+
+		nid = pxm_to_node(i);
+		if (nid == NUMA_NO_NODE)
+			continue;
+
+		bitmap_zero(apicid_map, MAX_LOCAL_APIC);
+		for (j = 0; j < MAX_LOCAL_APIC; j++)
+			if (apicid_to_node[j] == nid)
+				set_bit(j, apicid_map);
+
+		if (bitmap_empty(apicid_map, MAX_LOCAL_APIC))
+			continue;
+
+		/*
+		 * If the bitmap cannot be listed in a buffer of length
+		 * APICID_LIST_LEN, then it is suffixed with "...".
+		 */
+		len = bitmap_scnlistprintf(apicid_list, APICID_LIST_LEN,
+					   apicid_map, MAX_LOCAL_APIC);
+		pr_info("SRAT: PXM %u -> APIC {%s%s} -> Node %u\n",
+			i, apicid_list,
+			(len == APICID_LIST_LEN - 1) ? "..." : "",
+			nid);
+	}
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -281,6 +281,10 @@ acpi_table_parse_srat(enum acpi_srat_type id,
 					    handler, max_entries);
 }
 
+void __init __attribute__((weak)) acpi_numa_print_srat_mapping(void)
+{
+}
+
 int __init acpi_numa_init(void)
 {
 	/* SRAT: Static Resource Affinity Table */
@@ -292,6 +296,7 @@ int __init acpi_numa_init(void)
 		acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 				      acpi_parse_memory_affinity,
 				      NR_NODE_MEMBLKS);
+		acpi_numa_print_srat_mapping();
 	}
 
 	/* SLIT: System Locality Information Table */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -92,12 +92,13 @@ int acpi_table_parse_madt (enum acpi_madt_type id, acpi_table_entry_handler hand
 int acpi_parse_mcfg (struct acpi_table_header *header);
 void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
 
-/* the following four functions are architecture-dependent */
+/* the following six functions are architecture-dependent */
 void acpi_numa_slit_init (struct acpi_table_slit *slit);
 void acpi_numa_processor_affinity_init (struct acpi_srat_cpu_affinity *pa);
 void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
 void acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
 void acpi_numa_arch_fixup(void);
+void acpi_numa_print_srat_mapping(void);
 
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[Andi Kleen]

> +		/*
> +		 * If the bitmap cannot be listed in a buffer of length
> +		 * APICID_LIST_LEN, then it is suffixed with "...".
> +		 */
> +		len = bitmap_scnlistprintf(apicid_list, APICID_LIST_LEN,
> +					   apicid_map, MAX_LOCAL_APIC);
> +		pr_info("SRAT: PXM %u -> APIC {%s%s} -> Node %u\n",
> +			i, apicid_list,
> +			(len == APICID_LIST_LEN - 1) ? "..." : "",

Is the - 1 really correct? If scnlistprintf follows snprintf semantics then it would not
be and my understanding is it is supposed to.

Other than that it looks good.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Andi Kleen wrote:

> > +		/*
> > +		 * If the bitmap cannot be listed in a buffer of length
> > +		 * APICID_LIST_LEN, then it is suffixed with "...".
> > +		 */
> > +		len = bitmap_scnlistprintf(apicid_list, APICID_LIST_LEN,
> > +					   apicid_map, MAX_LOCAL_APIC);
> > +		pr_info("SRAT: PXM %u -> APIC {%s%s} -> Node %u\n",
> > +			i, apicid_list,
> > +			(len == APICID_LIST_LEN - 1) ? "..." : "",
> 
> Is the - 1 really correct? If scnlistprintf follows snprintf semantics then it would not
> be and my understanding is it is supposed to.
> 

It is, bitmap_scnlistprintf() returns the number of characters printed to 
the buffer minus the trailing '\0', which is different from snprintf().  
APICID_LIST_LEN-1 then identifies when the buffer was max'd out.  It still 
adds a trailing "..." if the list is exactly 128 characters long, but this 
isn't addressed to avoid added complexity.

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Tue, 27 Oct 2009, David Rientjes wrote:

> x86: reduce srat verbosity in the kernel log
> 
> It's possible to reduce the number of SRAT messages emitted to the kernel
> log by printing each valid pxm once and then creating bitmaps to represent
> the apic ids that map to the same node.
> 
> This reduces lines such as
> 
> 	SRAT: PXM 0 -> APIC 0 -> Node 0
> 	SRAT: PXM 0 -> APIC 1 -> Node 0
> 	SRAT: PXM 1 -> APIC 2 -> Node 1
> 	SRAT: PXM 1 -> APIC 3 -> Node 1
> 
> to
> 
> 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
> 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
> 
> The buffer used to store the apic id list is 128 characters in length.
> If that is too small to represent all the apic id ranges that are bound
> to a single pxm, a trailing "..." is added.  APICID_LIST_LEN should be
> manually increased for such configurations.
> 
> Acked-by: Mike Travis <travis@sgi.com>
> Signed-off-by: David Rientjes <rientjes@google.com>

Ingo, have you had a chance to look at merging this yet?

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[Ingo Molnar]

* David Rientjes <rientjes@google.com> wrote:

> On Tue, 27 Oct 2009, David Rientjes wrote:
> 
> > x86: reduce srat verbosity in the kernel log
> > 
> > It's possible to reduce the number of SRAT messages emitted to the kernel
> > log by printing each valid pxm once and then creating bitmaps to represent
> > the apic ids that map to the same node.
> > 
> > This reduces lines such as
> > 
> > 	SRAT: PXM 0 -> APIC 0 -> Node 0
> > 	SRAT: PXM 0 -> APIC 1 -> Node 0
> > 	SRAT: PXM 1 -> APIC 2 -> Node 1
> > 	SRAT: PXM 1 -> APIC 3 -> Node 1
> > 
> > to
> > 
> > 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
> > 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
> > 
> > The buffer used to store the apic id list is 128 characters in length.
> > If that is too small to represent all the apic id ranges that are bound
> > to a single pxm, a trailing "..." is added.  APICID_LIST_LEN should be
> > manually increased for such configurations.
> > 
> > Acked-by: Mike Travis <travis@sgi.com>
> > Signed-off-by: David Rientjes <rientjes@google.com>
> 
> Ingo, have you had a chance to look at merging this yet?

I'm waiting for Mike to test them (and other patches) and send a new 
series out with bits to pick up.

But i really dont like such type of buffering - in the past they tended 
to be problematic. Why print this info at all in the default bootup?  
It's not needed on a correctly functioning system.

For failure analysis make it opt-in available via a boot parameter (if 
it's needed for bootup analysis) - but otherwise just dont print it.

	Ingo

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[Yinghai Lu]

Ingo Molnar wrote:
> * David Rientjes <rientjes@google.com> wrote:
> 
>> On Tue, 27 Oct 2009, David Rientjes wrote:
>>
>>> x86: reduce srat verbosity in the kernel log
>>>
>>> It's possible to reduce the number of SRAT messages emitted to the kernel
>>> log by printing each valid pxm once and then creating bitmaps to represent
>>> the apic ids that map to the same node.
>>>
>>> This reduces lines such as
>>>
>>> 	SRAT: PXM 0 -> APIC 0 -> Node 0
>>> 	SRAT: PXM 0 -> APIC 1 -> Node 0
>>> 	SRAT: PXM 1 -> APIC 2 -> Node 1
>>> 	SRAT: PXM 1 -> APIC 3 -> Node 1
>>>
>>> to
>>>
>>> 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
>>> 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
>>>
>>> The buffer used to store the apic id list is 128 characters in length.
>>> If that is too small to represent all the apic id ranges that are bound
>>> to a single pxm, a trailing "..." is added.  APICID_LIST_LEN should be
>>> manually increased for such configurations.
>>>
>>> Acked-by: Mike Travis <travis@sgi.com>
>>> Signed-off-by: David Rientjes <rientjes@google.com>
>> Ingo, have you had a chance to look at merging this yet?
> 
> I'm waiting for Mike to test them (and other patches) and send a new 
> series out with bits to pick up.
> 
> But i really dont like such type of buffering - in the past they tended 
> to be problematic. Why print this info at all in the default bootup?  
> It's not needed on a correctly functioning system.
> 
> For failure analysis make it opt-in available via a boot parameter (if 
> it's needed for bootup analysis) - but otherwise just dont print it.
> 
make them to depend on apic=debug or apic=verbose?

YH

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > * David Rientjes <rientjes@google.com> wrote:
> > 
> >> On Tue, 27 Oct 2009, David Rientjes wrote:
> >>
> >>> x86: reduce srat verbosity in the kernel log
> >>>
> >>> It's possible to reduce the number of SRAT messages emitted to the kernel
> >>> log by printing each valid pxm once and then creating bitmaps to represent
> >>> the apic ids that map to the same node.
> >>>
> >>> This reduces lines such as
> >>>
> >>> 	SRAT: PXM 0 -> APIC 0 -> Node 0
> >>> 	SRAT: PXM 0 -> APIC 1 -> Node 0
> >>> 	SRAT: PXM 1 -> APIC 2 -> Node 1
> >>> 	SRAT: PXM 1 -> APIC 3 -> Node 1
> >>>
> >>> to
> >>>
> >>> 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
> >>> 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
> >>>
> >>> The buffer used to store the apic id list is 128 characters in length.
> >>> If that is too small to represent all the apic id ranges that are bound
> >>> to a single pxm, a trailing "..." is added.  APICID_LIST_LEN should be
> >>> manually increased for such configurations.
> >>>
> >>> Acked-by: Mike Travis <travis@sgi.com>
> >>> Signed-off-by: David Rientjes <rientjes@google.com>
> >> Ingo, have you had a chance to look at merging this yet?
> > 
> > I'm waiting for Mike to test them (and other patches) and send a new 
> > series out with bits to pick up.
> > 
> > But i really dont like such type of buffering - in the past they tended 
> > to be problematic. Why print this info at all in the default bootup?  
> > It's not needed on a correctly functioning system.
> > 
> > For failure analysis make it opt-in available via a boot parameter (if 
> > it's needed for bootup analysis) - but otherwise just dont print it.
> > 
> make them to depend on apic=debug or apic=verbose?

Yeah - i'd definitely suggest to not splinter the boot flag space too 
much - users wont know what to enable in case of trouble.

But that is a detail (and it can be improved later on as well).

	Ingo

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[Mike Travis]

Ingo Molnar wrote:
> * David Rientjes <rientjes@google.com> wrote:
> 
>> On Tue, 27 Oct 2009, David Rientjes wrote:
>>
>>> x86: reduce srat verbosity in the kernel log
>>>
>>> It's possible to reduce the number of SRAT messages emitted to the kernel
>>> log by printing each valid pxm once and then creating bitmaps to represent
>>> the apic ids that map to the same node.
>>>
>>> This reduces lines such as
>>>
>>> 	SRAT: PXM 0 -> APIC 0 -> Node 0
>>> 	SRAT: PXM 0 -> APIC 1 -> Node 0
>>> 	SRAT: PXM 1 -> APIC 2 -> Node 1
>>> 	SRAT: PXM 1 -> APIC 3 -> Node 1
>>>
>>> to
>>>
>>> 	SRAT: PXM 0 -> APIC {0-1} -> Node 0
>>> 	SRAT: PXM 1 -> APIC {2-3} -> Node 1
>>>
>>> The buffer used to store the apic id list is 128 characters in length.
>>> If that is too small to represent all the apic id ranges that are bound
>>> to a single pxm, a trailing "..." is added.  APICID_LIST_LEN should be
>>> manually increased for such configurations.
>>>
>>> Acked-by: Mike Travis <travis@sgi.com>
>>> Signed-off-by: David Rientjes <rientjes@google.com>
>> Ingo, have you had a chance to look at merging this yet?
> 
> I'm waiting for Mike to test them (and other patches) and send a new 
> series out with bits to pick up.
> 
> But i really dont like such type of buffering - in the past they tended 
> to be problematic. Why print this info at all in the default bootup?  
> It's not needed on a correctly functioning system.
> 
> For failure analysis make it opt-in available via a boot parameter (if 
> it's needed for bootup analysis) - but otherwise just dont print it.
> 
> 	Ingo

Hi,

Sorry, it's been time consuming getting this checked out as our test
systems are much more in demand right now (SC09 is here.)

I'm very close to submitting another version, just picking up
everyone's comments now.  One more test run this afternoon and
I should be able to submit the patches.  I believe I've got a
good compromise between informative messages and compactness,
without any additional overhead.

I've also tested David's patch in every run and it hasn't shown any
problems at all.  (In fact, a recent merge of ACPI 4.0 code and it
still works flawlessly.)

Thanks,
Mike

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Tue, 10 Nov 2009, Ingo Molnar wrote:

> I'm waiting for Mike to test them (and other patches) and send a new 
> series out with bits to pick up.
> 

Mike posted his series today without including my patch, so I've replied 
to it.

> But i really dont like such type of buffering - in the past they tended 
> to be problematic.

I'm not sure that I'd call it buffering when iterating through all apic 
id's and setting appropriate bits in a bitmap when they map to a node id.  
It's apparently not been problematic either on my machines, Mike's 
machines, or his merge with ACPI 4.0 code.  I think the code is pretty 
straight forward.

> Why print this info at all in the default bootup?  
> It's not needed on a correctly functioning system.
> 

We have no other export of the apic id to to node mappings in the kernel.  
We already show each pxm's address range, each node's address range, and 
the pxm to node map.  The only other way to map apic ids to nodes is by 
looking for the lines "CPU 0/0 -> Node 0," which I believe are being 
removed.

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[Mike Travis]

David Rientjes wrote:
> On Tue, 10 Nov 2009, Ingo Molnar wrote:
> 
>> I'm waiting for Mike to test them (and other patches) and send a new 
>> series out with bits to pick up.
>>
> 
> Mike posted his series today without including my patch, so I've replied 
> to it.

Sorry, I wasn't aware I should have.
> 
>> But i really dont like such type of buffering - in the past they tended 
>> to be problematic.
> 
> I'm not sure that I'd call it buffering when iterating through all apic 
> id's and setting appropriate bits in a bitmap when they map to a node id.  
> It's apparently not been problematic either on my machines, Mike's 
> machines, or his merge with ACPI 4.0 code.  I think the code is pretty 
> straight forward.
> 
>> Why print this info at all in the default bootup?  
>> It's not needed on a correctly functioning system.
>>
> 
> We have no other export of the apic id to to node mappings in the kernel.  
> We already show each pxm's address range, each node's address range, and 
> the pxm to node map.  The only other way to map apic ids to nodes is by 
> looking for the lines "CPU 0/0 -> Node 0," which I believe are being 
> removed.

The bootup messages in my patch 1/7 list nodes and their processors as each
boots.  And this is easily found under /sysfs.

Also, I think in general that all the apic messages, unless they represent
"system boot progress" should be displayed only when asked for, like with
apic=debug or verbose?   Something more like:

BIOS-provided physical RAM map processed.
EFI: memory allocated.
SRAT: table interpreted.
Bootmem setups complete.
ACPI: APIC's enabled.
PM: Registered all nosave memory.

Removing the above tables remove about 3400 lines of console output on a 1k
thread machine.  There are 20,000+ lines of output before you get to the
login prompt (even with the removal of cpu bootup messages).

But you are right, the apic info should be available via /sysfs or /procfs.

The next BIG output is from devices.  Listing all the pci busses available
is an overkill as that info is also readily available when the system is
running.

Re: [patch v2] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Thu, 12 Nov 2009, Mike Travis wrote:

> Also, I think in general that all the apic messages, unless they represent
> "system boot progress" should be displayed only when asked for, like with
> apic=debug or verbose?   Something more like:
> 

That's outside the scope of my patch.  My patch does what the title says, 
it reduces srat verbosity in the kernel log.  If an additional change 
would like to suppress that output with a kernel parameter, that's fine, 
but it's an additional change and not what I was addressing.

When posting a patchset like this where all patches are related for a 
common goal and one patch (mine) was proposed during the development of 
the set, it's normal to include that patch in future postings with proper 
attribution given by indicating an author other than yourself in the very 
first line of the email:

	From: David Rientjes <rientjes@google.com>

and retaining your acked-by line, my signed-off-by line, and then adding 
your own signed-off-by line.

If a subsequent patch were to suppress this for kernels not using a 
certain parameter, I certainly wouldn't object to it.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Mike Travis]

Andi Kleen wrote:
>> MAX_LOCAL_APIC was definitely an arbitrary choice here and has very little 
>> relevance.  scnlistprintf will protect against overflow, but we still need 
>> to decide upon a constant that will emit the most information possible 
>> while not overly polluting the printk and saving on bss, as you mentioned.  
>> I suspect we could agree on a value as little as 128 and it would work for 
>> the overwhelming majority (all?) of users.
> 
> For now at least seems reasonable to limit to 128 or so yes (and go
> back to the stack). if we ever have sparse apic ids for nodes 
> then that might change; but in this case could still just do
> a acpidump or teach the printer to be more clever and support
> strides.
> 
> It would be just good to have some indication in the output
> if there was a overflow.
> 
> -Andi
> 

I don't understand the importance of this when the memory is given back
after the system starts up anyway...?

Thanks,
Mike

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Mike Travis wrote:

> I don't understand the importance of this when the memory is given back
> after the system starts up anyway...?
> 

Printing a list of apic ids longer than 128 characters would pollute the 
kernel log and this upper bound will probably never be reached based on 
the way apic ids are created for physical and logical processors: they are 
normally reduced to ranges instead of comma seperated entities.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Mike Travis]

David Rientjes wrote:
> On Wed, 28 Oct 2009, Mike Travis wrote:
> 
>> I don't understand the importance of this when the memory is given back
>> after the system starts up anyway...?
>>
> 
> Printing a list of apic ids longer than 128 characters would pollute the 
> kernel log and this upper bound will probably never be reached based on 
> the way apic ids are created for physical and logical processors: they are 
> normally reduced to ranges instead of comma seperated entities.

Your latest patch tested:

[    0.000000] SRAT: PXM 0 -> APIC {0-7,16-23} -> Node 0
[    0.000000] SRAT: PXM 1 -> APIC {32-39,48-55} -> Node 1
[    0.000000] SRAT: PXM 2 -> APIC {64-71,80-87} -> Node 2
[    0.000000] SRAT: PXM 3 -> APIC {96-103,112-119} -> Node 3
[    0.000000] SRAT: PXM 4 -> APIC {128-135,144-151} -> Node 4
[    0.000000] SRAT: PXM 5 -> APIC {160-167,176-183} -> Node 5
[    0.000000] SRAT: PXM 6 -> APIC {192-199,208-215} -> Node 6
[    0.000000] SRAT: PXM 7 -> APIC {224-231,240-247} -> Node 7
[    0.000000] SRAT: PXM 8 -> APIC {256-263,272-279} -> Node 8
[    0.000000] SRAT: PXM 9 -> APIC {288-295,304-311} -> Node 9
[    0.000000] SRAT: PXM 10 -> APIC {320-327,336-343} -> Node 10
[    0.000000] SRAT: PXM 11 -> APIC {352-359,368-375} -> Node 11
[    0.000000] SRAT: PXM 12 -> APIC {384-391,400-407} -> Node 12
[    0.000000] SRAT: PXM 13 -> APIC {416-423,432-439} -> Node 13
[    0.000000] SRAT: PXM 14 -> APIC {448-455,464-471} -> Node 14
[    0.000000] SRAT: PXM 15 -> APIC {480-487,496-503} -> Node 15
[    0.000000] SRAT: PXM 16 -> APIC {512-519,528-535} -> Node 16
[    0.000000] SRAT: PXM 17 -> APIC {544-551,560-567} -> Node 17
[    0.000000] SRAT: PXM 18 -> APIC {576-583,592-599} -> Node 18
[    0.000000] SRAT: PXM 19 -> APIC {608-615,624-631} -> Node 19
[    0.000000] SRAT: PXM 20 -> APIC {640-647,656-663} -> Node 20
[    0.000000] SRAT: PXM 21 -> APIC {672-679,688-695} -> Node 21
[    0.000000] SRAT: PXM 22 -> APIC {704-711,720-727} -> Node 22
[    0.000000] SRAT: PXM 23 -> APIC {736-743,752-759} -> Node 23
[    0.000000] SRAT: PXM 24 -> APIC {768-775,784-791} -> Node 24
[    0.000000] SRAT: PXM 25 -> APIC {800-807,816-823} -> Node 25
[    0.000000] SRAT: PXM 26 -> APIC {832-839,848-855} -> Node 26
[    0.000000] SRAT: PXM 27 -> APIC {864-871,880-887} -> Node 27
[    0.000000] SRAT: PXM 28 -> APIC {896-903,912-919} -> Node 28
[    0.000000] SRAT: PXM 29 -> APIC {928-935,944-951} -> Node 29
[    0.000000] SRAT: PXM 30 -> APIC {960-967,976-983} -> Node 30
[    0.000000] SRAT: PXM 31 -> APIC {992-999,1008-1015} -> Node 31
[    0.000000] SRAT: PXM 32 -> APIC {1024-1031,1040-1047} -> Node 32
[    0.000000] SRAT: PXM 33 -> APIC {1056-1063,1072-1079} -> Node 33
[    0.000000] SRAT: PXM 34 -> APIC {1088-1095,1104-1111} -> Node 34
[    0.000000] SRAT: PXM 35 -> APIC {1120-1127,1136-1143} -> Node 35
[    0.000000] SRAT: PXM 36 -> APIC {1152-1159,1168-1175} -> Node 36
[    0.000000] SRAT: PXM 37 -> APIC {1184-1191,1200-1207} -> Node 37
[    0.000000] SRAT: PXM 38 -> APIC {1216-1223,1232-1239} -> Node 38
[    0.000000] SRAT: PXM 39 -> APIC {1248-1255,1264-1271} -> Node 39                                                                                             

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Mike Travis wrote:

> Your latest patch tested:
> 
> [    0.000000] SRAT: PXM 0 -> APIC {0-7,16-23} -> Node 0
> [    0.000000] SRAT: PXM 1 -> APIC {32-39,48-55} -> Node 1
> [    0.000000] SRAT: PXM 2 -> APIC {64-71,80-87} -> Node 2
> [    0.000000] SRAT: PXM 3 -> APIC {96-103,112-119} -> Node 3
> [    0.000000] SRAT: PXM 4 -> APIC {128-135,144-151} -> Node 4
> [    0.000000] SRAT: PXM 5 -> APIC {160-167,176-183} -> Node 5
> [    0.000000] SRAT: PXM 6 -> APIC {192-199,208-215} -> Node 6
> [    0.000000] SRAT: PXM 7 -> APIC {224-231,240-247} -> Node 7
> [    0.000000] SRAT: PXM 8 -> APIC {256-263,272-279} -> Node 8
> [    0.000000] SRAT: PXM 9 -> APIC {288-295,304-311} -> Node 9
> [    0.000000] SRAT: PXM 10 -> APIC {320-327,336-343} -> Node 10
> [    0.000000] SRAT: PXM 11 -> APIC {352-359,368-375} -> Node 11
> [    0.000000] SRAT: PXM 12 -> APIC {384-391,400-407} -> Node 12
> [    0.000000] SRAT: PXM 13 -> APIC {416-423,432-439} -> Node 13
> [    0.000000] SRAT: PXM 14 -> APIC {448-455,464-471} -> Node 14
> [    0.000000] SRAT: PXM 15 -> APIC {480-487,496-503} -> Node 15
> [    0.000000] SRAT: PXM 16 -> APIC {512-519,528-535} -> Node 16
> [    0.000000] SRAT: PXM 17 -> APIC {544-551,560-567} -> Node 17
> [    0.000000] SRAT: PXM 18 -> APIC {576-583,592-599} -> Node 18
> [    0.000000] SRAT: PXM 19 -> APIC {608-615,624-631} -> Node 19
> [    0.000000] SRAT: PXM 20 -> APIC {640-647,656-663} -> Node 20
> [    0.000000] SRAT: PXM 21 -> APIC {672-679,688-695} -> Node 21
> [    0.000000] SRAT: PXM 22 -> APIC {704-711,720-727} -> Node 22
> [    0.000000] SRAT: PXM 23 -> APIC {736-743,752-759} -> Node 23
> [    0.000000] SRAT: PXM 24 -> APIC {768-775,784-791} -> Node 24
> [    0.000000] SRAT: PXM 25 -> APIC {800-807,816-823} -> Node 25
> [    0.000000] SRAT: PXM 26 -> APIC {832-839,848-855} -> Node 26
> [    0.000000] SRAT: PXM 27 -> APIC {864-871,880-887} -> Node 27
> [    0.000000] SRAT: PXM 28 -> APIC {896-903,912-919} -> Node 28
> [    0.000000] SRAT: PXM 29 -> APIC {928-935,944-951} -> Node 29
> [    0.000000] SRAT: PXM 30 -> APIC {960-967,976-983} -> Node 30
> [    0.000000] SRAT: PXM 31 -> APIC {992-999,1008-1015} -> Node 31
> [    0.000000] SRAT: PXM 32 -> APIC {1024-1031,1040-1047} -> Node 32
> [    0.000000] SRAT: PXM 33 -> APIC {1056-1063,1072-1079} -> Node 33
> [    0.000000] SRAT: PXM 34 -> APIC {1088-1095,1104-1111} -> Node 34
> [    0.000000] SRAT: PXM 35 -> APIC {1120-1127,1136-1143} -> Node 35
> [    0.000000] SRAT: PXM 36 -> APIC {1152-1159,1168-1175} -> Node 36
> [    0.000000] SRAT: PXM 37 -> APIC {1184-1191,1200-1207} -> Node 37
> [    0.000000] SRAT: PXM 38 -> APIC {1216-1223,1232-1239} -> Node 38
> [    0.000000] SRAT: PXM 39 -> APIC {1248-1255,1264-1271} -> Node 39                                                                                             

Looks good, 1272 lines reduced to 40.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Mike Travis]

David Rientjes wrote:
> On Wed, 28 Oct 2009, Mike Travis wrote:
> 
>> I don't understand the importance of this when the memory is given back
>> after the system starts up anyway...?
>>
> 
> Printing a list of apic ids longer than 128 characters would pollute the 
> kernel log and this upper bound will probably never be reached based on 
> the way apic ids are created for physical and logical processors: they are 
> normally reduced to ranges instead of comma seperated entities.

Ahh, ok, thanks.

Does that mean this 10,649 character line full of periods is illegal?

[  102.551570] Completing Region/Field/Buffer/Package initialization:
............... [long time later] .........
<4>Clocksource tsc unstable (delta = 4396383657849 ns)

I'm having trouble finding it.  Does it look familiar to anyone?

Thanks,
Mike

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Mike Travis wrote:

> > Printing a list of apic ids longer than 128 characters would pollute the
> > kernel log and this upper bound will probably never be reached based on the
> > way apic ids are created for physical and logical processors: they are
> > normally reduced to ranges instead of comma seperated entities.
> 
> Ahh, ok, thanks.
> 
> Does that mean this 10,649 character line full of periods is illegal?
> 

I'm not saying it would be illegal, merely that it would be harm 
readability.  Based on how apic id's are formed from processor ids, 
though, I think we're really talking about an upper limit (128) that will 
never be reached.

> [  102.551570] Completing Region/Field/Buffer/Package initialization:
> ............... [long time later] .........
> <4>Clocksource tsc unstable (delta = 4396383657849 ns)
> 
> I'm having trouble finding it.  Does it look familiar to anyone?
> 

It's debugging output from acpi_ns_initialize_objects() and each period is 
from acpi_ns_init_one_device().  You can suppress it by disabing 
CONFIG_ACPI_DEBUG.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Mike Travis]

David Rientjes wrote:
> On Wed, 28 Oct 2009, Mike Travis wrote:
> 
>>> Printing a list of apic ids longer than 128 characters would pollute the
>>> kernel log and this upper bound will probably never be reached based on the
>>> way apic ids are created for physical and logical processors: they are
>>> normally reduced to ranges instead of comma seperated entities.
>> Ahh, ok, thanks.
>>
>> Does that mean this 10,649 character line full of periods is illegal?
>>
> 
> I'm not saying it would be illegal, merely that it would be harm 
> readability.  Based on how apic id's are formed from processor ids, 
> though, I think we're really talking about an upper limit (128) that will 
> never be reached.

We actually have many, many more than that by adding on some extra bits
to the CPU's apicid.  These select which blade in the system to target.

> 
>> [  102.551570] Completing Region/Field/Buffer/Package initialization:
>> ............... [long time later] .........
>> <4>Clocksource tsc unstable (delta = 4396383657849 ns)
>>
>> I'm having trouble finding it.  Does it look familiar to anyone?
>>
> 
> It's debugging output from acpi_ns_initialize_objects() and each period is 
> from acpi_ns_init_one_device().  You can suppress it by disabing 
> CONFIG_ACPI_DEBUG.

Ahh, didn't know that was set in the (our) default config.  Is it normally
set by distros?  

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Wed, 28 Oct 2009, Mike Travis wrote:

> > I'm not saying it would be illegal, merely that it would be harm
> > readability.  Based on how apic id's are formed from processor ids, though,
> > I think we're really talking about an upper limit (128) that will never be
> > reached.
> 
> We actually have many, many more than that by adding on some extra bits
> to the CPU's apicid.  These select which blade in the system to target.
> 

Maybe I've been vague in my rationale for why this limit will probably 
never be reached.  The way apic ids are constructed, with physical and 
logical processor ids, it tends to lend itself to ranges where 
bitmap_scnlistprintf() can specify a large number of apic ids with 
relatively few ASCII characters because logical processors typically do 
not have differing pxms.  For us to reach the 128 character upper bound, 
scnlistprintf() would need to have many, many distinct ranges; your 
example showed two ranges per pxm (many more machines would have only a 
single range).  In other words, we're not predicting to have 
"1-2,4-6,8-9,11-13,15-17," etc, that we often have with nodemasks.

Re: [patch] x86: reduce srat verbosity in the kernel log

[Mike Travis]

David Rientjes wrote:
> On Wed, 28 Oct 2009, Mike Travis wrote:
> 
>>> I'm not saying it would be illegal, merely that it would be harm
>>> readability.  Based on how apic id's are formed from processor ids, though,
>>> I think we're really talking about an upper limit (128) that will never be
>>> reached.
>> We actually have many, many more than that by adding on some extra bits
>> to the CPU's apicid.  These select which blade in the system to target.
>>
> 
> Maybe I've been vague in my rationale for why this limit will probably 
> never be reached.  The way apic ids are constructed, with physical and 
> logical processor ids, it tends to lend itself to ranges where 
> bitmap_scnlistprintf() can specify a large number of apic ids with 
> relatively few ASCII characters because logical processors typically do 
> not have differing pxms.  For us to reach the 128 character upper bound, 
> scnlistprintf() would need to have many, many distinct ranges; your 
> example showed two ranges per pxm (many more machines would have only a 
> single range).  In other words, we're not predicting to have 
> "1-2,4-6,8-9,11-13,15-17," etc, that we often have with nodemasks.

Yes, you are correct.  (I was confused... ;-)

I believe the disjointed ranges came from the hyperthread cpus..?  Which if
true means there'll probably be as many distinct ranges as there are threads
per core?

Re: [patch] x86: reduce srat verbosity in the kernel log

[David Rientjes]

On Thu, 29 Oct 2009, Mike Travis wrote:

> I believe the disjointed ranges came from the hyperthread cpus..?  Which if
> true means there'll probably be as many distinct ranges as there are threads
> per core?
> 

Not necessarily, look at the first few lines of your new output:

	[    0.000000] SRAT: PXM 0 -> APIC {0-7,16-23} -> Node 0
	[    0.000000] SRAT: PXM 1 -> APIC {32-39,48-55} -> Node 1
	[    0.000000] SRAT: PXM 2 -> APIC {64-71,80-87} -> Node 2
	...

If those values are in hex, you have these apic id ranges:

	0x00-0x07, 0x10-0x17
	0x20-0x27, 0x30-0x37
	0x40-0x47, 0x50-0x57
	...

So it's most likely that each of the physical processors has eight logical 
processors (represented by the least significant three bits) and there are 
two physical processors (the more significant four bits) per node.

Re: [PATCH 3/8] SGI x86_64 UV: Limit the number of number of SRAT messages

[Cyrill Gorcunov]

[David Rientjes - Tue, Oct 27, 2009 at 12:45:00PM -0700]
...
| On Tue, 27 Oct 2009, Mike Travis wrote:
| +
| +void __init acpi_numa_print_srat_mapping(void)
| +{
| +	DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC);
| +	char apicid_list[MAX_LOCAL_APIC];

Hi David, I suppose 32K on stack is too much :)
(perhaps gcc will move it out of stack?)

...

	-- Cyrill

Re: [PATCH 3/8] SGI x86_64 UV: Limit the number of number of SRAT messages

[Mike Travis]

Cyrill Gorcunov wrote:
> [David Rientjes - Tue, Oct 27, 2009 at 12:45:00PM -0700]
> ...
> | On Tue, 27 Oct 2009, Mike Travis wrote:
> | +
> | +void __init acpi_numa_print_srat_mapping(void)
> | +{
> | +	DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC);
> | +	char apicid_list[MAX_LOCAL_APIC];
> 
> Hi David, I suppose 32K on stack is too much :)
> (perhaps gcc will move it out of stack?)
> 
> ...
> 
> 	-- Cyrill

Yeah, I missed that too on my first review.  (4k seems piddling
compared to 32k on the stack! ;-)

I moved them both to static and will be testing it shortly:

static DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC) __initdata;
static char apicid_list[MAX_LOCAL_APIC] __initdata;

Thanks,
Mike

Re: [PATCH 3/8] SGI x86_64 UV: Limit the number of number of SRAT messages

[Cyrill Gorcunov]

[Mike Travis - Tue, Oct 27, 2009 at 01:23:42PM -0700]
>
> I moved them both to static and will be testing it shortly:
>
> static DECLARE_BITMAP(apicid_map, MAX_LOCAL_APIC) __initdata;
> static char apicid_list[MAX_LOCAL_APIC] __initdata;
>
> Thanks,
> Mike
>

Great! David has just updated patch too.

	-- Cyrill