Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1584412 > unrolled thread

[PATCH v2 0/4] Revert works for the mapping of cpuid <-> nodeid

Started byDou Liyang <douly.fnst@cn.fujitsu.com>
First post2017-02-20 09:50 +0100
Last post2017-02-22 03:00 +0100
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/4] Revert works for the mapping of cpuid <-> nodeid Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-02-20 09:50 +0100
    [PATCH v2 3/4] acpi: Fix the check handle in case of declaring processors using the Device operator Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-02-20 09:50 +0100
    [PATCH v2 1/4] Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting" Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-02-20 09:50 +0100
    [PATCH v2 2/4] Revert"x86/acpi: Enable MADT APIs to return disabled apicids" Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-02-20 09:50 +0100
    Re: [PATCH v2 0/4] Revert works for the mapping of cpuid <-> nodeid Ye Xiaolong <xiaolong.ye@intel.com> - 2017-02-21 02:10 +0100
      Re: [PATCH v2 0/4] Revert works for the mapping of cpuid <-> nodeid Ye Xiaolong <xiaolong.ye@intel.com> - 2017-02-21 08:20 +0100
        Re: [PATCH v2 0/4] Revert works for the mapping of cpuid <-> nodeid Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-02-22 03:00 +0100

#1584412 — [PATCH v2 0/4] Revert works for the mapping of cpuid <-> nodeid

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-02-20 09:50 +0100
Subject[PATCH v2 0/4] Revert works for the mapping of cpuid <-> nodeid
Message-ID<tcRPr-2l8-7@gated-at.bofh.it>
Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time.
It keeps consistent with the WorkQueue and avoids some bugs which may be caused
by the dynamic assignment.
As we know, It is implemented by the patches as follows: 2532fc318d, f7c28833c2,
8f54969dc8, 8ad893faf2, dc6db24d24, which depend on ACPI table. Simply speaking:

Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT:
We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and
get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT.
So, we get the mapping of
*Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*

Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT:
The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in
each entities. we just use it directly.

So, at last we get the maaping of *Node ID <-> Logical CPU ID* according to
step1 and step2:
*Node ID(_PXM) <-> Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*

But, The ACPI table is unreliable and it is very risky that we use the entity
which isn't related to a physical device at booting time. Here has already two
bugs we found.
1. Duplicated Processor IDs in DSDT.
	It has been fixed by commit 8e089eaa19, fd74da217d.
2. The _PXM in DSDT is inconsistent with the one in MADT.
	It may cause the bug, which is shown in:
		https://lkml.org/lkml/2017/2/12/200
There may be more later. We shouldn't just only fix them everytime, we should
solve this problem from the source to avoid such problems happend again and
again.

Now, a simple and easy way is found, we revert our patches. Do the Step 2 
at hot-plug time, not at booting time where we did some useless work.

It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive
use of the ACPI table.

We have tested them in our box: Fujitsu PQ2000 with 2 nodes for hot-plug.
To Xiaolong: 
	Please help me to test it in the special machine.

Change log:
  v1 -> v2: 1. fix some comments.
            2. add the verification of duplicate processor id.

Dou Liyang (4):
  Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
  Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
  acpi: Fix the check handle in case of declaring processors using the
    Device operator
  acpi: Move the verification of duplicate proc_id from booting time to
    hot-plug time

 arch/x86/kernel/acpi/boot.c   |   2 +-
 drivers/acpi/acpi_processor.c |  50 +++++++++++-----
 drivers/acpi/bus.c            |   1 -
 drivers/acpi/processor_core.c | 133 +++++++-----------------------------------
 include/linux/acpi.h          |   5 +-
 5 files changed, 59 insertions(+), 132 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1584413 — [PATCH v2 3/4] acpi: Fix the check handle in case of declaring processors using the Device operator

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-02-20 09:50 +0100
Subject[PATCH v2 3/4] acpi: Fix the check handle in case of declaring processors using the Device operator
Message-ID<tcRPr-2l8-11@gated-at.bofh.it>
In reply to#1584412
In ACPI spec, we can declare processors using both Processor and
Device operator. And before we use the ACPI table, we should check
the correctness for all processors in ACPI namespace.

