ACPI driver should make sure all the processor IDs in their ACPI Namespace
are unique. the driver performs a depth-first walk of the namespace tree
and calls the acpi_processor_ids_walk() to check the duplicate IDs.
But, the acpi_processor_ids_walk() mistakes the return value. If a
processor is checked, it returns true which causes the walk break
immediately, and other processors will never be checked.
Repace the value with AE_OK which is the standard acpi_status value.
And don't abort the namespace walk even on error.
Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for processor enumeration")
Signed-off-by: Dou Liyang <[email protected]>
---
Changelog:
v1 --> v2:
- Fix the check against duplicate IDs suggested by Rafael.
Now，the duplicate IDs only be found in Ivb42 machine, and we have added this check at
linux-4.9. But, we introduced a bug in linux-4.12 by commit 8c8cb30f49b8.
For resolving the bug, firstly, I removed the check[1]. because Linux will compare
the coming ID with present processors when it hot-added a physical CPU and will avoid
using duplicate IDs.
But, seems we should consider all the possible processors. So, with this patch, All
the processors with the same IDs will never be hot-plugged.
[1] https://lkml.org/lkml/2018/5/28/213
---
drivers/acpi/acpi_processor.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 449d86d39965..a59870ccd5ca 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -643,7 +643,7 @@ static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
status = acpi_get_type(handle, &acpi_type);
if (ACPI_FAILURE(status))
- return false;
+ return_ACPI_STATUS(status);
switch (acpi_type) {
case ACPI_TYPE_PROCESSOR:
@@ -663,11 +663,12 @@ static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
}
processor_validated_ids_update(uid);
- return true;
+ return AE_OK;
err:
+ /* Exit on error, but don't abort the namespace walk */
acpi_handle_info(handle, "Invalid processor object\n");
- return false;
+ return AE_OK;
}
--
2.14.3
Hi Dou,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.18 next-20180822]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dou-Liyang/acpi-processor-Fix-the-return-value-of-acpi_processor_ids_walk/20180823-165002
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-r0-08231341 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
In file included from include/acpi/acpi.h:29:0,
from include/linux/acpi.h:34,
from drivers/acpi/acpi_processor.c:16:
drivers/acpi/acpi_processor.c: In function 'acpi_processor_ids_walk':
>> include/acpi/acoutput.h:391:19: error: implicit declaration of function 'acpi_ut_status_exit' [-Werror=implicit-function-declaration]
ACPI_TRACE_EXIT (acpi_ut_status_exit, acpi_status, status)
^
include/acpi/acoutput.h:274:44: note: in definition of macro 'ACPI_DO_WHILE0'
#define ACPI_DO_WHILE0(a) do a while(0)
^
include/acpi/acoutput.h:391:2: note: in expansion of macro 'ACPI_TRACE_EXIT'
ACPI_TRACE_EXIT (acpi_ut_status_exit, acpi_status, status)
^~~~~~~~~~~~~~~
drivers/acpi/acpi_processor.c:646:3: note: in expansion of macro 'return_ACPI_STATUS'
return_ACPI_STATUS(status);
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/acpi/acpi.h:29:0,
from include/linux/acpi.h:34,
from drivers//acpi/acpi_processor.c:16:
drivers//acpi/acpi_processor.c: In function 'acpi_processor_ids_walk':
>> include/acpi/acoutput.h:391:19: error: implicit declaration of function 'acpi_ut_status_exit' [-Werror=implicit-function-declaration]
ACPI_TRACE_EXIT (acpi_ut_status_exit, acpi_status, status)
^
include/acpi/acoutput.h:274:44: note: in definition of macro 'ACPI_DO_WHILE0'
#define ACPI_DO_WHILE0(a) do a while(0)
^
include/acpi/acoutput.h:391:2: note: in expansion of macro 'ACPI_TRACE_EXIT'
ACPI_TRACE_EXIT (acpi_ut_status_exit, acpi_status, status)
^~~~~~~~~~~~~~~
drivers//acpi/acpi_processor.c:646:3: note: in expansion of macro 'return_ACPI_STATUS'
return_ACPI_STATUS(status);
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/acpi_ut_status_exit +391 include/acpi/acoutput.h
0377b5ac Bob Moore 2012-12-31 383
0377b5ac Bob Moore 2012-12-31 384 #define return_VOID \
0377b5ac Bob Moore 2012-12-31 385 ACPI_DO_WHILE0 ({ \
0377b5ac Bob Moore 2012-12-31 386 acpi_ut_exit (ACPI_DEBUG_PARAMETERS); \
0377b5ac Bob Moore 2012-12-31 387 return; \
0377b5ac Bob Moore 2012-12-31 388 })
0377b5ac Bob Moore 2012-12-31 389
0377b5ac Bob Moore 2012-12-31 390 #define return_ACPI_STATUS(status) \
fd1af712 Bob Moore 2013-03-08 @391 ACPI_TRACE_EXIT (acpi_ut_status_exit, acpi_status, status)
0377b5ac Bob Moore 2012-12-31 392
:::::: The code at line 391 was first introduced by commit
:::::: fd1af7126fb62688cfcf4b563c73b2909ac30f74 ACPICA: Regression fix: reinstate safe exit macros
:::::: TO: Bob Moore <[email protected]>
:::::: CC: Rafael J. Wysocki <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Dou,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.18 next-20180822]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dou-Liyang/acpi-processor-Fix-the-return-value-of-acpi_processor_ids_walk/20180823-165002
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-h0-08231413 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers//acpi/acpi_processor.c: In function 'acpi_processor_ids_walk':
>> drivers//acpi/acpi_processor.c:646:3: error: implicit declaration of function 'acpi_ut_status_exit' [-Werror=implicit-function-declaration]
return_ACPI_STATUS(status);
^
cc1: some warnings being treated as errors
vim +/acpi_ut_status_exit +646 drivers//acpi/acpi_processor.c
632
633 static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
634 u32 lvl,
635 void *context,
636 void **rv)
637 {
638 acpi_status status;
639 acpi_object_type acpi_type;
640 unsigned long long uid;
641 union acpi_object object = { 0 };
642 struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
643
644 status = acpi_get_type(handle, &acpi_type);
645 if (ACPI_FAILURE(status))
> 646 return_ACPI_STATUS(status);
647
648 switch (acpi_type) {
649 case ACPI_TYPE_PROCESSOR:
650 status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
651 if (ACPI_FAILURE(status))
652 goto err;
653 uid = object.processor.proc_id;
654 break;
655
656 case ACPI_TYPE_DEVICE:
657 status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
658 if (ACPI_FAILURE(status))
659 goto err;
660 break;
661 default:
662 goto err;
663 }
664
665 processor_validated_ids_update(uid);
666 return AE_OK;
667
668 err:
669 /* Exit on error, but don't abort the namespace walk */
670 acpi_handle_info(handle, "Invalid processor object\n");
671 return AE_OK;
672
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation