2009-10-23 23:38:34

by Mike Travis

[permalink] [raw]
Subject: [PATCH 4/8] 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 <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Thomas Renninger <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Alexey Dobriyan <[email protected]>
Cc: Myron Stowe <[email protected]>
Cc: Feng Tang <[email protected]>
Cc: Suresh Siddha <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Mike Travis <[email protected]>
---
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;


--


2009-10-24 03:33:27

by Bjorn Helgaas

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

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 <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Thomas Renninger <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Alexey Dobriyan <[email protected]>
> Cc: Myron Stowe <[email protected]>
> Cc: Feng Tang <[email protected]>
> Cc: Suresh Siddha <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Mike Travis <[email protected]>
> ---
> 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;
>
>

2009-10-26 18:15:10

by Mike Travis

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



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 <[email protected]>
>> Cc: Len Brown <[email protected]>
>> Cc: Thomas Renninger <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Alexey Dobriyan <[email protected]>
>> Cc: Myron Stowe <[email protected]>
>> Cc: Feng Tang <[email protected]>
>> Cc: Suresh Siddha <[email protected]>
>> Cc: Yinghai Lu <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Mike Travis <[email protected]>
>> ---
>> 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;
>>
>>

2009-10-26 21:25:52

by Mike Travis

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



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 <[email protected]>
>>> Cc: Len Brown <[email protected]>
>>> Cc: Thomas Renninger <[email protected]>
>>> Cc: Bjorn Helgaas <[email protected]>
>>> Cc: Alexey Dobriyan <[email protected]>
>>> Cc: Myron Stowe <[email protected]>
>>> Cc: Feng Tang <[email protected]>
>>> Cc: Suresh Siddha <[email protected]>
>>> Cc: Yinghai Lu <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Signed-off-by: Mike Travis <[email protected]>
>>> ---
>>> 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.

2009-10-26 20:46:13

by Thomas Renninger

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

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 <[email protected]>
> > Cc: Len Brown <[email protected]>
> > Cc: Thomas Renninger <[email protected]>
> > Cc: Bjorn Helgaas <[email protected]>
> > Cc: Alexey Dobriyan <[email protected]>
> > Cc: Myron Stowe <[email protected]>
> > Cc: Feng Tang <[email protected]>
> > Cc: Suresh Siddha <[email protected]>
> > Cc: Yinghai Lu <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Mike Travis <[email protected]>
> > ---
> > 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.

2009-10-27 15:27:16

by Mike Travis

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

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 <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Thomas Renninger <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Alexey Dobriyan <[email protected]>
Cc: Myron Stowe <[email protected]>
Cc: Feng Tang <[email protected]>
Cc: Suresh Siddha <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Mike Travis <[email protected]>
---
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;

2009-10-27 15:51:16

by Bjorn Helgaas

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

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 <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Thomas Renninger <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Alexey Dobriyan <[email protected]>
> Cc: Myron Stowe <[email protected]>
> Cc: Feng Tang <[email protected]>
> Cc: Suresh Siddha <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Mike Travis <[email protected]>
> ---
> 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