But, Currently, the check handle is just include only the processors
which are declared by Processor operator. It misses the processors
declared by Device operator.

The patch adds the case of Device operator.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 drivers/acpi/acpi_processor.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index f43a586..eb500e1 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -633,25 +633,43 @@ static acpi_status __init acpi_processor_ids_walk(acpi_handle handle,
 						  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_evaluate_object(handle, NULL, NULL, &buffer);
-	if (ACPI_FAILURE(status))
-		acpi_handle_info(handle, "Not get the processor object\n");
-	else
-		processor_validated_ids_update(object.processor.proc_id);
+	status = acpi_get_type(handle, &acpi_type);
+	switch (acpi_type) {
+	case ACPI_TYPE_PROCESSOR:
+		status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+		if (ACPI_FAILURE(status))
+			acpi_handle_info(handle, "Not get the processor object\n");
+		else
+			processor_validated_ids_update(
+						object.processor.proc_id);
+		break;
+	case ACPI_TYPE_DEVICE:
+		status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
+		if (ACPI_FAILURE(status))
+			return false;
+		processor_validated_ids_update(uid);
+		break;
+	default:
+		return false;
+	}
 
 	return AE_OK;
 }
 
-static void __init acpi_processor_check_duplicates(void)
+void __init acpi_processor_check_duplicates(void)
 {
-	/* Search all processor nodes in ACPI namespace */
+	/* 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 __init acpi_processor_validate_proc_id(int proc_id)
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1584414 — [PATCH v2 1/4] Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-02-20 09:50 +0100
Subject[PATCH v2 1/4] Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
Message-ID<tcRPr-2l8-15@gated-at.bofh.it>
In reply to#1584412
Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time.
It keeps consistent with the WorkQueue and avoids some bugs which may be caused
by the dynamic assignment.

But, The ACPI table is unreliable and it is very risky that we use the entity
which isn't related to a physical device at booting time.

Now, we revert our patches. Do the last mapping of "cpuid <-> nodeid" at
hot-plug time, not at booting time where we did some useless work.
It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive
use of the ACPI table.

The patch revert the commit dc6db24d24:
  "x86/acpi: Set persistent cpuid <-> nodeid mapping when booting".

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/kernel/acpi/boot.c   |  2 +-
 drivers/acpi/acpi_processor.c |  5 ---
 drivers/acpi/bus.c            |  1 -
 drivers/acpi/processor_core.c | 73 -------------------------------------------
 include/linux/acpi.h          |  3 --
 5 files changed, 1 insertion(+), 83 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 64422f8..32846a2 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -709,7 +709,7 @@ static void __init acpi_set_irq_model_ioapic(void)
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 #include <acpi/processor.h>
 
-int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
+static int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
 {
 #ifdef CONFIG_ACPI_NUMA
 	int nid;
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 3de3b6b..f43a586 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -182,11 +182,6 @@ int __weak arch_register_cpu(int cpu)
 
 void __weak arch_unregister_cpu(int cpu) {}
 
-int __weak acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
-{
-	return -ENODEV;
-}
-
 static int acpi_processor_hotadd_init(struct acpi_processor *pr)
 {
 	unsigned long long sta;
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 95855cb..d4455e4 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1207,7 +1207,6 @@ static int __init acpi_init(void)
 	acpi_wakeup_device_init();
 	acpi_debugger_init();
 	acpi_setup_sb_notify_handler();
-	acpi_set_processor_mapping();
 	return 0;
 }
 
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 611a558..a843862 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -278,79 +278,6 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
 }
 EXPORT_SYMBOL_GPL(acpi_get_cpuid);
 
-#ifdef CONFIG_ACPI_HOTPLUG_CPU
-static bool __init
-map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
-{
-	int type, id;
-	u32 acpi_id;
-	acpi_status status;
-	acpi_object_type acpi_type;
-	unsigned long long tmp;
-	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))
-			return false;
-		acpi_id = object.processor.proc_id;
-
-		/* validate the acpi_id */
-		if(acpi_processor_validate_proc_id(acpi_id))
-			return false;
-		break;
-	case ACPI_TYPE_DEVICE:
-		status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
-		if (ACPI_FAILURE(status))
-			return false;
-		acpi_id = tmp;
-		break;
-	default:
-		return false;
-	}
-
-	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
-
-	*phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
-	id = acpi_map_cpuid(*phys_id, acpi_id);
-
-	if (id < 0)
-		return false;
-	*cpuid = id;
-	return true;
-}
-
-static acpi_status __init
-set_processor_node_mapping(acpi_handle handle, u32 lvl, void *context,
-			   void **rv)
-{
-	phys_cpuid_t phys_id;
-	int cpu_id;
-
-	if (!map_processor(handle, &phys_id, &cpu_id))
-		return AE_ERROR;
-
-	acpi_map_cpu2node(handle, cpu_id, phys_id);
-	return AE_OK;
-}
-
-void __init acpi_set_processor_mapping(void)
-{
-	/* Set persistent cpu <-> node mapping for all processors. */
-	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
-			    ACPI_UINT32_MAX, set_processor_node_mapping,
-			    NULL, NULL, NULL);
-}
-#else
-void __init acpi_set_processor_mapping(void) {}
-#endif /* CONFIG_ACPI_HOTPLUG_CPU */
-
 #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
 static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
 			 u64 *phys_addr, int *ioapic_id)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 5b36974..d180cbd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -293,11 +293,8 @@ bool acpi_processor_validate_proc_id(int proc_id);
 /* Arch dependent functions for cpu hotplug support */
 int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu);
 int acpi_unmap_cpu(int cpu);
-int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid);
 #endif /* CONFIG_ACPI_HOTPLUG_CPU */
 
-void acpi_set_processor_mapping(void);
-
 #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
 int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
 #endif
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1584416 — [PATCH v2 2/4] Revert"x86/acpi: Enable MADT APIs to return disabled apicids"

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-02-20 09:50 +0100
Subject[PATCH v2 2/4] Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
Message-ID<tcRPs-2l8-21@gated-at.bofh.it>
In reply to#1584412
After we never do the last mapping of "cpuid <-> nodeid" at booting time. we
also no need to enable MADT APIs to return disabled apicid.

So, The patch work for reverting the commit 8ad893faf2:
"x86/acpi: Enable MADT APIs to return disabled apicids"

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 drivers/acpi/processor_core.c | 60 ++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a843862..b933061 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -32,12 +32,12 @@ static struct acpi_table_madt *get_madt_table(void)
 }
 
 static int map_lapic_id(struct acpi_subtable_header *entry,
-		 u32 acpi_id, phys_cpuid_t *apic_id, bool ignore_disabled)
+		 u32 acpi_id, phys_cpuid_t *apic_id)
 {
 	struct acpi_madt_local_apic *lapic =
 		container_of(entry, struct acpi_madt_local_apic, header);
 
-	if (ignore_disabled && !(lapic->lapic_flags & ACPI_MADT_ENABLED))
+	if (!(lapic->lapic_flags & ACPI_MADT_ENABLED))
 		return -ENODEV;
 
 	if (lapic->processor_id != acpi_id)
@@ -48,13 +48,12 @@ static int map_lapic_id(struct acpi_subtable_header *entry,
 }
 
 static int map_x2apic_id(struct acpi_subtable_header *entry,
-		int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id,
-		bool ignore_disabled)
+		int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id)
 {
 	struct acpi_madt_local_x2apic *apic =
 		container_of(entry, struct acpi_madt_local_x2apic, header);
 
-	if (ignore_disabled && !(apic->lapic_flags & ACPI_MADT_ENABLED))
+	if (!(apic->lapic_flags & ACPI_MADT_ENABLED))
 		return -ENODEV;
 
 	if (device_declaration && (apic->uid == acpi_id)) {
@@ -66,13 +65,12 @@ static int map_x2apic_id(struct acpi_subtable_header *entry,
 }
 
 static int map_lsapic_id(struct acpi_subtable_header *entry,
-		int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id,
-		bool ignore_disabled)
+		int device_declaration, u32 acpi_id, phys_cpuid_t *apic_id)
 {
 	struct acpi_madt_local_sapic *lsapic =
 		container_of(entry, struct acpi_madt_local_sapic, header);
 
-	if (ignore_disabled && !(lsapic->lapic_flags & ACPI_MADT_ENABLED))
+	if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
 		return -ENODEV;
 
 	if (device_declaration) {
@@ -89,13 +87,12 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
  * Retrieve the ARM CPU physical identifier (MPIDR)
  */
 static int map_gicc_mpidr(struct acpi_subtable_header *entry,
-		int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr,
-		bool ignore_disabled)
+		int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr)
 {
 	struct acpi_madt_generic_interrupt *gicc =
 	    container_of(entry, struct acpi_madt_generic_interrupt, header);
 
-	if (ignore_disabled && !(gicc->flags & ACPI_MADT_ENABLED))
+	if (!(gicc->flags & ACPI_MADT_ENABLED))
 		return -ENODEV;
 
 	/* device_declaration means Device object in DSDT, in the
@@ -112,7 +109,7 @@ static int map_gicc_mpidr(struct acpi_subtable_header *entry,
 }
 
 static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt,
-				   int type, u32 acpi_id, bool ignore_disabled)
+				   int type, u32 acpi_id)
 {
 	unsigned long madt_end, entry;
 	phys_cpuid_t phys_id = PHYS_CPUID_INVALID;	/* CPU hardware ID */
@@ -130,20 +127,16 @@ static phys_cpuid_t map_madt_entry(struct acpi_table_madt *madt,
 		struct acpi_subtable_header *header =
 			(struct acpi_subtable_header *)entry;
 		if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
-			if (!map_lapic_id(header, acpi_id, &phys_id,
-					  ignore_disabled))
+			if (!map_lapic_id(header, acpi_id, &phys_id))
 				break;
 		} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
-			if (!map_x2apic_id(header, type, acpi_id, &phys_id,
-					   ignore_disabled))
+			if (!map_x2apic_id(header, type, acpi_id, &phys_id))
 				break;
 		} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
-			if (!map_lsapic_id(header, type, acpi_id, &phys_id,
-					   ignore_disabled))
+			if (!map_lsapic_id(header, type, acpi_id, &phys_id))
 				break;
 		} else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
-			if (!map_gicc_mpidr(header, type, acpi_id, &phys_id,
-					    ignore_disabled))
+			if (!map_gicc_mpidr(header, type, acpi_id, &phys_id))
 				break;
 		}
 		entry += header->length;
