We found there are some processors which have the same processor ID
but in different PXM in the ACPI namespace. such as this:

proc_id   |    pxm
--------------------
0       <->     0
1       <->     0
2       <->     1
3       <->     1
......
89      <->     0
89      <->     1
89      <->     2
89      <->     3
......

So we create a mechanism to validate them. make the processor(ID=89)
as invalid. And once a processor be hotplugged physically, we check its
processor id.

    Commit 8e089eaa1999 ("acpi: Provide mechanism to validate processors in the 
ACPI tables")
    Commit a77d6cd96849 ("acpi/processor: Check for duplicate processor ids at 
hotplug time")

Recently, I found the check mechanism has a bug, it didn't use the
acpi_processor_ids_walk() right and always gave us a wrong result.
First, I fixed it by modifying the value with AE_OK which is the
standard acpi_status value.

    https://lkml.org/lkml/2018/3/20/273

But, now, I even think this check is useless. my reasons are following:

    1). Based on the practical tests, It works well, and no bug be reported
    2). Based on the code, the duplicate cases can be dealed with by

       if (invalid_logical_cpuid(pr->id) || !cpu_present(pr->id))

That seems more reasonable, let's see the following case:

                          Before the patch,             After the patch

the first processor(ID=89)
hot-add                        failed                        success

the others processor(ID=89)
hot-add                        failed                        failed

So, Remove the check code.

Signed-off-by: Dou Liyang <[email protected]>

Signed-off-by: Dou Liyang <[email protected]>
---
 drivers/acpi/acpi_processor.c | 126 ------------------------------------------
 include/linux/acpi.h          |   3 -
 2 files changed, 129 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 449d86d39965..8358708e0bbb 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -281,13 +281,6 @@ static int acpi_processor_get_info(struct acpi_device 
*device)
                pr->acpi_id = value;
        }
 
-       if (acpi_duplicate_processor_id(pr->acpi_id)) {
-               dev_err(&device->dev,
-                       "Failed to get unique processor _UID (0x%x)\n",
-                       pr->acpi_id);
-               return -ENODEV;
-       }
-
        pr->phys_id = acpi_get_phys_id(pr->handle, device_declaration,
                                        pr->acpi_id);
        if (invalid_phys_cpuid(pr->phys_id))
@@ -579,127 +572,8 @@ static struct acpi_scan_handler 
processor_container_handler = {
        .attach = acpi_processor_container_attach,
 };
 
-/* The number of the unique processor IDs */
-static int nr_unique_ids __initdata;
-
-/* The number of the duplicate processor IDs */
-static int nr_duplicate_ids;
-
-/* Used to store the unique processor IDs */
-static int unique_processor_ids[] __initdata = {
-       [0 ... NR_CPUS - 1] = -1,
-};
-
-/* Used to store the duplicate processor IDs */
-static int duplicate_processor_ids[] = {
-       [0 ... NR_CPUS - 1] = -1,
-};
-
-static void __init processor_validated_ids_update(int proc_id)
-{
-       int i;
-
-       if (nr_unique_ids == NR_CPUS||nr_duplicate_ids == NR_CPUS)
-               return;
-
-       /*
-        * Firstly, compare the proc_id with duplicate IDs, if the proc_id is
-        * already in the IDs, do nothing.
-        */
-       for (i = 0; i < nr_duplicate_ids; i++) {
-               if (duplicate_processor_ids[i] == proc_id)
-                       return;
-       }
-
-       /*
-        * Secondly, compare the proc_id with unique IDs, if the proc_id is in
-        * the IDs, put it in the duplicate IDs.
-        */
-       for (i = 0; i < nr_unique_ids; i++) {
-               if (unique_processor_ids[i] == proc_id) {
-                       duplicate_processor_ids[nr_duplicate_ids] = proc_id;
-                       nr_duplicate_ids++;
-                       return;
-               }
-       }
-
-       /*
-        * Lastly, the proc_id is a unique ID, put it in the unique IDs.
-        */
-       unique_processor_ids[nr_unique_ids] = proc_id;
-       nr_unique_ids++;
-}
-
-static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
-                                                 u32 lvl,
-                                                 void *context,
-                                                 void **rv)
-{
-       acpi_status status;
-       acpi_object_type acpi_type;
-       unsigned long long uid;
-       union acpi_object object = { 0 };
-       struct acpi_buffer buffer = { sizeof(union acpi_object), &object };
-
-       status = acpi_get_type(handle, &acpi_type);
-       if (ACPI_FAILURE(status))
-               return false;
-
-       switch (acpi_type) {
-       case ACPI_TYPE_PROCESSOR:
-               status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
-               if (ACPI_FAILURE(status))
-                       goto err;
-               uid = object.processor.proc_id;
-               break;
-
-       case ACPI_TYPE_DEVICE:
-               status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
-               if (ACPI_FAILURE(status))
-                       goto err;
-               break;
-       default:
-               goto err;
-       }
-
-       processor_validated_ids_update(uid);
-       return true;
-
-err:
-       acpi_handle_info(handle, "Invalid processor object\n");
-       return false;
-
-}
-
-static void __init acpi_processor_check_duplicates(void)
-{
-       /* check the correctness for all processors in ACPI namespace */
-       acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
-                                               ACPI_UINT32_MAX,
-                                               acpi_processor_ids_walk,
-                                               NULL, NULL, NULL);
-       acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID, acpi_processor_ids_walk,
-                                               NULL, NULL);
-}
-
-bool acpi_duplicate_processor_id(int proc_id)
-{
-       int i;
-
-       /*
-        * compare the proc_id with duplicate IDs, if the proc_id is already
-        * in the duplicate IDs, return true, otherwise, return false.
-        */
-       for (i = 0; i < nr_duplicate_ids; i++) {
-               if (duplicate_processor_ids[i] == proc_id)
-                       return true;
-       }
-       return false;
-}
-
 void __init acpi_processor_init(void)
 {
-       acpi_processor_check_duplicates();
        acpi_scan_add_handler_with_hotplug(&processor_handler, "processor");
        acpi_scan_add_handler(&processor_container_handler);
 }
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 15bfb15c2fa5..068dcfe6768b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -285,9 +285,6 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id)
        return phys_id == PHYS_CPUID_INVALID;
 }
 
-/* Validate the processor object's proc_id */
-bool acpi_duplicate_processor_id(int proc_id);
-
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */
 int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id,
-- 
2.14.3



Reply via email to