| Message ID | 1268145670-10094-2-git-send-email-chase.douglas@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Andy Whitcroft |
| Headers | show |
On 10 Mar 09, Chase Douglas wrote: > By default, ACPI resource conflict messages are logged at level > KERN_ERR. This is a rather high level for a message that is more a > warning than an indication of a real kernel error. Also, KERN_ERR level > messages can appear over some boot splash screens, and this message is > not serious enough to warrant such treatment. Thus, the log level has > been reduced to KERN_INFO. > > Also, cleanup message to use %pR resource printing format. This was rather interesting. So %pR replaces the entire [0x%llx-0x%llx] mess? Cool. > BugLink: http://bugs.launchpad.net/bugs/440470 > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > --- > drivers/acpi/osl.c | 14 ++++---------- > 1 files changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > index 7c1c59e..f89f141 100644 > --- a/drivers/acpi/osl.c > +++ b/drivers/acpi/osl.c > @@ -1151,16 +1151,10 @@ int acpi_check_resource_conflict(struct resource *res) > > if (clash) { > if (acpi_enforce_resources != ENFORCE_RESOURCES_NO) { > - printk("%sACPI: %s resource %s [0x%llx-0x%llx]" > - " conflicts with ACPI region %s" > - " [0x%llx-0x%llx]\n", > - acpi_enforce_resources == ENFORCE_RESOURCES_LAX > - ? KERN_WARNING : KERN_ERR, > - ioport ? "I/O" : "Memory", res->name, > - (long long) res->start, (long long) res->end, > - res_list_elem->name, > - (long long) res_list_elem->start, > - (long long) res_list_elem->end); > + printk(KERN_INFO "ACPI: resource %s %pR" > + " conflicts with ACPI region %s %pR\n", > + res->name, res, res_list_elem->name, > + res_list_elem); > if (acpi_enforce_resources == ENFORCE_RESOURCES_LAX) > printk(KERN_NOTICE "ACPI: This conflict may" > " cause random problems and system" > -- > 1.6.3.3 > The very next line is: if (acpi_enforce_resources == ENFORCE_RESOURCES_STRICT) return -EBUSY; And, acpi_enforce_resources is initialised to ENFORCE_RESOURCES_STRICT by default. So upstream considers this to be desirable behaviour. Should we really change the seriousness of the message in that case? Regards, Amit
On Tue, Mar 9, 2010 at 12:29 PM, Amit Kucheria <amit.kucheria@canonical.com> wrote: > On 10 Mar 09, Chase Douglas wrote: >> Also, cleanup message to use %pR resource printing format. > > This was rather interesting. So %pR replaces the entire [0x%llx-0x%llx] mess? > Cool. It also prints out whether the resource was a memory region or io port (I was also unaware until it was pointed out to me on linux-acpi). There's some documentation on this and other extensions at [1]. > The very next line is: > if (acpi_enforce_resources == ENFORCE_RESOURCES_STRICT) > return -EBUSY; > > And, acpi_enforce_resources is initialised to ENFORCE_RESOURCES_STRICT by > default. So upstream considers this to be desirable behaviour. Should we > really change the seriousness of the message in that case? First, Matthew Garret upstream has stated that he believes KERN_INFO to be reasonable. For a long time, the native hwmon drivers were accessing resources that were owned by the ACPI driver as well. This hasn't caused any issues yet because there weren't any ACPI drivers that accessed the regions. The preferred method would be to have ACPI drivers for all hwmon chips. Now, there's one ACPI driver for some ASUS motherboards. In 2.6.31 they decided to default to strictly locking out native hwmon drivers whenever they conflict with ACPI resources. Thus, this code prevents the hwmon from working and prints out a message at KERN_ERR level. In reality, most people don't care about hwmon drivers. However, they see this ugly error message at startup because KERN_ERR messages are overlayed on top of the boot splash (plymouth?). This is one of those "death by a thousand paper cuts" issues. If a user really wants hwmon drivers, they likely searched out how to do it and will see error messages in their dmesg and logs. It's also a well known issue and is listed in the lmsensors FAQ, along with the work around boot parameter. So basically it's just ugly and unnecessary for the normal user. And the "real" fix is to get ACPI drivers into the kernel in place of native hwmon drivers. We don't need a warning message over our boot splash just because there's a missing ACPI hwmon driver. [1] http://lxr.linux.no/linux+v2.6.33/lib/vsprintf.c#L1171
On Tue, Mar 09, 2010 at 09:41:10AM -0500, Chase Douglas wrote: > By default, ACPI resource conflict messages are logged at level > KERN_ERR. This is a rather high level for a message that is more a > warning than an indication of a real kernel error. Also, KERN_ERR level > messages can appear over some boot splash screens, and this message is > not serious enough to warrant such treatment. Thus, the log level has > been reduced to KERN_INFO. > > Also, cleanup message to use %pR resource printing format. > > BugLink: http://bugs.launchpad.net/bugs/440470 > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > --- > drivers/acpi/osl.c | 14 ++++---------- > 1 files changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > index 7c1c59e..f89f141 100644 > --- a/drivers/acpi/osl.c > +++ b/drivers/acpi/osl.c > @@ -1151,16 +1151,10 @@ int acpi_check_resource_conflict(struct resource *res) > > if (clash) { > if (acpi_enforce_resources != ENFORCE_RESOURCES_NO) { > - printk("%sACPI: %s resource %s [0x%llx-0x%llx]" > - " conflicts with ACPI region %s" > - " [0x%llx-0x%llx]\n", > - acpi_enforce_resources == ENFORCE_RESOURCES_LAX > - ? KERN_WARNING : KERN_ERR, > - ioport ? "I/O" : "Memory", res->name, > - (long long) res->start, (long long) res->end, > - res_list_elem->name, > - (long long) res_list_elem->start, > - (long long) res_list_elem->end); > + printk(KERN_INFO "ACPI: resource %s %pR" > + " conflicts with ACPI region %s %pR\n", > + res->name, res, res_list_elem->name, > + res_list_elem); > if (acpi_enforce_resources == ENFORCE_RESOURCES_LAX) > printk(KERN_NOTICE "ACPI: This conflict may" > " cause random problems and system" Sounds like upstream concurs on the level and this simplifies things generally. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 7c1c59e..f89f141 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -1151,16 +1151,10 @@ int acpi_check_resource_conflict(struct resource *res) if (clash) { if (acpi_enforce_resources != ENFORCE_RESOURCES_NO) { - printk("%sACPI: %s resource %s [0x%llx-0x%llx]" - " conflicts with ACPI region %s" - " [0x%llx-0x%llx]\n", - acpi_enforce_resources == ENFORCE_RESOURCES_LAX - ? KERN_WARNING : KERN_ERR, - ioport ? "I/O" : "Memory", res->name, - (long long) res->start, (long long) res->end, - res_list_elem->name, - (long long) res_list_elem->start, - (long long) res_list_elem->end); + printk(KERN_INFO "ACPI: resource %s %pR" + " conflicts with ACPI region %s %pR\n", + res->name, res, res_list_elem->name, + res_list_elem); if (acpi_enforce_resources == ENFORCE_RESOURCES_LAX) printk(KERN_NOTICE "ACPI: This conflict may" " cause random problems and system"
By default, ACPI resource conflict messages are logged at level KERN_ERR. This is a rather high level for a message that is more a warning than an indication of a real kernel error. Also, KERN_ERR level messages can appear over some boot splash screens, and this message is not serious enough to warrant such treatment. Thus, the log level has been reduced to KERN_INFO. Also, cleanup message to use %pR resource printing format. BugLink: http://bugs.launchpad.net/bugs/440470 Signed-off-by: Chase Douglas <chase.douglas@canonical.com> --- drivers/acpi/osl.c | 14 ++++---------- 1 files changed, 4 insertions(+), 10 deletions(-)