@@ -161,15 +154,14 @@ phys_cpuid_t __init acpi_map_madt_entry(u32 acpi_id)
 	if (!madt)
 		return PHYS_CPUID_INVALID;
 
-	rv = map_madt_entry(madt, 1, acpi_id, true);
+	rv = map_madt_entry(madt, 1, acpi_id);
 
 	acpi_put_table((struct acpi_table_header *)madt);
 
 	return rv;
 }
 
-static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id,
-				  bool ignore_disabled)
+static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
@@ -190,38 +182,30 @@ static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id,
 
 	header = (struct acpi_subtable_header *)obj->buffer.pointer;
 	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
-		map_lapic_id(header, acpi_id, &phys_id, ignore_disabled);
+		map_lapic_id(header, acpi_id, &phys_id);
 	else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
-		map_lsapic_id(header, type, acpi_id, &phys_id, ignore_disabled);
+		map_lsapic_id(header, type, acpi_id, &phys_id);
 	else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
-		map_x2apic_id(header, type, acpi_id, &phys_id, ignore_disabled);
+		map_x2apic_id(header, type, acpi_id, &phys_id);
 	else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT)
-		map_gicc_mpidr(header, type, acpi_id, &phys_id,
-			       ignore_disabled);
+		map_gicc_mpidr(header, type, acpi_id, &phys_id);
 
 exit:
 	kfree(buffer.pointer);
 	return phys_id;
 }
 
