* [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-01 10:36 Hans de Goede
2017-03-02 2:03 ` [Devel] " Zheng, Lv
0 siblings, 1 reply; 18+ messages in thread
From: Hans de Goede @ 2017-03-01 10:36 UTC (permalink / raw)
To: Rafael J . Wysocki, Len Brown, Robert Moore, Lv Zheng
Cc: Hans de Goede, linux-acpi, devel
Honor the "Ignore errors while loading tables, get as many as possible"
comment in the tbxfload.c code and log any exceptions and errors during
loading extra tables as warnings.
This is important because many desktop and embedded applicance Linux
use-cases have a hard requirement of not showing any (scary) text
messages during system bootup, which get broken by errors reported
by attempts to load the extra tables, as messaged logged at a KERN_ERR
level are always shown even if the quiet kernel cmdline option is used.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/acpica/acglobal.h | 1 +
drivers/acpi/acpica/tbxfload.c | 4 +++-
drivers/acpi/acpica/uterror.c | 10 ++++++++--
drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
4 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index edbb42e..5ae9eee 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
+ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
/* Global handlers */
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index 1971cd7..b0c5478 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
}
/* Ignore errors while loading tables, get as many as possible */
+ acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
status = acpi_ns_load_table(i, acpi_gbl_root_node);
@@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
tables_loaded++;
}
}
+ acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
if (!tables_failed) {
ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
} else {
- ACPI_ERROR((AE_INFO,
+ ACPI_WARNING((AE_INFO,
"%u table load failures, %u successful",
tables_failed, tables_loaded));
diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
index 475932c..e01ff1f 100644
--- a/drivers/acpi/acpica/uterror.c
+++ b/drivers/acpi/acpica/uterror.c
@@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
char *name = NULL;
ACPI_MSG_REDIRECT_BEGIN;
- acpi_os_printf(ACPI_MSG_ERROR);
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
+ acpi_os_printf(ACPI_MSG_ERROR);
if (lookup_status == AE_BAD_CHARACTER) {
@@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
struct acpi_namespace_node *node = prefix_node;
ACPI_MSG_REDIRECT_BEGIN;
- acpi_os_printf(ACPI_MSG_ERROR);
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
+ acpi_os_printf(ACPI_MSG_ERROR);
if (path) {
status = acpi_ns_get_node(prefix_node, path,
diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
index d9f15cb..c8ca25af 100644
--- a/drivers/acpi/acpica/utxferror.c
+++ b/drivers/acpi/acpica/utxferror.c
@@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
va_list arg_list;
ACPI_MSG_REDIRECT_BEGIN;
- acpi_os_printf(ACPI_MSG_ERROR);
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
+ acpi_os_printf(ACPI_MSG_ERROR);
va_start(arg_list, format);
acpi_os_vprintf(format, arg_list);
@@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
va_list arg_list;
ACPI_MSG_REDIRECT_BEGIN;
-
- /* For AE_OK, just print the message */
-
- if (ACPI_SUCCESS(status)) {
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
acpi_os_printf(ACPI_MSG_EXCEPTION);
- } else {
- acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
- acpi_format_exception(status));
- }
+ /* For AE_OK, just print the message */
+ if (ACPI_FAILURE(status))
+ acpi_os_printf("%s, ", acpi_format_exception(status));
va_start(arg_list, format);
acpi_os_vprintf(format, arg_list);
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* RE: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-02 2:03 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-02 2:03 UTC (permalink / raw)
To: Hans de Goede, Rafael J . Wysocki, Len Brown, Moore, Robert
Cc: linux-acpi, devel
Hi,
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Honor the "Ignore errors while loading tables, get as many as possible"
> comment in the tbxfload.c code and log any exceptions and errors during
> loading extra tables as warnings.
>
> This is important because many desktop and embedded applicance Linux
> use-cases have a hard requirement of not showing any (scary) text
> messages during system bootup, which get broken by errors reported
> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> level are always shown even if the quiet kernel cmdline option is used.
This looks worse than comparing the tables.
So before determining what should be done for the check of the duplicated aml tables.
Let's skip this submission.
Thanks
Lv
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/acpi/acpica/acglobal.h | 1 +
> drivers/acpi/acpica/tbxfload.c | 4 +++-
> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> 4 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> index edbb42e..5ae9eee 100644
> --- a/drivers/acpi/acpica/acglobal.h
> +++ b/drivers/acpi/acpica/acglobal.h
> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
>
> /* Global handlers */
>
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index 1971cd7..b0c5478 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> }
>
> /* Ignore errors while loading tables, get as many as possible */
> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
>
> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> tables_loaded++;
> }
> }
> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
>
> if (!tables_failed) {
> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> } else {
> - ACPI_ERROR((AE_INFO,
> + ACPI_WARNING((AE_INFO,
> "%u table load failures, %u successful",
> tables_failed, tables_loaded));
>
> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> index 475932c..e01ff1f 100644
> --- a/drivers/acpi/acpica/uterror.c
> +++ b/drivers/acpi/acpica/uterror.c
> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> char *name = NULL;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> if (lookup_status == AE_BAD_CHARACTER) {
>
> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> struct acpi_namespace_node *node = prefix_node;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> if (path) {
> status = acpi_ns_get_node(prefix_node, path,
> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> index d9f15cb..c8ca25af 100644
> --- a/drivers/acpi/acpica/utxferror.c
> +++ b/drivers/acpi/acpica/utxferror.c
> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> va_list arg_list;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> va_start(arg_list, format);
> acpi_os_vprintf(format, arg_list);
> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> va_list arg_list;
>
> ACPI_MSG_REDIRECT_BEGIN;
> -
> - /* For AE_OK, just print the message */
> -
> - if (ACPI_SUCCESS(status)) {
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> acpi_os_printf(ACPI_MSG_EXCEPTION);
>
> - } else {
> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> - acpi_format_exception(status));
> - }
> + /* For AE_OK, just print the message */
> + if (ACPI_FAILURE(status))
> + acpi_os_printf("%s, ", acpi_format_exception(status));
>
> va_start(arg_list, format);
> acpi_os_vprintf(format, arg_list);
> --
> 2.9.3
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Devel] [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-02 2:03 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-02 2:03 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 5000 bytes --]
Hi,
> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Honor the "Ignore errors while loading tables, get as many as possible"
> comment in the tbxfload.c code and log any exceptions and errors during
> loading extra tables as warnings.
>
> This is important because many desktop and embedded applicance Linux
> use-cases have a hard requirement of not showing any (scary) text
> messages during system bootup, which get broken by errors reported
> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> level are always shown even if the quiet kernel cmdline option is used.
This looks worse than comparing the tables.
So before determining what should be done for the check of the duplicated aml tables.
Let's skip this submission.
Thanks
Lv
>
> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
> ---
> drivers/acpi/acpica/acglobal.h | 1 +
> drivers/acpi/acpica/tbxfload.c | 4 +++-
> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> 4 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> index edbb42e..5ae9eee 100644
> --- a/drivers/acpi/acpica/acglobal.h
> +++ b/drivers/acpi/acpica/acglobal.h
> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
>
> /* Global handlers */
>
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index 1971cd7..b0c5478 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> }
>
> /* Ignore errors while loading tables, get as many as possible */
> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
>
> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> tables_loaded++;
> }
> }
> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
>
> if (!tables_failed) {
> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> } else {
> - ACPI_ERROR((AE_INFO,
> + ACPI_WARNING((AE_INFO,
> "%u table load failures, %u successful",
> tables_failed, tables_loaded));
>
> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> index 475932c..e01ff1f 100644
> --- a/drivers/acpi/acpica/uterror.c
> +++ b/drivers/acpi/acpica/uterror.c
> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> char *name = NULL;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> if (lookup_status == AE_BAD_CHARACTER) {
>
> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> struct acpi_namespace_node *node = prefix_node;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> if (path) {
> status = acpi_ns_get_node(prefix_node, path,
> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> index d9f15cb..c8ca25af 100644
> --- a/drivers/acpi/acpica/utxferror.c
> +++ b/drivers/acpi/acpica/utxferror.c
> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> va_list arg_list;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> va_start(arg_list, format);
> acpi_os_vprintf(format, arg_list);
> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> va_list arg_list;
>
> ACPI_MSG_REDIRECT_BEGIN;
> -
> - /* For AE_OK, just print the message */
> -
> - if (ACPI_SUCCESS(status)) {
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> acpi_os_printf(ACPI_MSG_EXCEPTION);
>
> - } else {
> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> - acpi_format_exception(status));
> - }
> + /* For AE_OK, just print the message */
> + if (ACPI_FAILURE(status))
> + acpi_os_printf("%s, ", acpi_format_exception(status));
>
> va_start(arg_list, format);
> acpi_os_vprintf(format, arg_list);
> --
> 2.9.3
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
2017-03-02 2:03 ` [Devel] " Zheng, Lv
(?)
@ 2017-03-02 15:27 ` Hans de Goede
2017-03-03 2:48 ` [Devel] " Zheng, Lv
-1 siblings, 1 reply; 18+ messages in thread
From: Hans de Goede @ 2017-03-02 15:27 UTC (permalink / raw)
To: Zheng, Lv, Rafael J . Wysocki, Len Brown, Moore, Robert; +Cc: linux-acpi, devel
Hi,
On 02-03-17 03:03, Zheng, Lv wrote:
> Hi,
>
>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>
>> Honor the "Ignore errors while loading tables, get as many as possible"
>> comment in the tbxfload.c code and log any exceptions and errors during
>> loading extra tables as warnings.
>>
>> This is important because many desktop and embedded applicance Linux
>> use-cases have a hard requirement of not showing any (scary) text
>> messages during system bootup, which get broken by errors reported
>> by attempts to load the extra tables, as messaged logged at a KERN_ERR
>> level are always shown even if the quiet kernel cmdline option is used.
>
> This looks worse than comparing the tables.
This is really another patch entirely, it would also make the duplicate
table thing less of an issue, but even with a fix for the duplicate tables
we still need something like this to lower the log level of errors while
loading extra tables as that currently causes errors to be spewn on
way to many machines, e.g. on my main workstation I get:
[ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND (20160930/dswload-210)
[ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
[ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20160930/tbxfload-268)
[ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
(this is with the patch, before that this were all errors and exceptions).
Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
Regards,
Hans
> So before determining what should be done for the check of the duplicated aml tables.
> Let's skip this submission.
>
> Thanks
> Lv
>
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/acpi/acpica/acglobal.h | 1 +
>> drivers/acpi/acpica/tbxfload.c | 4 +++-
>> drivers/acpi/acpica/uterror.c | 10 ++++++++--
>> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
>> 4 files changed, 22 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
>> index edbb42e..5ae9eee 100644
>> --- a/drivers/acpi/acpica/acglobal.h
>> +++ b/drivers/acpi/acpica/acglobal.h
>> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
>> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
>> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
>> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
>> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
>>
>> /* Global handlers */
>>
>> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
>> index 1971cd7..b0c5478 100644
>> --- a/drivers/acpi/acpica/tbxfload.c
>> +++ b/drivers/acpi/acpica/tbxfload.c
>> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
>> }
>>
>> /* Ignore errors while loading tables, get as many as possible */
>> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
>>
>> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
>> status = acpi_ns_load_table(i, acpi_gbl_root_node);
>> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
>> tables_loaded++;
>> }
>> }
>> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
>>
>> if (!tables_failed) {
>> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
>> } else {
>> - ACPI_ERROR((AE_INFO,
>> + ACPI_WARNING((AE_INFO,
>> "%u table load failures, %u successful",
>> tables_failed, tables_loaded));
>>
>> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
>> index 475932c..e01ff1f 100644
>> --- a/drivers/acpi/acpica/uterror.c
>> +++ b/drivers/acpi/acpica/uterror.c
>> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
>> char *name = NULL;
>>
>> ACPI_MSG_REDIRECT_BEGIN;
>> - acpi_os_printf(ACPI_MSG_ERROR);
>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>> + acpi_os_printf(ACPI_MSG_WARNING);
>> + else
>> + acpi_os_printf(ACPI_MSG_ERROR);
>>
>> if (lookup_status == AE_BAD_CHARACTER) {
>>
>> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
>> struct acpi_namespace_node *node = prefix_node;
>>
>> ACPI_MSG_REDIRECT_BEGIN;
>> - acpi_os_printf(ACPI_MSG_ERROR);
>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>> + acpi_os_printf(ACPI_MSG_WARNING);
>> + else
>> + acpi_os_printf(ACPI_MSG_ERROR);
>>
>> if (path) {
>> status = acpi_ns_get_node(prefix_node, path,
>> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
>> index d9f15cb..c8ca25af 100644
>> --- a/drivers/acpi/acpica/utxferror.c
>> +++ b/drivers/acpi/acpica/utxferror.c
>> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
>> va_list arg_list;
>>
>> ACPI_MSG_REDIRECT_BEGIN;
>> - acpi_os_printf(ACPI_MSG_ERROR);
>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>> + acpi_os_printf(ACPI_MSG_WARNING);
>> + else
>> + acpi_os_printf(ACPI_MSG_ERROR);
>>
>> va_start(arg_list, format);
>> acpi_os_vprintf(format, arg_list);
>> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
>> va_list arg_list;
>>
>> ACPI_MSG_REDIRECT_BEGIN;
>> -
>> - /* For AE_OK, just print the message */
>> -
>> - if (ACPI_SUCCESS(status)) {
>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>> + acpi_os_printf(ACPI_MSG_WARNING);
>> + else
>> acpi_os_printf(ACPI_MSG_EXCEPTION);
>>
>> - } else {
>> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
>> - acpi_format_exception(status));
>> - }
>> + /* For AE_OK, just print the message */
>> + if (ACPI_FAILURE(status))
>> + acpi_os_printf("%s, ", acpi_format_exception(status));
>>
>> va_start(arg_list, format);
>> acpi_os_vprintf(format, arg_list);
>> --
>> 2.9.3
>
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-03 2:48 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-03 2:48 UTC (permalink / raw)
To: Hans de Goede, Rafael J . Wysocki, Len Brown, Moore, Robert
Cc: linux-acpi, devel
Hi,
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 02-03-17 03:03, Zheng, Lv wrote:
> > Hi,
> >
> >> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Honor the "Ignore errors while loading tables, get as many as possible"
> >> comment in the tbxfload.c code and log any exceptions and errors during
> >> loading extra tables as warnings.
> >>
> >> This is important because many desktop and embedded applicance Linux
> >> use-cases have a hard requirement of not showing any (scary) text
> >> messages during system bootup, which get broken by errors reported
> >> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> >> level are always shown even if the quiet kernel cmdline option is used.
> >
> > This looks worse than comparing the tables.
>
> This is really another patch entirely, it would also make the duplicate
> table thing less of an issue, but even with a fix for the duplicate tables
> we still need something like this to lower the log level of errors while
> loading extra tables as that currently causes errors to be spewn on
> way to many machines, e.g. on my main workstation I get:
>
> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
> (20160930/dswload-210)
> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20160930/tbxfload-268)
> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
However some of them indicating a real issue that can be fixed by:
1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer dependent on other fixes)
2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
It's not far away than actually solving the real issues.
So why do we use a one-size-fits-all solution for such kind of errors?
Thanks and best regards
Lv
>
> (this is with the patch, before that this were all errors and exceptions).
>
> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
>
> Regards,
>
> Hans
>
>
>
> > So before determining what should be done for the check of the duplicated aml tables.
> > Let's skip this submission.
> >
> > Thanks
> > Lv
> >
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >> drivers/acpi/acpica/acglobal.h | 1 +
> >> drivers/acpi/acpica/tbxfload.c | 4 +++-
> >> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> >> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> >> 4 files changed, 22 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> >> index edbb42e..5ae9eee 100644
> >> --- a/drivers/acpi/acpica/acglobal.h
> >> +++ b/drivers/acpi/acpica/acglobal.h
> >> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> >> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> >> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> >> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> >> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
> >>
> >> /* Global handlers */
> >>
> >> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> >> index 1971cd7..b0c5478 100644
> >> --- a/drivers/acpi/acpica/tbxfload.c
> >> +++ b/drivers/acpi/acpica/tbxfload.c
> >> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> >> }
> >>
> >> /* Ignore errors while loading tables, get as many as possible */
> >> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
> >>
> >> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> >> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> >> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> >> tables_loaded++;
> >> }
> >> }
> >> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
> >>
> >> if (!tables_failed) {
> >> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> >> } else {
> >> - ACPI_ERROR((AE_INFO,
> >> + ACPI_WARNING((AE_INFO,
> >> "%u table load failures, %u successful",
> >> tables_failed, tables_loaded));
> >>
> >> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> >> index 475932c..e01ff1f 100644
> >> --- a/drivers/acpi/acpica/uterror.c
> >> +++ b/drivers/acpi/acpica/uterror.c
> >> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> >> char *name = NULL;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> - acpi_os_printf(ACPI_MSG_ERROR);
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> + acpi_os_printf(ACPI_MSG_ERROR);
> >>
> >> if (lookup_status == AE_BAD_CHARACTER) {
> >>
> >> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> >> struct acpi_namespace_node *node = prefix_node;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> - acpi_os_printf(ACPI_MSG_ERROR);
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> + acpi_os_printf(ACPI_MSG_ERROR);
> >>
> >> if (path) {
> >> status = acpi_ns_get_node(prefix_node, path,
> >> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> >> index d9f15cb..c8ca25af 100644
> >> --- a/drivers/acpi/acpica/utxferror.c
> >> +++ b/drivers/acpi/acpica/utxferror.c
> >> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> >> va_list arg_list;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> - acpi_os_printf(ACPI_MSG_ERROR);
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> + acpi_os_printf(ACPI_MSG_ERROR);
> >>
> >> va_start(arg_list, format);
> >> acpi_os_vprintf(format, arg_list);
> >> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> >> va_list arg_list;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> -
> >> - /* For AE_OK, just print the message */
> >> -
> >> - if (ACPI_SUCCESS(status)) {
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> acpi_os_printf(ACPI_MSG_EXCEPTION);
> >>
> >> - } else {
> >> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> >> - acpi_format_exception(status));
> >> - }
> >> + /* For AE_OK, just print the message */
> >> + if (ACPI_FAILURE(status))
> >> + acpi_os_printf("%s, ", acpi_format_exception(status));
> >>
> >> va_start(arg_list, format);
> >> acpi_os_vprintf(format, arg_list);
> >> --
> >> 2.9.3
> >
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Devel] [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-03 2:48 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-03 2:48 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 7007 bytes --]
Hi,
> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 02-03-17 03:03, Zheng, Lv wrote:
> > Hi,
> >
> >> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> >> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Honor the "Ignore errors while loading tables, get as many as possible"
> >> comment in the tbxfload.c code and log any exceptions and errors during
> >> loading extra tables as warnings.
> >>
> >> This is important because many desktop and embedded applicance Linux
> >> use-cases have a hard requirement of not showing any (scary) text
> >> messages during system bootup, which get broken by errors reported
> >> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> >> level are always shown even if the quiet kernel cmdline option is used.
> >
> > This looks worse than comparing the tables.
>
> This is really another patch entirely, it would also make the duplicate
> table thing less of an issue, but even with a fix for the duplicate tables
> we still need something like this to lower the log level of errors while
> loading extra tables as that currently causes errors to be spewn on
> way to many machines, e.g. on my main workstation I get:
>
> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
> (20160930/dswload-210)
> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20160930/tbxfload-268)
> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
However some of them indicating a real issue that can be fixed by:
1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer dependent on other fixes)
2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
It's not far away than actually solving the real issues.
So why do we use a one-size-fits-all solution for such kind of errors?
Thanks and best regards
Lv
>
> (this is with the patch, before that this were all errors and exceptions).
>
> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
>
> Regards,
>
> Hans
>
>
>
> > So before determining what should be done for the check of the duplicated aml tables.
> > Let's skip this submission.
> >
> > Thanks
> > Lv
> >
> >>
> >> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
> >> ---
> >> drivers/acpi/acpica/acglobal.h | 1 +
> >> drivers/acpi/acpica/tbxfload.c | 4 +++-
> >> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> >> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> >> 4 files changed, 22 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> >> index edbb42e..5ae9eee 100644
> >> --- a/drivers/acpi/acpica/acglobal.h
> >> +++ b/drivers/acpi/acpica/acglobal.h
> >> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> >> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> >> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> >> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> >> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
> >>
> >> /* Global handlers */
> >>
> >> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> >> index 1971cd7..b0c5478 100644
> >> --- a/drivers/acpi/acpica/tbxfload.c
> >> +++ b/drivers/acpi/acpica/tbxfload.c
> >> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> >> }
> >>
> >> /* Ignore errors while loading tables, get as many as possible */
> >> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
> >>
> >> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> >> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> >> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> >> tables_loaded++;
> >> }
> >> }
> >> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
> >>
> >> if (!tables_failed) {
> >> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> >> } else {
> >> - ACPI_ERROR((AE_INFO,
> >> + ACPI_WARNING((AE_INFO,
> >> "%u table load failures, %u successful",
> >> tables_failed, tables_loaded));
> >>
> >> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> >> index 475932c..e01ff1f 100644
> >> --- a/drivers/acpi/acpica/uterror.c
> >> +++ b/drivers/acpi/acpica/uterror.c
> >> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> >> char *name = NULL;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> - acpi_os_printf(ACPI_MSG_ERROR);
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> + acpi_os_printf(ACPI_MSG_ERROR);
> >>
> >> if (lookup_status == AE_BAD_CHARACTER) {
> >>
> >> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> >> struct acpi_namespace_node *node = prefix_node;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> - acpi_os_printf(ACPI_MSG_ERROR);
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> + acpi_os_printf(ACPI_MSG_ERROR);
> >>
> >> if (path) {
> >> status = acpi_ns_get_node(prefix_node, path,
> >> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> >> index d9f15cb..c8ca25af 100644
> >> --- a/drivers/acpi/acpica/utxferror.c
> >> +++ b/drivers/acpi/acpica/utxferror.c
> >> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> >> va_list arg_list;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> - acpi_os_printf(ACPI_MSG_ERROR);
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> + acpi_os_printf(ACPI_MSG_ERROR);
> >>
> >> va_start(arg_list, format);
> >> acpi_os_vprintf(format, arg_list);
> >> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> >> va_list arg_list;
> >>
> >> ACPI_MSG_REDIRECT_BEGIN;
> >> -
> >> - /* For AE_OK, just print the message */
> >> -
> >> - if (ACPI_SUCCESS(status)) {
> >> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >> + acpi_os_printf(ACPI_MSG_WARNING);
> >> + else
> >> acpi_os_printf(ACPI_MSG_EXCEPTION);
> >>
> >> - } else {
> >> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> >> - acpi_format_exception(status));
> >> - }
> >> + /* For AE_OK, just print the message */
> >> + if (ACPI_FAILURE(status))
> >> + acpi_os_printf("%s, ", acpi_format_exception(status));
> >>
> >> va_start(arg_list, format);
> >> acpi_os_vprintf(format, arg_list);
> >> --
> >> 2.9.3
> >
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
2017-03-03 2:48 ` [Devel] " Zheng, Lv
(?)
@ 2017-03-03 13:55 ` Hans de Goede
2017-03-13 9:52 ` [Devel] " Zheng, Lv
-1 siblings, 1 reply; 18+ messages in thread
From: Hans de Goede @ 2017-03-03 13:55 UTC (permalink / raw)
To: Zheng, Lv, Rafael J . Wysocki, Len Brown, Moore, Robert; +Cc: linux-acpi, devel
Hi,
On 03-03-17 03:48, Zheng, Lv wrote:
> Hi,
>
>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>
>> Hi,
>>
>> On 02-03-17 03:03, Zheng, Lv wrote:
>>> Hi,
>>>
>>>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>>>> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>>>
>>>> Honor the "Ignore errors while loading tables, get as many as possible"
>>>> comment in the tbxfload.c code and log any exceptions and errors during
>>>> loading extra tables as warnings.
>>>>
>>>> This is important because many desktop and embedded applicance Linux
>>>> use-cases have a hard requirement of not showing any (scary) text
>>>> messages during system bootup, which get broken by errors reported
>>>> by attempts to load the extra tables, as messaged logged at a KERN_ERR
>>>> level are always shown even if the quiet kernel cmdline option is used.
>>>
>>> This looks worse than comparing the tables.
>>
>> This is really another patch entirely, it would also make the duplicate
>> table thing less of an issue, but even with a fix for the duplicate tables
>> we still need something like this to lower the log level of errors while
>> loading extra tables as that currently causes errors to be spewn on
>> way to many machines, e.g. on my main workstation I get:
>>
>> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
>> (20160930/dswload-210)
>> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
>> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20160930/tbxfload-268)
>> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
>
> However some of them indicating a real issue that can be fixed by:
> 1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer dependent on other fixes)
> 2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
> It's not far away than actually solving the real issues.
> So why do we use a one-size-fits-all solution for such kind of errors?
Ok, I agree that doing proper fixes for errors like this is better,
so lets drop this patch.
Regards,
Hans
>
> Thanks and best regards
> Lv
>
>
>>
>> (this is with the patch, before that this were all errors and exceptions).
>>
>> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>> So before determining what should be done for the check of the duplicated aml tables.
>>> Let's skip this submission.
>>>
>>> Thanks
>>> Lv
>>>
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>> drivers/acpi/acpica/acglobal.h | 1 +
>>>> drivers/acpi/acpica/tbxfload.c | 4 +++-
>>>> drivers/acpi/acpica/uterror.c | 10 ++++++++--
>>>> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
>>>> 4 files changed, 22 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
>>>> index edbb42e..5ae9eee 100644
>>>> --- a/drivers/acpi/acpica/acglobal.h
>>>> +++ b/drivers/acpi/acpica/acglobal.h
>>>> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
>>>> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
>>>> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
>>>>
>>>> /* Global handlers */
>>>>
>>>> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
>>>> index 1971cd7..b0c5478 100644
>>>> --- a/drivers/acpi/acpica/tbxfload.c
>>>> +++ b/drivers/acpi/acpica/tbxfload.c
>>>> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
>>>> }
>>>>
>>>> /* Ignore errors while loading tables, get as many as possible */
>>>> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
>>>>
>>>> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
>>>> status = acpi_ns_load_table(i, acpi_gbl_root_node);
>>>> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
>>>> tables_loaded++;
>>>> }
>>>> }
>>>> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
>>>>
>>>> if (!tables_failed) {
>>>> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
>>>> } else {
>>>> - ACPI_ERROR((AE_INFO,
>>>> + ACPI_WARNING((AE_INFO,
>>>> "%u table load failures, %u successful",
>>>> tables_failed, tables_loaded));
>>>>
>>>> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
>>>> index 475932c..e01ff1f 100644
>>>> --- a/drivers/acpi/acpica/uterror.c
>>>> +++ b/drivers/acpi/acpica/uterror.c
>>>> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
>>>> char *name = NULL;
>>>>
>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>> - acpi_os_printf(ACPI_MSG_ERROR);
>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>> + else
>>>> + acpi_os_printf(ACPI_MSG_ERROR);
>>>>
>>>> if (lookup_status == AE_BAD_CHARACTER) {
>>>>
>>>> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
>>>> struct acpi_namespace_node *node = prefix_node;
>>>>
>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>> - acpi_os_printf(ACPI_MSG_ERROR);
>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>> + else
>>>> + acpi_os_printf(ACPI_MSG_ERROR);
>>>>
>>>> if (path) {
>>>> status = acpi_ns_get_node(prefix_node, path,
>>>> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
>>>> index d9f15cb..c8ca25af 100644
>>>> --- a/drivers/acpi/acpica/utxferror.c
>>>> +++ b/drivers/acpi/acpica/utxferror.c
>>>> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
>>>> va_list arg_list;
>>>>
>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>> - acpi_os_printf(ACPI_MSG_ERROR);
>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>> + else
>>>> + acpi_os_printf(ACPI_MSG_ERROR);
>>>>
>>>> va_start(arg_list, format);
>>>> acpi_os_vprintf(format, arg_list);
>>>> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
>>>> va_list arg_list;
>>>>
>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>> -
>>>> - /* For AE_OK, just print the message */
>>>> -
>>>> - if (ACPI_SUCCESS(status)) {
>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>> + else
>>>> acpi_os_printf(ACPI_MSG_EXCEPTION);
>>>>
>>>> - } else {
>>>> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
>>>> - acpi_format_exception(status));
>>>> - }
>>>> + /* For AE_OK, just print the message */
>>>> + if (ACPI_FAILURE(status))
>>>> + acpi_os_printf("%s, ", acpi_format_exception(status));
>>>>
>>>> va_start(arg_list, format);
>>>> acpi_os_vprintf(format, arg_list);
>>>> --
>>>> 2.9.3
>>>
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-13 9:52 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-13 9:52 UTC (permalink / raw)
To: Hans de Goede, Rafael J . Wysocki, Len Brown, Moore, Robert
Cc: linux-acpi, devel
Hi, Hans
For log level issue, is this fix useful for you?
https://github.com/acpica/acpica/pull/121/commits/a505d3942
Thanks and best regards
Lv
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 03-03-17 03:48, Zheng, Lv wrote:
> > Hi,
> >
> >> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Hi,
> >>
> >> On 02-03-17 03:03, Zheng, Lv wrote:
> >>> Hi,
> >>>
> >>>> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >>>> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>>>
> >>>> Honor the "Ignore errors while loading tables, get as many as possible"
> >>>> comment in the tbxfload.c code and log any exceptions and errors during
> >>>> loading extra tables as warnings.
> >>>>
> >>>> This is important because many desktop and embedded applicance Linux
> >>>> use-cases have a hard requirement of not showing any (scary) text
> >>>> messages during system bootup, which get broken by errors reported
> >>>> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> >>>> level are always shown even if the quiet kernel cmdline option is used.
> >>>
> >>> This looks worse than comparing the tables.
> >>
> >> This is really another patch entirely, it would also make the duplicate
> >> table thing less of an issue, but even with a fix for the duplicate tables
> >> we still need something like this to lower the log level of errors while
> >> loading extra tables as that currently causes errors to be spewn on
> >> way to many machines, e.g. on my main workstation I get:
> >>
> >> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
> >> (20160930/dswload-210)
> >> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
> >> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20160930/tbxfload-
> 268)
> >> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
> >
> > However some of them indicating a real issue that can be fixed by:
> > 1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer
> dependent on other fixes)
> > 2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
> > It's not far away than actually solving the real issues.
> > So why do we use a one-size-fits-all solution for such kind of errors?
>
> Ok, I agree that doing proper fixes for errors like this is better,
> so lets drop this patch.
>
> Regards,
>
> Hans
>
>
> >
> > Thanks and best regards
> > Lv
> >
> >
> >>
> >> (this is with the patch, before that this were all errors and exceptions).
> >>
> >> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>
> >>> So before determining what should be done for the check of the duplicated aml tables.
> >>> Let's skip this submission.
> >>>
> >>> Thanks
> >>> Lv
> >>>
> >>>>
> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>>> ---
> >>>> drivers/acpi/acpica/acglobal.h | 1 +
> >>>> drivers/acpi/acpica/tbxfload.c | 4 +++-
> >>>> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> >>>> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> >>>> 4 files changed, 22 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> >>>> index edbb42e..5ae9eee 100644
> >>>> --- a/drivers/acpi/acpica/acglobal.h
> >>>> +++ b/drivers/acpi/acpica/acglobal.h
> >>>> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> >>>> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> >>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> >>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> >>>> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
> >>>>
> >>>> /* Global handlers */
> >>>>
> >>>> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> >>>> index 1971cd7..b0c5478 100644
> >>>> --- a/drivers/acpi/acpica/tbxfload.c
> >>>> +++ b/drivers/acpi/acpica/tbxfload.c
> >>>> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> >>>> }
> >>>>
> >>>> /* Ignore errors while loading tables, get as many as possible */
> >>>> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
> >>>>
> >>>> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> >>>> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> >>>> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> >>>> tables_loaded++;
> >>>> }
> >>>> }
> >>>> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
> >>>>
> >>>> if (!tables_failed) {
> >>>> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> >>>> } else {
> >>>> - ACPI_ERROR((AE_INFO,
> >>>> + ACPI_WARNING((AE_INFO,
> >>>> "%u table load failures, %u successful",
> >>>> tables_failed, tables_loaded));
> >>>>
> >>>> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> >>>> index 475932c..e01ff1f 100644
> >>>> --- a/drivers/acpi/acpica/uterror.c
> >>>> +++ b/drivers/acpi/acpica/uterror.c
> >>>> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> >>>> char *name = NULL;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>
> >>>> if (lookup_status == AE_BAD_CHARACTER) {
> >>>>
> >>>> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> >>>> struct acpi_namespace_node *node = prefix_node;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>
> >>>> if (path) {
> >>>> status = acpi_ns_get_node(prefix_node, path,
> >>>> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> >>>> index d9f15cb..c8ca25af 100644
> >>>> --- a/drivers/acpi/acpica/utxferror.c
> >>>> +++ b/drivers/acpi/acpica/utxferror.c
> >>>> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> >>>> va_list arg_list;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>
> >>>> va_start(arg_list, format);
> >>>> acpi_os_vprintf(format, arg_list);
> >>>> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> >>>> va_list arg_list;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> -
> >>>> - /* For AE_OK, just print the message */
> >>>> -
> >>>> - if (ACPI_SUCCESS(status)) {
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> acpi_os_printf(ACPI_MSG_EXCEPTION);
> >>>>
> >>>> - } else {
> >>>> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> >>>> - acpi_format_exception(status));
> >>>> - }
> >>>> + /* For AE_OK, just print the message */
> >>>> + if (ACPI_FAILURE(status))
> >>>> + acpi_os_printf("%s, ", acpi_format_exception(status));
> >>>>
> >>>> va_start(arg_list, format);
> >>>> acpi_os_vprintf(format, arg_list);
> >>>> --
> >>>> 2.9.3
> >>>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Devel] [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-13 9:52 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-13 9:52 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 7914 bytes --]
Hi, Hans
For log level issue, is this fix useful for you?
https://github.com/acpica/acpica/pull/121/commits/a505d3942
Thanks and best regards
Lv
> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 03-03-17 03:48, Zheng, Lv wrote:
> > Hi,
> >
> >> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> >> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Hi,
> >>
> >> On 02-03-17 03:03, Zheng, Lv wrote:
> >>> Hi,
> >>>
> >>>> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> >>>> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>>>
> >>>> Honor the "Ignore errors while loading tables, get as many as possible"
> >>>> comment in the tbxfload.c code and log any exceptions and errors during
> >>>> loading extra tables as warnings.
> >>>>
> >>>> This is important because many desktop and embedded applicance Linux
> >>>> use-cases have a hard requirement of not showing any (scary) text
> >>>> messages during system bootup, which get broken by errors reported
> >>>> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> >>>> level are always shown even if the quiet kernel cmdline option is used.
> >>>
> >>> This looks worse than comparing the tables.
> >>
> >> This is really another patch entirely, it would also make the duplicate
> >> table thing less of an issue, but even with a fix for the duplicate tables
> >> we still need something like this to lower the log level of errors while
> >> loading extra tables as that currently causes errors to be spewn on
> >> way to many machines, e.g. on my main workstation I get:
> >>
> >> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
> >> (20160930/dswload-210)
> >> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
> >> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20160930/tbxfload-
> 268)
> >> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
> >
> > However some of them indicating a real issue that can be fixed by:
> > 1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer
> dependent on other fixes)
> > 2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
> > It's not far away than actually solving the real issues.
> > So why do we use a one-size-fits-all solution for such kind of errors?
>
> Ok, I agree that doing proper fixes for errors like this is better,
> so lets drop this patch.
>
> Regards,
>
> Hans
>
>
> >
> > Thanks and best regards
> > Lv
> >
> >
> >>
> >> (this is with the patch, before that this were all errors and exceptions).
> >>
> >> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>
> >>> So before determining what should be done for the check of the duplicated aml tables.
> >>> Let's skip this submission.
> >>>
> >>> Thanks
> >>> Lv
> >>>
> >>>>
> >>>> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
> >>>> ---
> >>>> drivers/acpi/acpica/acglobal.h | 1 +
> >>>> drivers/acpi/acpica/tbxfload.c | 4 +++-
> >>>> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> >>>> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> >>>> 4 files changed, 22 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> >>>> index edbb42e..5ae9eee 100644
> >>>> --- a/drivers/acpi/acpica/acglobal.h
> >>>> +++ b/drivers/acpi/acpica/acglobal.h
> >>>> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> >>>> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> >>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> >>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> >>>> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
> >>>>
> >>>> /* Global handlers */
> >>>>
> >>>> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> >>>> index 1971cd7..b0c5478 100644
> >>>> --- a/drivers/acpi/acpica/tbxfload.c
> >>>> +++ b/drivers/acpi/acpica/tbxfload.c
> >>>> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> >>>> }
> >>>>
> >>>> /* Ignore errors while loading tables, get as many as possible */
> >>>> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
> >>>>
> >>>> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> >>>> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> >>>> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> >>>> tables_loaded++;
> >>>> }
> >>>> }
> >>>> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
> >>>>
> >>>> if (!tables_failed) {
> >>>> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> >>>> } else {
> >>>> - ACPI_ERROR((AE_INFO,
> >>>> + ACPI_WARNING((AE_INFO,
> >>>> "%u table load failures, %u successful",
> >>>> tables_failed, tables_loaded));
> >>>>
> >>>> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> >>>> index 475932c..e01ff1f 100644
> >>>> --- a/drivers/acpi/acpica/uterror.c
> >>>> +++ b/drivers/acpi/acpica/uterror.c
> >>>> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> >>>> char *name = NULL;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>
> >>>> if (lookup_status == AE_BAD_CHARACTER) {
> >>>>
> >>>> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> >>>> struct acpi_namespace_node *node = prefix_node;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>
> >>>> if (path) {
> >>>> status = acpi_ns_get_node(prefix_node, path,
> >>>> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> >>>> index d9f15cb..c8ca25af 100644
> >>>> --- a/drivers/acpi/acpica/utxferror.c
> >>>> +++ b/drivers/acpi/acpica/utxferror.c
> >>>> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> >>>> va_list arg_list;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>
> >>>> va_start(arg_list, format);
> >>>> acpi_os_vprintf(format, arg_list);
> >>>> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> >>>> va_list arg_list;
> >>>>
> >>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>> -
> >>>> - /* For AE_OK, just print the message */
> >>>> -
> >>>> - if (ACPI_SUCCESS(status)) {
> >>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>> + else
> >>>> acpi_os_printf(ACPI_MSG_EXCEPTION);
> >>>>
> >>>> - } else {
> >>>> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> >>>> - acpi_format_exception(status));
> >>>> - }
> >>>> + /* For AE_OK, just print the message */
> >>>> + if (ACPI_FAILURE(status))
> >>>> + acpi_os_printf("%s, ", acpi_format_exception(status));
> >>>>
> >>>> va_start(arg_list, format);
> >>>> acpi_os_vprintf(format, arg_list);
> >>>> --
> >>>> 2.9.3
> >>>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
2017-03-13 9:52 ` [Devel] " Zheng, Lv
(?)
@ 2017-03-13 10:06 ` Hans de Goede
2017-03-14 8:15 ` [Devel] " Zheng, Lv
-1 siblings, 1 reply; 18+ messages in thread
From: Hans de Goede @ 2017-03-13 10:06 UTC (permalink / raw)
To: Zheng, Lv, Rafael J . Wysocki, Len Brown, Moore, Robert; +Cc: linux-acpi, devel
Hi,
On 13-03-17 10:52, Zheng, Lv wrote:
> Hi, Hans
>
> For log level issue, is this fix useful for you?
> https://github.com/acpica/acpica/pull/121/commits/a505d3942
That fixes reloading already loaded tables, the problem I'm
getting errors about its loading a different table with identical
contents.
I will look into your suggestion to do something similar to
AcpiTbInstallStandardTable using AcpiTbCompareTables for the
SSDT tables. I will send a new patch when I can make some time
to look into this.
Regards,
Hans
>
> Thanks and best regards
> Lv
>
>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>
>> Hi,
>>
>> On 03-03-17 03:48, Zheng, Lv wrote:
>>> Hi,
>>>
>>>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>>>> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>>>
>>>> Hi,
>>>>
>>>> On 02-03-17 03:03, Zheng, Lv wrote:
>>>>> Hi,
>>>>>
>>>>>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>>>>>> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>>>>>
>>>>>> Honor the "Ignore errors while loading tables, get as many as possible"
>>>>>> comment in the tbxfload.c code and log any exceptions and errors during
>>>>>> loading extra tables as warnings.
>>>>>>
>>>>>> This is important because many desktop and embedded applicance Linux
>>>>>> use-cases have a hard requirement of not showing any (scary) text
>>>>>> messages during system bootup, which get broken by errors reported
>>>>>> by attempts to load the extra tables, as messaged logged at a KERN_ERR
>>>>>> level are always shown even if the quiet kernel cmdline option is used.
>>>>>
>>>>> This looks worse than comparing the tables.
>>>>
>>>> This is really another patch entirely, it would also make the duplicate
>>>> table thing less of an issue, but even with a fix for the duplicate tables
>>>> we still need something like this to lower the log level of errors while
>>>> loading extra tables as that currently causes errors to be spewn on
>>>> way to many machines, e.g. on my main workstation I get:
>>>>
>>>> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
>>>> (20160930/dswload-210)
>>>> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
>>>> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table (20160930/tbxfload-
>> 268)
>>>> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
>>>
>>> However some of them indicating a real issue that can be fixed by:
>>> 1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer
>> dependent on other fixes)
>>> 2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
>>> It's not far away than actually solving the real issues.
>>> So why do we use a one-size-fits-all solution for such kind of errors?
>>
>> Ok, I agree that doing proper fixes for errors like this is better,
>> so lets drop this patch.
>>
>> Regards,
>>
>> Hans
>>
>>
>>>
>>> Thanks and best regards
>>> Lv
>>>
>>>
>>>>
>>>> (this is with the patch, before that this were all errors and exceptions).
>>>>
>>>> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>>
>>>>
>>>>
>>>>> So before determining what should be done for the check of the duplicated aml tables.
>>>>> Let's skip this submission.
>>>>>
>>>>> Thanks
>>>>> Lv
>>>>>
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>> ---
>>>>>> drivers/acpi/acpica/acglobal.h | 1 +
>>>>>> drivers/acpi/acpica/tbxfload.c | 4 +++-
>>>>>> drivers/acpi/acpica/uterror.c | 10 ++++++++--
>>>>>> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
>>>>>> 4 files changed, 22 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
>>>>>> index edbb42e..5ae9eee 100644
>>>>>> --- a/drivers/acpi/acpica/acglobal.h
>>>>>> +++ b/drivers/acpi/acpica/acglobal.h
>>>>>> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
>>>>>> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
>>>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
>>>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
>>>>>> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
>>>>>>
>>>>>> /* Global handlers */
>>>>>>
>>>>>> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
>>>>>> index 1971cd7..b0c5478 100644
>>>>>> --- a/drivers/acpi/acpica/tbxfload.c
>>>>>> +++ b/drivers/acpi/acpica/tbxfload.c
>>>>>> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
>>>>>> }
>>>>>>
>>>>>> /* Ignore errors while loading tables, get as many as possible */
>>>>>> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
>>>>>>
>>>>>> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
>>>>>> status = acpi_ns_load_table(i, acpi_gbl_root_node);
>>>>>> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
>>>>>> tables_loaded++;
>>>>>> }
>>>>>> }
>>>>>> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
>>>>>>
>>>>>> if (!tables_failed) {
>>>>>> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
>>>>>> } else {
>>>>>> - ACPI_ERROR((AE_INFO,
>>>>>> + ACPI_WARNING((AE_INFO,
>>>>>> "%u table load failures, %u successful",
>>>>>> tables_failed, tables_loaded));
>>>>>>
>>>>>> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
>>>>>> index 475932c..e01ff1f 100644
>>>>>> --- a/drivers/acpi/acpica/uterror.c
>>>>>> +++ b/drivers/acpi/acpica/uterror.c
>>>>>> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
>>>>>> char *name = NULL;
>>>>>>
>>>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
>>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>>>> + else
>>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
>>>>>>
>>>>>> if (lookup_status == AE_BAD_CHARACTER) {
>>>>>>
>>>>>> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
>>>>>> struct acpi_namespace_node *node = prefix_node;
>>>>>>
>>>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
>>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>>>> + else
>>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
>>>>>>
>>>>>> if (path) {
>>>>>> status = acpi_ns_get_node(prefix_node, path,
>>>>>> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
>>>>>> index d9f15cb..c8ca25af 100644
>>>>>> --- a/drivers/acpi/acpica/utxferror.c
>>>>>> +++ b/drivers/acpi/acpica/utxferror.c
>>>>>> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
>>>>>> va_list arg_list;
>>>>>>
>>>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
>>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>>>> + else
>>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
>>>>>>
>>>>>> va_start(arg_list, format);
>>>>>> acpi_os_vprintf(format, arg_list);
>>>>>> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
>>>>>> va_list arg_list;
>>>>>>
>>>>>> ACPI_MSG_REDIRECT_BEGIN;
>>>>>> -
>>>>>> - /* For AE_OK, just print the message */
>>>>>> -
>>>>>> - if (ACPI_SUCCESS(status)) {
>>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
>>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
>>>>>> + else
>>>>>> acpi_os_printf(ACPI_MSG_EXCEPTION);
>>>>>>
>>>>>> - } else {
>>>>>> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
>>>>>> - acpi_format_exception(status));
>>>>>> - }
>>>>>> + /* For AE_OK, just print the message */
>>>>>> + if (ACPI_FAILURE(status))
>>>>>> + acpi_os_printf("%s, ", acpi_format_exception(status));
>>>>>>
>>>>>> va_start(arg_list, format);
>>>>>> acpi_os_vprintf(format, arg_list);
>>>>>> --
>>>>>> 2.9.3
>>>>>
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-14 8:15 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-14 8:15 UTC (permalink / raw)
To: Hans de Goede, Rafael J . Wysocki, Len Brown, Moore, Robert
Cc: linux-acpi, devel
Hi, Hans
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 13-03-17 10:52, Zheng, Lv wrote:
> > Hi, Hans
> >
> > For log level issue, is this fix useful for you?
> > https://github.com/acpica/acpica/pull/121/commits/a505d3942
>
> That fixes reloading already loaded tables, the problem I'm
> getting errors about its loading a different table with identical
> contents.
>
> I will look into your suggestion to do something similar to
> AcpiTbInstallStandardTable using AcpiTbCompareTables for the
> SSDT tables. I will send a new patch when I can make some time
> to look into this.
I just completed a prototype here:
https://github.com/acpica/acpica/pull/121
I guess the original "duplicate table check" couldn't cover static SSDTs.
Actually the duplicate table check should be a sanity check of table load.
And for table install, we should have a different sanity check like:
https://github.com/acpica/acpica/pull/121/commits/6e825cae5e5
I'm not sure if this is what you want.
Thanks
Lv
>
> Regards,
>
> Hans
>
>
>
> >
> > Thanks and best regards
> > Lv
> >
> >> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Hi,
> >>
> >> On 03-03-17 03:48, Zheng, Lv wrote:
> >>> Hi,
> >>>
> >>>> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >>>> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 02-03-17 03:03, Zheng, Lv wrote:
> >>>>> Hi,
> >>>>>
> >>>>>> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >>>>>> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>>>>>
> >>>>>> Honor the "Ignore errors while loading tables, get as many as possible"
> >>>>>> comment in the tbxfload.c code and log any exceptions and errors during
> >>>>>> loading extra tables as warnings.
> >>>>>>
> >>>>>> This is important because many desktop and embedded applicance Linux
> >>>>>> use-cases have a hard requirement of not showing any (scary) text
> >>>>>> messages during system bootup, which get broken by errors reported
> >>>>>> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> >>>>>> level are always shown even if the quiet kernel cmdline option is used.
> >>>>>
> >>>>> This looks worse than comparing the tables.
> >>>>
> >>>> This is really another patch entirely, it would also make the duplicate
> >>>> table thing less of an issue, but even with a fix for the duplicate tables
> >>>> we still need something like this to lower the log level of errors while
> >>>> loading extra tables as that currently causes errors to be spewn on
> >>>> way to many machines, e.g. on my main workstation I get:
> >>>>
> >>>> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
> >>>> (20160930/dswload-210)
> >>>> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
> >>>> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table
> (20160930/tbxfload-
> >> 268)
> >>>> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
> >>>
> >>> However some of them indicating a real issue that can be fixed by:
> >>> 1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer
> >> dependent on other fixes)
> >>> 2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
> >>> It's not far away than actually solving the real issues.
> >>> So why do we use a one-size-fits-all solution for such kind of errors?
> >>
> >> Ok, I agree that doing proper fixes for errors like this is better,
> >> so lets drop this patch.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>>
> >>> Thanks and best regards
> >>> Lv
> >>>
> >>>
> >>>>
> >>>> (this is with the patch, before that this were all errors and exceptions).
> >>>>
> >>>> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Hans
> >>>>
> >>>>
> >>>>
> >>>>> So before determining what should be done for the check of the duplicated aml tables.
> >>>>> Let's skip this submission.
> >>>>>
> >>>>> Thanks
> >>>>> Lv
> >>>>>
> >>>>>>
> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>>>>> ---
> >>>>>> drivers/acpi/acpica/acglobal.h | 1 +
> >>>>>> drivers/acpi/acpica/tbxfload.c | 4 +++-
> >>>>>> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> >>>>>> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> >>>>>> 4 files changed, 22 insertions(+), 12 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> >>>>>> index edbb42e..5ae9eee 100644
> >>>>>> --- a/drivers/acpi/acpica/acglobal.h
> >>>>>> +++ b/drivers/acpi/acpica/acglobal.h
> >>>>>> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> >>>>>> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> >>>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> >>>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> >>>>>> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
> >>>>>>
> >>>>>> /* Global handlers */
> >>>>>>
> >>>>>> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> >>>>>> index 1971cd7..b0c5478 100644
> >>>>>> --- a/drivers/acpi/acpica/tbxfload.c
> >>>>>> +++ b/drivers/acpi/acpica/tbxfload.c
> >>>>>> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> >>>>>> }
> >>>>>>
> >>>>>> /* Ignore errors while loading tables, get as many as possible */
> >>>>>> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
> >>>>>>
> >>>>>> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> >>>>>> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> >>>>>> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> >>>>>> tables_loaded++;
> >>>>>> }
> >>>>>> }
> >>>>>> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
> >>>>>>
> >>>>>> if (!tables_failed) {
> >>>>>> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> >>>>>> } else {
> >>>>>> - ACPI_ERROR((AE_INFO,
> >>>>>> + ACPI_WARNING((AE_INFO,
> >>>>>> "%u table load failures, %u successful",
> >>>>>> tables_failed, tables_loaded));
> >>>>>>
> >>>>>> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> >>>>>> index 475932c..e01ff1f 100644
> >>>>>> --- a/drivers/acpi/acpica/uterror.c
> >>>>>> +++ b/drivers/acpi/acpica/uterror.c
> >>>>>> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> >>>>>> char *name = NULL;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>>
> >>>>>> if (lookup_status == AE_BAD_CHARACTER) {
> >>>>>>
> >>>>>> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> >>>>>> struct acpi_namespace_node *node = prefix_node;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>>
> >>>>>> if (path) {
> >>>>>> status = acpi_ns_get_node(prefix_node, path,
> >>>>>> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> >>>>>> index d9f15cb..c8ca25af 100644
> >>>>>> --- a/drivers/acpi/acpica/utxferror.c
> >>>>>> +++ b/drivers/acpi/acpica/utxferror.c
> >>>>>> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> >>>>>> va_list arg_list;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>>
> >>>>>> va_start(arg_list, format);
> >>>>>> acpi_os_vprintf(format, arg_list);
> >>>>>> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> >>>>>> va_list arg_list;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> -
> >>>>>> - /* For AE_OK, just print the message */
> >>>>>> -
> >>>>>> - if (ACPI_SUCCESS(status)) {
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> acpi_os_printf(ACPI_MSG_EXCEPTION);
> >>>>>>
> >>>>>> - } else {
> >>>>>> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> >>>>>> - acpi_format_exception(status));
> >>>>>> - }
> >>>>>> + /* For AE_OK, just print the message */
> >>>>>> + if (ACPI_FAILURE(status))
> >>>>>> + acpi_os_printf("%s, ", acpi_format_exception(status));
> >>>>>>
> >>>>>> va_start(arg_list, format);
> >>>>>> acpi_os_vprintf(format, arg_list);
> >>>>>> --
> >>>>>> 2.9.3
> >>>>>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Devel] [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-14 8:15 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-14 8:15 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 9399 bytes --]
Hi, Hans
> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 13-03-17 10:52, Zheng, Lv wrote:
> > Hi, Hans
> >
> > For log level issue, is this fix useful for you?
> > https://github.com/acpica/acpica/pull/121/commits/a505d3942
>
> That fixes reloading already loaded tables, the problem I'm
> getting errors about its loading a different table with identical
> contents.
>
> I will look into your suggestion to do something similar to
> AcpiTbInstallStandardTable using AcpiTbCompareTables for the
> SSDT tables. I will send a new patch when I can make some time
> to look into this.
I just completed a prototype here:
https://github.com/acpica/acpica/pull/121
I guess the original "duplicate table check" couldn't cover static SSDTs.
Actually the duplicate table check should be a sanity check of table load.
And for table install, we should have a different sanity check like:
https://github.com/acpica/acpica/pull/121/commits/6e825cae5e5
I'm not sure if this is what you want.
Thanks
Lv
>
> Regards,
>
> Hans
>
>
>
> >
> > Thanks and best regards
> > Lv
> >
> >> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> >> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Hi,
> >>
> >> On 03-03-17 03:48, Zheng, Lv wrote:
> >>> Hi,
> >>>
> >>>> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> >>>> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 02-03-17 03:03, Zheng, Lv wrote:
> >>>>> Hi,
> >>>>>
> >>>>>> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> >>>>>> Subject: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>>>>>
> >>>>>> Honor the "Ignore errors while loading tables, get as many as possible"
> >>>>>> comment in the tbxfload.c code and log any exceptions and errors during
> >>>>>> loading extra tables as warnings.
> >>>>>>
> >>>>>> This is important because many desktop and embedded applicance Linux
> >>>>>> use-cases have a hard requirement of not showing any (scary) text
> >>>>>> messages during system bootup, which get broken by errors reported
> >>>>>> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> >>>>>> level are always shown even if the quiet kernel cmdline option is used.
> >>>>>
> >>>>> This looks worse than comparing the tables.
> >>>>
> >>>> This is really another patch entirely, it would also make the duplicate
> >>>> table thing less of an issue, but even with a fix for the duplicate tables
> >>>> we still need something like this to lower the log level of errors while
> >>>> loading extra tables as that currently causes errors to be spewn on
> >>>> way to many machines, e.g. on my main workstation I get:
> >>>>
> >>>> [ 0.019856] ACPI Warning: [\_SB_.PCI0.XHC_.RHUB.HS11] Namespace lookup failure, AE_NOT_FOUND
> >>>> (20160930/dswload-210)
> >>>> [ 0.019859] ACPI Warning: AE_NOT_FOUND, During name lookup/catalog (20160930/psobject-227)
> >>>> [ 0.019886] ACPI Warning: AE_NOT_FOUND, (SSDT:xh_rvp08) while loading table
> (20160930/tbxfload-
> >> 268)
> >>>> [ 0.026491] ACPI Warning: 1 table load failures, 5 successful (20160930/tbxfload-287)
> >>>
> >>> However some of them indicating a real issue that can be fixed by:
> >>> 1. Merging this pull request: https://github.com/acpica/acpica/pull/189 (I made it no longer
> >> dependent on other fixes)
> >>> 2. Setting acpi_gbl_parse_table_as_term_list to TRUE.
> >>> It's not far away than actually solving the real issues.
> >>> So why do we use a one-size-fits-all solution for such kind of errors?
> >>
> >> Ok, I agree that doing proper fixes for errors like this is better,
> >> so lets drop this patch.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>>
> >>> Thanks and best regards
> >>> Lv
> >>>
> >>>
> >>>>
> >>>> (this is with the patch, before that this were all errors and exceptions).
> >>>>
> >>>> Add IIRC I'm getting yet another set of errors on my XPS 15, etc.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Hans
> >>>>
> >>>>
> >>>>
> >>>>> So before determining what should be done for the check of the duplicated aml tables.
> >>>>> Let's skip this submission.
> >>>>>
> >>>>> Thanks
> >>>>> Lv
> >>>>>
> >>>>>>
> >>>>>> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
> >>>>>> ---
> >>>>>> drivers/acpi/acpica/acglobal.h | 1 +
> >>>>>> drivers/acpi/acpica/tbxfload.c | 4 +++-
> >>>>>> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> >>>>>> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> >>>>>> 4 files changed, 22 insertions(+), 12 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> >>>>>> index edbb42e..5ae9eee 100644
> >>>>>> --- a/drivers/acpi/acpica/acglobal.h
> >>>>>> +++ b/drivers/acpi/acpica/acglobal.h
> >>>>>> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> >>>>>> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> >>>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> >>>>>> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> >>>>>> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
> >>>>>>
> >>>>>> /* Global handlers */
> >>>>>>
> >>>>>> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> >>>>>> index 1971cd7..b0c5478 100644
> >>>>>> --- a/drivers/acpi/acpica/tbxfload.c
> >>>>>> +++ b/drivers/acpi/acpica/tbxfload.c
> >>>>>> @@ -256,6 +256,7 @@ acpi_status acpi_tb_load_namespace(void)
> >>>>>> }
> >>>>>>
> >>>>>> /* Ignore errors while loading tables, get as many as possible */
> >>>>>> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
> >>>>>>
> >>>>>> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> >>>>>> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> >>>>>> @@ -276,11 +277,12 @@ acpi_status acpi_tb_load_namespace(void)
> >>>>>> tables_loaded++;
> >>>>>> }
> >>>>>> }
> >>>>>> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
> >>>>>>
> >>>>>> if (!tables_failed) {
> >>>>>> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> >>>>>> } else {
> >>>>>> - ACPI_ERROR((AE_INFO,
> >>>>>> + ACPI_WARNING((AE_INFO,
> >>>>>> "%u table load failures, %u successful",
> >>>>>> tables_failed, tables_loaded));
> >>>>>>
> >>>>>> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> >>>>>> index 475932c..e01ff1f 100644
> >>>>>> --- a/drivers/acpi/acpica/uterror.c
> >>>>>> +++ b/drivers/acpi/acpica/uterror.c
> >>>>>> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> >>>>>> char *name = NULL;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>>
> >>>>>> if (lookup_status == AE_BAD_CHARACTER) {
> >>>>>>
> >>>>>> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> >>>>>> struct acpi_namespace_node *node = prefix_node;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>>
> >>>>>> if (path) {
> >>>>>> status = acpi_ns_get_node(prefix_node, path,
> >>>>>> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> >>>>>> index d9f15cb..c8ca25af 100644
> >>>>>> --- a/drivers/acpi/acpica/utxferror.c
> >>>>>> +++ b/drivers/acpi/acpica/utxferror.c
> >>>>>> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> >>>>>> va_list arg_list;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> - acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> + acpi_os_printf(ACPI_MSG_ERROR);
> >>>>>>
> >>>>>> va_start(arg_list, format);
> >>>>>> acpi_os_vprintf(format, arg_list);
> >>>>>> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> >>>>>> va_list arg_list;
> >>>>>>
> >>>>>> ACPI_MSG_REDIRECT_BEGIN;
> >>>>>> -
> >>>>>> - /* For AE_OK, just print the message */
> >>>>>> -
> >>>>>> - if (ACPI_SUCCESS(status)) {
> >>>>>> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> >>>>>> + acpi_os_printf(ACPI_MSG_WARNING);
> >>>>>> + else
> >>>>>> acpi_os_printf(ACPI_MSG_EXCEPTION);
> >>>>>>
> >>>>>> - } else {
> >>>>>> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> >>>>>> - acpi_format_exception(status));
> >>>>>> - }
> >>>>>> + /* For AE_OK, just print the message */
> >>>>>> + if (ACPI_FAILURE(status))
> >>>>>> + acpi_os_printf("%s, ", acpi_format_exception(status));
> >>>>>>
> >>>>>> va_start(arg_list, format);
> >>>>>> acpi_os_vprintf(format, arg_list);
> >>>>>> --
> >>>>>> 2.9.3
> >>>>>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
2017-03-14 8:15 ` [Devel] " Zheng, Lv
(?)
@ 2017-03-14 8:56 ` Hans de Goede
2017-03-14 11:54 ` Hans de Goede
2017-03-15 1:16 ` [Devel] " Zheng, Lv
-1 siblings, 2 replies; 18+ messages in thread
From: Hans de Goede @ 2017-03-14 8:56 UTC (permalink / raw)
To: Zheng, Lv, Rafael J . Wysocki, Len Brown, Moore, Robert; +Cc: linux-acpi, devel
[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]
Hi,
On 14-03-17 09:15, Zheng, Lv wrote:
> Hi, Hans
>
>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>
>> Hi,
>>
>> On 13-03-17 10:52, Zheng, Lv wrote:
>>> Hi, Hans
>>>
>>> For log level issue, is this fix useful for you?
>>> https://github.com/acpica/acpica/pull/121/commits/a505d3942
>>
>> That fixes reloading already loaded tables, the problem I'm
>> getting errors about its loading a different table with identical
>> contents.
>>
>> I will look into your suggestion to do something similar to
>> AcpiTbInstallStandardTable using AcpiTbCompareTables for the
>> SSDT tables. I will send a new patch when I can make some time
>> to look into this.
>
> I just completed a prototype here:
> https://github.com/acpica/acpica/pull/121
>
> I guess the original "duplicate table check" couldn't cover static SSDTs.
> Actually the duplicate table check should be a sanity check of table load.
> And for table install, we should have a different sanity check like:
> https://github.com/acpica/acpica/pull/121/commits/6e825cae5e5
> I'm not sure if this is what you want.
This checks for having 2 table_descriptors pointing to the same table
(address in memory). But in my case there are 2 identical copies of
the table at 2 different addresses in memory, so this won't work.
After looking at this a bit myself, I think fixing this is actually
quite easy (now that you've pointed me to acpi_tb_install_standard_table()
I've come up with the attached patch to fix this. I will give this a test
spin and then submit it officially (assuming it works).
Regards,
Hans
[-- Attachment #2: 0001-ACPICA-Always-check-for-duplicate-tables-in-acpi_tb_.patch --]
[-- Type: text/x-patch, Size: 4674 bytes --]
>From ded25995e0071c8e48f7e634e50efd9c675dcfce Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 14 Mar 2017 09:49:03 +0100
Subject: [PATCH v3] ACPICA: Always check for duplicate tables in
acpi_tb_install_standard_table
Always check for duplicate tables in acpi_tb_install_standard_table,
not just when the reload flag is set.
Some firmware contains duplicate tables in their RSDT or XSDT, leading
to a bunch of errors being printed on Linux boot.
This commit moves the check for duplicate tables in
acpi_tb_install_standard_table out of the if (reload) {} block,
to catch this and ignore the duplicate tables.
Note for reviewers: all this does is move the check outside of the
if (reload) {} block, no other changes are made.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/acpica/tbinstal.c | 86 +++++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
index 4620f3c..8622a73 100644
--- a/drivers/acpi/acpica/tbinstal.c
+++ b/drivers/acpi/acpica/tbinstal.c
@@ -250,54 +250,54 @@ acpi_tb_install_standard_table(acpi_physical_address address,
status = AE_BAD_SIGNATURE;
goto unlock_and_exit;
}
+ }
- /* Check if table is already registered */
+ /* Check if table is already registered */
- for (i = 0; i < acpi_gbl_root_table_list.current_table_count;
- ++i) {
- /*
- * Check for a table match on the entire table length,
- * not just the header.
- */
- if (!acpi_tb_compare_tables(&new_table_desc, i)) {
- continue;
- }
+ for (i = 0; i < acpi_gbl_root_table_list.current_table_count;
+ ++i) {
+ /*
+ * Check for a table match on the entire table length,
+ * not just the header.
+ */
+ if (!acpi_tb_compare_tables(&new_table_desc, i)) {
+ continue;
+ }
+ /*
+ * Note: the current mechanism does not unregister a table if it is
+ * dynamically unloaded. The related namespace entries are deleted,
+ * but the table remains in the root table list.
+ *
+ * The assumption here is that the number of different tables that
+ * will be loaded is actually small, and there is minimal overhead
+ * in just keeping the table in case it is needed again.
+ *
+ * If this assumption changes in the future (perhaps on large
+ * machines with many table load/unload operations), tables will
+ * need to be unregistered when they are unloaded, and slots in the
+ * root table list should be reused when empty.
+ */
+ if (acpi_gbl_root_table_list.tables[i].flags &
+ ACPI_TABLE_IS_LOADED) {
+
+ /* Table is still loaded, this is an error */
+
+ status = AE_ALREADY_EXISTS;
+ goto unlock_and_exit;
+ } else {
/*
- * Note: the current mechanism does not unregister a table if it is
- * dynamically unloaded. The related namespace entries are deleted,
- * but the table remains in the root table list.
- *
- * The assumption here is that the number of different tables that
- * will be loaded is actually small, and there is minimal overhead
- * in just keeping the table in case it is needed again.
- *
- * If this assumption changes in the future (perhaps on large
- * machines with many table load/unload operations), tables will
- * need to be unregistered when they are unloaded, and slots in the
- * root table list should be reused when empty.
+ * Table was unloaded, allow it to be reloaded.
+ * As we are going to return AE_OK to the caller, we should
+ * take the responsibility of freeing the input descriptor.
+ * Refill the input descriptor to ensure
+ * acpi_tb_install_table_with_override() can be called again to
+ * indicate the re-installation.
*/
- if (acpi_gbl_root_table_list.tables[i].flags &
- ACPI_TABLE_IS_LOADED) {
-
- /* Table is still loaded, this is an error */
-
- status = AE_ALREADY_EXISTS;
- goto unlock_and_exit;
- } else {
- /*
- * Table was unloaded, allow it to be reloaded.
- * As we are going to return AE_OK to the caller, we should
- * take the responsibility of freeing the input descriptor.
- * Refill the input descriptor to ensure
- * acpi_tb_install_table_with_override() can be called again to
- * indicate the re-installation.
- */
- acpi_tb_uninstall_table(&new_table_desc);
- (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
- *table_index = i;
- return_ACPI_STATUS(AE_OK);
- }
+ acpi_tb_uninstall_table(&new_table_desc);
+ (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
+ *table_index = i;
+ return_ACPI_STATUS(AE_OK);
}
}
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
2017-03-14 8:56 ` Hans de Goede
@ 2017-03-14 11:54 ` Hans de Goede
2017-03-15 1:16 ` [Devel] " Zheng, Lv
1 sibling, 0 replies; 18+ messages in thread
From: Hans de Goede @ 2017-03-14 11:54 UTC (permalink / raw)
To: Zheng, Lv, Rafael J . Wysocki, Len Brown, Moore, Robert; +Cc: linux-acpi, devel
Hi,
On 14-03-17 09:56, Hans de Goede wrote:
> Hi,
>
> On 14-03-17 09:15, Zheng, Lv wrote:
>> Hi, Hans
>>
>>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>>> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>>>
>>> Hi,
>>>
>>> On 13-03-17 10:52, Zheng, Lv wrote:
>>>> Hi, Hans
>>>>
>>>> For log level issue, is this fix useful for you?
>>>> https://github.com/acpica/acpica/pull/121/commits/a505d3942
>>>
>>> That fixes reloading already loaded tables, the problem I'm
>>> getting errors about its loading a different table with identical
>>> contents.
>>>
>>> I will look into your suggestion to do something similar to
>>> AcpiTbInstallStandardTable using AcpiTbCompareTables for the
>>> SSDT tables. I will send a new patch when I can make some time
>>> to look into this.
>>
>> I just completed a prototype here:
>> https://github.com/acpica/acpica/pull/121
>>
>> I guess the original "duplicate table check" couldn't cover static SSDTs.
>> Actually the duplicate table check should be a sanity check of table load.
>> And for table install, we should have a different sanity check like:
>> https://github.com/acpica/acpica/pull/121/commits/6e825cae5e5
>> I'm not sure if this is what you want.
>
> This checks for having 2 table_descriptors pointing to the same table
> (address in memory). But in my case there are 2 identical copies of
> the table at 2 different addresses in memory, so this won't work.
>
> After looking at this a bit myself, I think fixing this is actually
> quite easy (now that you've pointed me to acpi_tb_install_standard_table()
>
> I've come up with the attached patch to fix this. I will give this a test
> spin and then submit it officially (assuming it works).
Ok the approach of doing the check during acpi_tb_install_standard_table
does not work because then acpi_gbl_verify_table_checksum is false so
we are only loading the header of the table, that and we are not supposed
to load more data / use more mem this early, which the call to
acpi_tb_acquire_table() will do for the table being compared against.
So it looks like we will need to go with some version of my patch which
does the check later when acpi_gbl_verify_table_checksum is true.
Regards,
Hans
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-15 1:16 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-15 1:16 UTC (permalink / raw)
To: Hans de Goede, Rafael J . Wysocki, Len Brown, Moore, Robert
Cc: linux-acpi, devel
Hi, Hans
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 14-03-17 09:15, Zheng, Lv wrote:
> > Hi, Hans
> >
> >> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Hi,
> >>
> >> On 13-03-17 10:52, Zheng, Lv wrote:
> >>> Hi, Hans
> >>>
> >>> For log level issue, is this fix useful for you?
> >>> https://github.com/acpica/acpica/pull/121/commits/a505d3942
> >>
> >> That fixes reloading already loaded tables, the problem I'm
> >> getting errors about its loading a different table with identical
> >> contents.
> >>
> >> I will look into your suggestion to do something similar to
> >> AcpiTbInstallStandardTable using AcpiTbCompareTables for the
> >> SSDT tables. I will send a new patch when I can make some time
> >> to look into this.
> >
> > I just completed a prototype here:
> > https://github.com/acpica/acpica/pull/121
> >
> > I guess the original "duplicate table check" couldn't cover static SSDTs.
> > Actually the duplicate table check should be a sanity check of table load.
> > And for table install, we should have a different sanity check like:
> > https://github.com/acpica/acpica/pull/121/commits/6e825cae5e5
> > I'm not sure if this is what you want.
>
> This checks for having 2 table_descriptors pointing to the same table
> (address in memory). But in my case there are 2 identical copies of
> the table at 2 different addresses in memory, so this won't work.
>
> After looking at this a bit myself, I think fixing this is actually
> quite easy (now that you've pointed me to acpi_tb_install_standard_table()
>
> I've come up with the attached patch to fix this. I will give this a test
> spin and then submit it officially (assuming it works).
In the pull request, there are 4 fixes:
1. add table install sanity check by detecting duplicates with table address and table type.
2. extend table load sanity check to LoadTable opcode.
3. extend table load sanity check to static loaded SSDTs - I think this should be able to cover your case.
4. if install/load sanity checks detect duplicate tables, they return AE_ALREADY_EXISTS and the error is silently discarded.
That's the whole story for the pull request.
I think you only checked one of them.
Thanks and best regards
Lv
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Devel] [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2017-03-15 1:16 ` Zheng, Lv
0 siblings, 0 replies; 18+ messages in thread
From: Zheng, Lv @ 2017-03-15 1:16 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 2490 bytes --]
Hi, Hans
> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
>
> Hi,
>
> On 14-03-17 09:15, Zheng, Lv wrote:
> > Hi, Hans
> >
> >> From: Hans de Goede [mailto:hdegoede(a)redhat.com]
> >> Subject: Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
> >>
> >> Hi,
> >>
> >> On 13-03-17 10:52, Zheng, Lv wrote:
> >>> Hi, Hans
> >>>
> >>> For log level issue, is this fix useful for you?
> >>> https://github.com/acpica/acpica/pull/121/commits/a505d3942
> >>
> >> That fixes reloading already loaded tables, the problem I'm
> >> getting errors about its loading a different table with identical
> >> contents.
> >>
> >> I will look into your suggestion to do something similar to
> >> AcpiTbInstallStandardTable using AcpiTbCompareTables for the
> >> SSDT tables. I will send a new patch when I can make some time
> >> to look into this.
> >
> > I just completed a prototype here:
> > https://github.com/acpica/acpica/pull/121
> >
> > I guess the original "duplicate table check" couldn't cover static SSDTs.
> > Actually the duplicate table check should be a sanity check of table load.
> > And for table install, we should have a different sanity check like:
> > https://github.com/acpica/acpica/pull/121/commits/6e825cae5e5
> > I'm not sure if this is what you want.
>
> This checks for having 2 table_descriptors pointing to the same table
> (address in memory). But in my case there are 2 identical copies of
> the table at 2 different addresses in memory, so this won't work.
>
> After looking at this a bit myself, I think fixing this is actually
> quite easy (now that you've pointed me to acpi_tb_install_standard_table()
>
> I've come up with the attached patch to fix this. I will give this a test
> spin and then submit it officially (assuming it works).
In the pull request, there are 4 fixes:
1. add table install sanity check by detecting duplicates with table address and table type.
2. extend table load sanity check to LoadTable opcode.
3. extend table load sanity check to static loaded SSDTs - I think this should be able to cover your case.
4. if install/load sanity checks detect duplicate tables, they return AE_ALREADY_EXISTS and the error is silently discarded.
That's the whole story for the pull request.
I think you only checked one of them.
Thanks and best regards
Lv
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/1] ACPICA: Log Exceptions and Errors as warning while loading extra tables
@ 2018-01-11 19:28 Hans de Goede
2018-01-11 19:28 ` [PATCH] " Hans de Goede
0 siblings, 1 reply; 18+ messages in thread
From: Hans de Goede @ 2018-01-11 19:28 UTC (permalink / raw)
To: Len Brown, Robert Moore, Lv Zheng; +Cc: Hans de Goede, linux-acpi, devel
Hi All,
So this is a rebased resend of an old patch of mine, which back then was
rejected because all the ugly ACPI errors scaring the user during boot
would need to be resolved in either the ACPICA code or in the BIOS /
firmware in question.
It is a year later now and as all the people commenting on:
https://bugzilla.kernel.org/show_bug.cgi?id=198167
show (33 comments and counting in a short time), people experience these
kinda errors as a real problem. Note this bug is (mostly) about a new bunch
of ACPI error messages which are not fixed by this patch actually, but it
does nicely illustrate that people experience these errors as a real problem /
a real nuisance and I agree with them.
https://bugzilla.kernel.org/show_bug.cgi?id=198167#c33
also shows that the chances of ever getting these errors fixed in the case
where the problem is in the DSDT is very close to 0, so rejecting this
patch with an "ask your vendor to fix the firmware" is really not a useful
answer, people have tried and this is simply not going to happen.
Given all the effort distros have done with splash-screens to give
users a nice clean boot experience, we really want dmesg --level=err
to not print anything unless there is a real problem with either the
hardware or the kernel. And in this case neither is not true and these
unfixable and therefor useless errors only lead to a very jarring boot
experience (hence all the users complaining in bug 198167).
Note this patch does not silence or removes the messages about these
problems in the ACPI tables, it merely lowers their log level from
error to warning so that the messages do not show on the console, they
will still show in dmesg and other logs.
Regards,
Hans
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
2018-01-11 19:28 [PATCH 0/1] " Hans de Goede
@ 2018-01-11 19:28 ` Hans de Goede
2018-01-12 10:25 ` Hans de Goede
0 siblings, 1 reply; 18+ messages in thread
From: Hans de Goede @ 2018-01-11 19:28 UTC (permalink / raw)
To: Len Brown, Robert Moore, Lv Zheng; +Cc: Hans de Goede, linux-acpi, devel
Honor the "Ignore errors while loading tables, get as many as possible"
comment in the tbxfload.c code and log any exceptions and errors during
loading extra tables as warnings.
This is important because many desktop and embedded applicance Linux
use-cases have a hard requirement of not showing any (scary) text
messages during system bootup, which get broken by errors reported
by attempts to load the extra tables, as messaged logged at a KERN_ERR
level are always shown even if the quiet kernel cmdline option is used.
Ideally all ACPI tables would be free of errors, but in practice that
is just not the case and vendors are not always willing / responsive
to fixing issues.
Note this commit only lowers the loglevel while loading extra / optional
tables from acpi_tb_load_namespace() all other parsing errors are still
logged with a loglevel of error.
BugLink: http://en.community.dell.com/techcenter/os-applications/f/4457/t/20006889
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109511
Related: https://bugzilla.kernel.org/show_bug.cgi?id=198167
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/acpica/acglobal.h | 1 +
drivers/acpi/acpica/tbxfload.c | 4 +++-
drivers/acpi/acpica/uterror.c | 10 ++++++++--
drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
4 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index 95eed442703f..7cb2ee8ce945 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
+ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
/* Global handlers */
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index d81f442228b8..a1eaa69d0749 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -217,6 +217,7 @@ acpi_status acpi_tb_load_namespace(void)
}
/* Ignore errors while loading tables, get as many as possible */
+ acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
status = acpi_ns_load_table(i, acpi_gbl_root_node);
@@ -237,11 +238,12 @@ acpi_status acpi_tb_load_namespace(void)
tables_loaded++;
}
}
+ acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
if (!tables_failed) {
ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
} else {
- ACPI_ERROR((AE_INFO,
+ ACPI_WARNING((AE_INFO,
"%u table load failures, %u successful",
tables_failed, tables_loaded));
diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
index e3368186e1c1..fc3794c1c324 100644
--- a/drivers/acpi/acpica/uterror.c
+++ b/drivers/acpi/acpica/uterror.c
@@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
char *name = NULL;
ACPI_MSG_REDIRECT_BEGIN;
- acpi_os_printf(ACPI_MSG_ERROR);
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
+ acpi_os_printf(ACPI_MSG_ERROR);
if (lookup_status == AE_BAD_CHARACTER) {
@@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
struct acpi_namespace_node *node = prefix_node;
ACPI_MSG_REDIRECT_BEGIN;
- acpi_os_printf(ACPI_MSG_ERROR);
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
+ acpi_os_printf(ACPI_MSG_ERROR);
if (path) {
status = acpi_ns_get_node(prefix_node, path,
diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
index 950a1e500bfa..553b4eee8cec 100644
--- a/drivers/acpi/acpica/utxferror.c
+++ b/drivers/acpi/acpica/utxferror.c
@@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
va_list arg_list;
ACPI_MSG_REDIRECT_BEGIN;
- acpi_os_printf(ACPI_MSG_ERROR);
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
+ acpi_os_printf(ACPI_MSG_ERROR);
va_start(arg_list, format);
acpi_os_vprintf(format, arg_list);
@@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
va_list arg_list;
ACPI_MSG_REDIRECT_BEGIN;
-
- /* For AE_OK, just print the message */
-
- if (ACPI_SUCCESS(status)) {
+ if (acpi_gbl_log_errors_exceptions_as_warnings)
+ acpi_os_printf(ACPI_MSG_WARNING);
+ else
acpi_os_printf(ACPI_MSG_EXCEPTION);
- } else {
- acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
- acpi_format_exception(status));
- }
+ /* For failures append the formatted exception */
+ if (ACPI_FAILURE(status))
+ acpi_os_printf("%s, ", acpi_format_exception(status));
va_start(arg_list, format);
acpi_os_vprintf(format, arg_list);
--
2.14.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables
2018-01-11 19:28 ` [PATCH] " Hans de Goede
@ 2018-01-12 10:25 ` Hans de Goede
0 siblings, 0 replies; 18+ messages in thread
From: Hans de Goede @ 2018-01-12 10:25 UTC (permalink / raw)
To: Len Brown, Robert Moore, Lv Zheng; +Cc: linux-acpi, devel
Hi,
On 11-01-18 20:28, Hans de Goede wrote:
> Honor the "Ignore errors while loading tables, get as many as possible"
> comment in the tbxfload.c code and log any exceptions and errors during
> loading extra tables as warnings.
>
> This is important because many desktop and embedded applicance Linux
> use-cases have a hard requirement of not showing any (scary) text
> messages during system bootup, which get broken by errors reported
> by attempts to load the extra tables, as messaged logged at a KERN_ERR
> level are always shown even if the quiet kernel cmdline option is used.
>
> Ideally all ACPI tables would be free of errors, but in practice that
> is just not the case and vendors are not always willing / responsive
> to fixing issues.
>
> Note this commit only lowers the loglevel while loading extra / optional
> tables from acpi_tb_load_namespace() all other parsing errors are still
> logged with a loglevel of error.
>
> BugLink: http://en.community.dell.com/techcenter/os-applications/f/4457/t/20006889
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109511
> Related: https://bugzilla.kernel.org/show_bug.cgi?id=198167
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
I ended up looking at this again today and specifically also at
fixing https://bugzilla.kernel.org/show_bug.cgi?id=198167.
I believe I've a cleaner fix without the global flag for this, I will
post a new series replacing this patch later today,
Regards,
Hans
> ---
> drivers/acpi/acpica/acglobal.h | 1 +
> drivers/acpi/acpica/tbxfload.c | 4 +++-
> drivers/acpi/acpica/uterror.c | 10 ++++++++--
> drivers/acpi/acpica/utxferror.c | 19 ++++++++++---------
> 4 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> index 95eed442703f..7cb2ee8ce945 100644
> --- a/drivers/acpi/acpica/acglobal.h
> +++ b/drivers/acpi/acpica/acglobal.h
> @@ -146,6 +146,7 @@ ACPI_GLOBAL(acpi_cache_t *, acpi_gbl_operand_cache);
> ACPI_INIT_GLOBAL(u32, acpi_gbl_startup_flags, 0);
> ACPI_INIT_GLOBAL(u8, acpi_gbl_shutdown, TRUE);
> ACPI_INIT_GLOBAL(u8, acpi_gbl_early_initialization, TRUE);
> +ACPI_INIT_GLOBAL(u8, acpi_gbl_log_errors_exceptions_as_warnings, FALSE);
>
> /* Global handlers */
>
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index d81f442228b8..a1eaa69d0749 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -217,6 +217,7 @@ acpi_status acpi_tb_load_namespace(void)
> }
>
> /* Ignore errors while loading tables, get as many as possible */
> + acpi_gbl_log_errors_exceptions_as_warnings = TRUE;
>
> (void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
> status = acpi_ns_load_table(i, acpi_gbl_root_node);
> @@ -237,11 +238,12 @@ acpi_status acpi_tb_load_namespace(void)
> tables_loaded++;
> }
> }
> + acpi_gbl_log_errors_exceptions_as_warnings = FALSE;
>
> if (!tables_failed) {
> ACPI_INFO(("%u ACPI AML tables successfully acquired and loaded", tables_loaded));
> } else {
> - ACPI_ERROR((AE_INFO,
> + ACPI_WARNING((AE_INFO,
> "%u table load failures, %u successful",
> tables_failed, tables_loaded));
>
> diff --git a/drivers/acpi/acpica/uterror.c b/drivers/acpi/acpica/uterror.c
> index e3368186e1c1..fc3794c1c324 100644
> --- a/drivers/acpi/acpica/uterror.c
> +++ b/drivers/acpi/acpica/uterror.c
> @@ -205,7 +205,10 @@ acpi_ut_namespace_error(const char *module_name,
> char *name = NULL;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> if (lookup_status == AE_BAD_CHARACTER) {
>
> @@ -269,7 +272,10 @@ acpi_ut_method_error(const char *module_name,
> struct acpi_namespace_node *node = prefix_node;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> if (path) {
> status = acpi_ns_get_node(prefix_node, path,
> diff --git a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> index 950a1e500bfa..553b4eee8cec 100644
> --- a/drivers/acpi/acpica/utxferror.c
> +++ b/drivers/acpi/acpica/utxferror.c
> @@ -73,7 +73,10 @@ acpi_error(const char *module_name, u32 line_number, const char *format, ...)
> va_list arg_list;
>
> ACPI_MSG_REDIRECT_BEGIN;
> - acpi_os_printf(ACPI_MSG_ERROR);
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> + acpi_os_printf(ACPI_MSG_ERROR);
>
> va_start(arg_list, format);
> acpi_os_vprintf(format, arg_list);
> @@ -107,16 +110,14 @@ acpi_exception(const char *module_name,
> va_list arg_list;
>
> ACPI_MSG_REDIRECT_BEGIN;
> -
> - /* For AE_OK, just print the message */
> -
> - if (ACPI_SUCCESS(status)) {
> + if (acpi_gbl_log_errors_exceptions_as_warnings)
> + acpi_os_printf(ACPI_MSG_WARNING);
> + else
> acpi_os_printf(ACPI_MSG_EXCEPTION);
>
> - } else {
> - acpi_os_printf(ACPI_MSG_EXCEPTION "%s, ",
> - acpi_format_exception(status));
> - }
> + /* For failures append the formatted exception */
> + if (ACPI_FAILURE(status))
> + acpi_os_printf("%s, ", acpi_format_exception(status));
>
> va_start(arg_list, format);
> acpi_os_vprintf(format, arg_list);
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-01-12 10:25 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-01 10:36 [PATCH] ACPICA: Log Exceptions and Errors as warning while loading extra tables Hans de Goede
2017-03-02 2:03 ` Zheng, Lv
2017-03-02 2:03 ` [Devel] " Zheng, Lv
2017-03-02 15:27 ` Hans de Goede
2017-03-03 2:48 ` Zheng, Lv
2017-03-03 2:48 ` [Devel] " Zheng, Lv
2017-03-03 13:55 ` Hans de Goede
2017-03-13 9:52 ` Zheng, Lv
2017-03-13 9:52 ` [Devel] " Zheng, Lv
2017-03-13 10:06 ` Hans de Goede
2017-03-14 8:15 ` Zheng, Lv
2017-03-14 8:15 ` [Devel] " Zheng, Lv
2017-03-14 8:56 ` Hans de Goede
2017-03-14 11:54 ` Hans de Goede
2017-03-15 1:16 ` Zheng, Lv
2017-03-15 1:16 ` [Devel] " Zheng, Lv
2018-01-11 19:28 [PATCH 0/1] " Hans de Goede
2018-01-11 19:28 ` [PATCH] " Hans de Goede
2018-01-12 10:25 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.