-static phys_cpuid_t __acpi_get_phys_id(acpi_handle handle, int type,
-				       u32 acpi_id, bool ignore_disabled)
+phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id)
 {
 	phys_cpuid_t phys_id;
 
-	phys_id = map_mat_entry(handle, type, acpi_id, ignore_disabled);
+	phys_id = map_mat_entry(handle, type, acpi_id);
 	if (invalid_phys_cpuid(phys_id))
-		phys_id = map_madt_entry(get_madt_table(), type, acpi_id,
-					   ignore_disabled);
+		phys_id = map_madt_entry(get_madt_table(), type, acpi_id);
 
 	return phys_id;
 }
 
-phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id)
-{
-	return __acpi_get_phys_id(handle, type, acpi_id, true);
-}
-
 int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id)
 {
 #ifdef CONFIG_SMP
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1584984

FromYe Xiaolong <xiaolong.ye@intel.com>
Date2017-02-21 02:10 +0100
Message-ID<td77Q-3PO-7@gated-at.bofh.it>
In reply to#1584412
On 02/20, Dou Liyang wrote:
>Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time.
>It keeps consistent with the WorkQueue and avoids some bugs which may be caused
>by the dynamic assignment.
>As we know, It is implemented by the patches as follows: 2532fc318d, f7c28833c2,
>8f54969dc8, 8ad893faf2, dc6db24d24, which depend on ACPI table. Simply speaking:
>
>Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT:
>We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and
>get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT.
>So, we get the mapping of
>*Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>
>Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT:
>The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in
>each entities. we just use it directly.
>
>So, at last we get the maaping of *Node ID <-> Logical CPU ID* according to
>step1 and step2:
>*Node ID(_PXM) <-> Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>
>But, The ACPI table is unreliable and it is very risky that we use the entity
>which isn't related to a physical device at booting time. Here has already two
>bugs we found.
>1. Duplicated Processor IDs in DSDT.
>	It has been fixed by commit 8e089eaa19, fd74da217d.
>2. The _PXM in DSDT is inconsistent with the one in MADT.
>	It may cause the bug, which is shown in:
>		https://lkml.org/lkml/2017/2/12/200
>There may be more later. We shouldn't just only fix them everytime, we should
>solve this problem from the source to avoid such problems happend again and
>again.
>
>Now, a simple and easy way is found, we revert our patches. Do the Step 2 
>at hot-plug time, not at booting time where we did some useless work.
>
>It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive
>use of the ACPI table.
>
>We have tested them in our box: Fujitsu PQ2000 with 2 nodes for hot-plug.
>To Xiaolong: 
>	Please help me to test it in the special machine.

Got it, I'll queue the tests on the previous machine and let you know the result
once I get it.

Thanks,
Xiaolong
>
>Change log:
>  v1 -> v2: 1. fix some comments.
>            2. add the verification of duplicate processor id.
>
>Dou Liyang (4):
>  Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
>  Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
>  acpi: Fix the check handle in case of declaring processors using the
>    Device operator
>  acpi: Move the verification of duplicate proc_id from booting time to
>    hot-plug time
>
> arch/x86/kernel/acpi/boot.c   |   2 +-
> drivers/acpi/acpi_processor.c |  50 +++++++++++-----
> drivers/acpi/bus.c            |   1 -
> drivers/acpi/processor_core.c | 133 +++++++-----------------------------------
> include/linux/acpi.h          |   5 +-
> 5 files changed, 59 insertions(+), 132 deletions(-)
>
>-- 
>2.5.5
>
>
>

[toc] | [prev] | [next] | [standalone]

#1585088

FromYe Xiaolong <xiaolong.ye@intel.com>
Date2017-02-21 08:20 +0100
Message-ID<tdcTU-7HX-5@gated-at.bofh.it>
In reply to#1584984
On 02/21, Ye Xiaolong wrote:
>On 02/20, Dou Liyang wrote:
>>Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time.
>>It keeps consistent with the WorkQueue and avoids some bugs which may be caused
>>by the dynamic assignment.
>>As we know, It is implemented by the patches as follows: 2532fc318d, f7c28833c2,
>>8f54969dc8, 8ad893faf2, dc6db24d24, which depend on ACPI table. Simply speaking:
>>
>>Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT:
>>We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and
>>get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT.
>>So, we get the mapping of
>>*Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>>
>>Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT:
>>The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in
>>each entities. we just use it directly.
>>
>>So, at last we get the maaping of *Node ID <-> Logical CPU ID* according to
>>step1 and step2:
>>*Node ID(_PXM) <-> Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>>
>>But, The ACPI table is unreliable and it is very risky that we use the entity
>>which isn't related to a physical device at booting time. Here has already two
>>bugs we found.
>>1. Duplicated Processor IDs in DSDT.
>>	It has been fixed by commit 8e089eaa19, fd74da217d.
>>2. The _PXM in DSDT is inconsistent with the one in MADT.
>>	It may cause the bug, which is shown in:
>>		https://lkml.org/lkml/2017/2/12/200
>>There may be more later. We shouldn't just only fix them everytime, we should
>>solve this problem from the source to avoid such problems happend again and
>>again.
>>
>>Now, a simple and easy way is found, we revert our patches. Do the Step 2 
>>at hot-plug time, not at booting time where we did some useless work.
>>
>>It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive
>>use of the ACPI table.
>>
>>We have tested them in our box: Fujitsu PQ2000 with 2 nodes for hot-plug.
>>To Xiaolong: 
>>	Please help me to test it in the special machine.
>
>Got it, I'll queue the tests on the previous machine and let you know the result
>once I get it.

Previous kernel panic and incomplete run issue (described in [1]) in 0day
system is gone with this series.

Tested-by: Xiaolong Ye <xiaolong.ye@intel.com>

Here is the comparison:

$ compare -at dc6db24d2476cd09c0ecf2b8d80313539f737a89 2e61bac54fad4c018afd23c118bce2399e504020
tests: 1
testcase/path_params/tbox_group/run: vm-scalability/300-never-never-1-1-swap-w-rand-performance/lkp-hsw-ep2

Here dc6db24d24 is previous first bad commit, 2e61bac54 is the head commit of your series
applied on top of latest tip of linus/master c945d0227d ("Merge branch 'x86-platform-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip")

dc6db24d2476cd09  2e61bac54fad4c018afd23c118  
----------------  --------------------------  
       fail:runs  %reproduction    fail:runs
           |             |             |    
           :12          12%           1:8     last_state.OOM
           :12          12%           1:8     dmesg.page_allocation_failure:order:#,mode:#(GFP_USER|GFP_DMA32|__GFP_ZERO)
           :12          12%           1:8     dmesg.Mem-Info
         12:12        -100%            :8     dmesg.BUG:unable_to_handle_kernel
         12:12        -100%            :8     dmesg.Oops
         12:12        -100%            :8     dmesg.RIP:get_partial_node
          9:12         -75%            :8     dmesg.RIP:_raw_spin_lock_irqsave
          3:12         -25%            :8     dmesg.general_protection_fault:#[##]SMP
          3:12         -25%            :8     dmesg.RIP:native_queued_spin_lock_slowpath
          3:12         -25%            :8     dmesg.Kernel_panic-not_syncing:Hard_LOCKUP
          2:12         -17%            :8     dmesg.RIP:load_balance
          2:12         -17%            :8     dmesg.Kernel_panic-not_syncing:Fatal_exception_in_interrupt
          1:12          -8%            :8     dmesg.RIP:resched_curr
          1:12          -8%            :8     dmesg.Kernel_panic-not_syncing:Fatal_exception
          5:12         -42%            :8     dmesg.WARNING:at_include/linux/uaccess.h:#__probe_kernel_read
          1:12          -8%            :8     dmesg.WARNING:at_lib/list_debug.c:#__list_add


[1] https://lkml.org/lkml/2017/2/12/200

Thanks,
Xiaolong

>
>Thanks,
>Xiaolong
>>
>>Change log:
>>  v1 -> v2: 1. fix some comments.
>>            2. add the verification of duplicate processor id.
>>
>>Dou Liyang (4):
>>  Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
>>  Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
>>  acpi: Fix the check handle in case of declaring processors using the
>>    Device operator
>>  acpi: Move the verification of duplicate proc_id from booting time to
>>    hot-plug time
>>
>> arch/x86/kernel/acpi/boot.c   |   2 +-
>> drivers/acpi/acpi_processor.c |  50 +++++++++++-----
>> drivers/acpi/bus.c            |   1 -
>> drivers/acpi/processor_core.c | 133 +++++++-----------------------------------
>> include/linux/acpi.h          |   5 +-
>> 5 files changed, 59 insertions(+), 132 deletions(-)
>>
>>-- 
>>2.5.5
>>
>>
>>

[toc] | [prev] | [next] | [standalone]

#1585861

FromDou Liyang <douly.fnst@cn.fujitsu.com>
Date2017-02-22 03:00 +0100
Message-ID<tdunM-2ss-11@gated-at.bofh.it>
In reply to#1585088
Hi, Xiaolong

At 02/21/2017 03:10 PM, Ye Xiaolong wrote:
> On 02/21, Ye Xiaolong wrote:
>> On 02/20, Dou Liyang wrote:
>>> Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time.
>>> It keeps consistent with the WorkQueue and avoids some bugs which may be caused
>>> by the dynamic assignment.
>>> As we know, It is implemented by the patches as follows: 2532fc318d, f7c28833c2,
>>> 8f54969dc8, 8ad893faf2, dc6db24d24, which depend on ACPI table. Simply speaking:
>>>
>>> Step 1. Make the "Logical CPU ID <-> Processor ID/UID" fixed Using MADT:
>>> We generate the logical CPU IDs by the Local APIC/x2APIC IDs orderly and
>>> get the mapping of Processor ID/UID <-> Local Apic ID directly in MADT.
>>> So, we get the mapping of
>>> *Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>>>
>>> Step 2. Make the "Processor ID/UID <-> Node ID(_PXM)" fixed Using DSDT:
>>> The maaping of "Processor ID/UID <-> Node ID(_PXM)" is ready-made in
>>> each entities. we just use it directly.
>>>
>>> So, at last we get the maaping of *Node ID <-> Logical CPU ID* according to
>>> step1 and step2:
>>> *Node ID(_PXM) <-> Processor ID/UID <-> Local Apic ID <-> Logical CPU ID*
>>>
>>> But, The ACPI table is unreliable and it is very risky that we use the entity
>>> which isn't related to a physical device at booting time. Here has already two
>>> bugs we found.
>>> 1. Duplicated Processor IDs in DSDT.
>>> 	It has been fixed by commit 8e089eaa19, fd74da217d.
>>> 2. The _PXM in DSDT is inconsistent with the one in MADT.
>>> 	It may cause the bug, which is shown in:
>>> 		https://lkml.org/lkml/2017/2/12/200
>>> There may be more later. We shouldn't just only fix them everytime, we should
>>> solve this problem from the source to avoid such problems happend again and
>>> again.
>>>
>>> Now, a simple and easy way is found, we revert our patches. Do the Step 2
>>> at hot-plug time, not at booting time where we did some useless work.
>>>
>>> It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive
>>> use of the ACPI table.
>>>
>>> We have tested them in our box: Fujitsu PQ2000 with 2 nodes for hot-plug.
>>> To Xiaolong:
>>> 	Please help me to test it in the special machine.
>>
>> Got it, I'll queue the tests on the previous machine and let you know the result
>> once I get it.
>
> Previous kernel panic and incomplete run issue (described in [1]) in 0day
> system is gone with this series.
>

Thanks very much, I am glad to hear that!

> Tested-by: Xiaolong Ye <xiaolong.ye@intel.com>
>

I will add it in my next version.

Thanks,
Liyang

> Here is the comparison:
>
> $ compare -at dc6db24d2476cd09c0ecf2b8d80313539f737a89 2e61bac54fad4c018afd23c118bce2399e504020
> tests: 1
> testcase/path_params/tbox_group/run: vm-scalability/300-never-never-1-1-swap-w-rand-performance/lkp-hsw-ep2
>
> Here dc6db24d24 is previous first bad commit, 2e61bac54 is the head commit of your series
> applied on top of latest tip of linus/master c945d0227d ("Merge branch 'x86-platform-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip")
>
> dc6db24d2476cd09  2e61bac54fad4c018afd23c118
> ----------------  --------------------------
>        fail:runs  %reproduction    fail:runs
>            |             |             |
>            :12          12%           1:8     last_state.OOM
>            :12          12%           1:8     dmesg.page_allocation_failure:order:#,mode:#(GFP_USER|GFP_DMA32|__GFP_ZERO)
>            :12          12%           1:8     dmesg.Mem-Info
>          12:12        -100%            :8     dmesg.BUG:unable_to_handle_kernel
>          12:12        -100%            :8     dmesg.Oops
>          12:12        -100%            :8     dmesg.RIP:get_partial_node
>           9:12         -75%            :8     dmesg.RIP:_raw_spin_lock_irqsave
>           3:12         -25%            :8     dmesg.general_protection_fault:#[##]SMP
>           3:12         -25%            :8     dmesg.RIP:native_queued_spin_lock_slowpath
>           3:12         -25%            :8     dmesg.Kernel_panic-not_syncing:Hard_LOCKUP
>           2:12         -17%            :8     dmesg.RIP:load_balance
>           2:12         -17%            :8     dmesg.Kernel_panic-not_syncing:Fatal_exception_in_interrupt
>           1:12          -8%            :8     dmesg.RIP:resched_curr
>           1:12          -8%            :8     dmesg.Kernel_panic-not_syncing:Fatal_exception
>           5:12         -42%            :8     dmesg.WARNING:at_include/linux/uaccess.h:#__probe_kernel_read
>           1:12          -8%            :8     dmesg.WARNING:at_lib/list_debug.c:#__list_add
>
>
> [1] https://lkml.org/lkml/2017/2/12/200
>
> Thanks,
> Xiaolong
>
>>
>> Thanks,
>> Xiaolong
>>>
>>> Change log:
>>>  v1 -> v2: 1. fix some comments.
>>>            2. add the verification of duplicate processor id.
>>>
>>> Dou Liyang (4):
>>>  Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"
>>>  Revert"x86/acpi: Enable MADT APIs to return disabled apicids"
>>>  acpi: Fix the check handle in case of declaring processors using the
>>>    Device operator
>>>  acpi: Move the verification of duplicate proc_id from booting time to
>>>    hot-plug time
>>>
>>> arch/x86/kernel/acpi/boot.c   |   2 +-
>>> drivers/acpi/acpi_processor.c |  50 +++++++++++-----
>>> drivers/acpi/bus.c            |   1 -
>>> drivers/acpi/processor_core.c | 133 +++++++-----------------------------------
>>> include/linux/acpi.h          |   5 +-
>>> 5 files changed, 59 insertions(+), 132 deletions(-)
>>>
>>> --
>>> 2.5.5
>>>
>>>
>>>
>
>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web