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

Groups > linux.kernel > #1383805 > unrolled thread

Re: [PATCH] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present

Started by"Rafael J. Wysocki" <rjw@rjwysocki.net>
First post2016-04-21 02:10 +0200
Last post2016-04-21 11:30 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-04-21 02:10 +0200
    [PATCH v2] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> - 2016-04-21 11:30 +0200
      Re: [PATCH v2] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-04-27 23:30 +0200
    Re: [PATCH] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> - 2016-04-21 11:30 +0200

#1383805 — Re: [PATCH] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-04-21 02:10 +0200
SubjectRe: [PATCH] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present
Message-ID<rqalY-8hU-9@gated-at.bofh.it>
On Thursday, April 07, 2016 10:21:28 AM Lukasz Anaczkowski wrote:
> SRAT maps APIC ID to proximity domains ids. Mapping from proximity
> domain ids to NUMA node ids is based on order of entries in SRAT table.
> SRAT table has just LAPIC entires or mix of LAPIC and X2APIC entries.
> As long as there are only LAPIC entires, mapping from proximity domain
> id to NUMA node id is as assumed by BIOS. However, once APIC entries are
> mixed, X2APIC entries would be first mapped which causes unexpected NUMA
> node mapping.

The changelog describes the problem (good), but it doesn't say anything
about the mitigation (not good).  You should say something about what the
patch does to address the issue, like "To address that problem, do X".

> Fixes: d81056b5278 (Handle apic/x2apic entries in MADT in correct order)
> Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
> ---
>  drivers/acpi/numa.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index 72b6e9e..d176e0e 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -327,10 +327,18 @@ int __init acpi_numa_init(void)
>  
>  	/* SRAT: Static Resource Affinity Table */
>  	if (!acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) {
> -		acpi_table_parse_srat(ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY,
> -				     acpi_parse_x2apic_affinity, 0);
> -		acpi_table_parse_srat(ACPI_SRAT_TYPE_CPU_AFFINITY,
> -				     acpi_parse_processor_affinity, 0);
> +		struct acpi_subtable_proc srat_proc[2];
> +
> +		memset(srat_proc, 0, sizeof(srat_proc));
> +		srat_proc[0].id = ACPI_SRAT_TYPE_CPU_AFFINITY;
> +		srat_proc[0].handler = acpi_parse_processor_affinity;
> +		srat_proc[1].id = ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY;
> +		srat_proc[1].handler = acpi_parse_x2apic_affinity;
> +
> +		acpi_table_parse_entries_array(ACPI_SIG_SRAT,
> +					sizeof(struct acpi_table_srat),
> +					srat_proc, ARRAY_SIZE(srat_proc), 0);
> +

Looking at the code, the idea is to check each entry against both LAPIC and
X2APIC in table order so as to cause them to be mapped in that order too, right?

>  		cnt = acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
>  					    acpi_parse_memory_affinity,
>  					    NR_NODE_MEMBLKS);
> 

Thanks,
Rafael

[toc] | [next] | [standalone]

#1383988 — [PATCH v2] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present

FromLukasz Anaczkowski <lukasz.anaczkowski@intel.com>
Date2016-04-21 11:30 +0200
Subject[PATCH v2] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present
Message-ID<rqj5T-6Uw-1@gated-at.bofh.it>
In reply to#1383805
SRAT maps APIC ID to proximity domains ids (PXM). Mapping from PXM to
NUMA node ids is based on order of entries in SRAT table.
SRAT table has just LAPIC entires or mix of LAPIC and X2APIC entries.
As long as there are only LAPIC entires, mapping from proximity domain
id to NUMA node id is as assumed by BIOS. However, once APIC entries are
mixed, X2APIC entries would be first mapped which causes unexpected NUMA
node mapping.
To fix that, change parsing to check each entry against both LAPIC and
X2APIC so mapping is in the SRAT/PXM order.
This is supplemental change to the fix made by d81056b5278 (thus
"Fixes:" tag) and using same mechanism introduced by 9b3fedd (ACPI / tables:
Add acpi_subtable_proc to ACPI table parsers).

Fixes: d81056b5278 (Handle apic/x2apic entries in MADT in correct order)
Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
---
 drivers/acpi/numa.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 72b6e9e..d176e0e 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -327,10 +327,18 @@ int __init acpi_numa_init(void)
 
 	/* SRAT: Static Resource Affinity Table */
 	if (!acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) {
-		acpi_table_parse_srat(ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY,
-				     acpi_parse_x2apic_affinity, 0);
-		acpi_table_parse_srat(ACPI_SRAT_TYPE_CPU_AFFINITY,
-				     acpi_parse_processor_affinity, 0);
+		struct acpi_subtable_proc srat_proc[2];
+
+		memset(srat_proc, 0, sizeof(srat_proc));
+		srat_proc[0].id = ACPI_SRAT_TYPE_CPU_AFFINITY;
+		srat_proc[0].handler = acpi_parse_processor_affinity;
+		srat_proc[1].id = ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY;
+		srat_proc[1].handler = acpi_parse_x2apic_affinity;
+
+		acpi_table_parse_entries_array(ACPI_SIG_SRAT,
+					sizeof(struct acpi_table_srat),
+					srat_proc, ARRAY_SIZE(srat_proc), 0);
+
 		cnt = acpi_table_parse_srat(ACPI_SRAT_TYPE_MEMORY_AFFINITY,
 					    acpi_parse_memory_affinity,
 					    NR_NODE_MEMBLKS);
-- 
1.8.3.1

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

#1389594 — Re: [PATCH v2] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-04-27 23:30 +0200
SubjectRe: [PATCH v2] ACPI/SRAT: fix SRAT order parsing when both LAPIC and X2APIC present
Message-ID<rsFbY-8m2-7@gated-at.bofh.it>
In reply to#1383988
On Thursday, April 21, 2016 11:29:00 AM Lukasz Anaczkowski wrote:
> SRAT maps APIC ID to proximity domains ids (PXM). Mapping from PXM to
> NUMA node ids is based on order of entries in SRAT table.
> SRAT table has just LAPIC entires or mix of LAPIC and X2APIC entries.
> As long as there are only LAPIC entires, mapping from proximity domain
> id to NUMA node id is as assumed by BIOS. However, once APIC entries are
> mixed, X2APIC entries would be first mapped which causes unexpected NUMA
> node mapping.
> To fix that, change parsing to check each entry against both LAPIC and
> X2APIC so mapping is in the SRAT/PXM order.
> This is supplemental change to the fix made by d81056b5278 (thus
> "Fixes:" tag) and using same mechanism introduced by 9b3fedd (ACPI / tables:
> Add acpi_subtable_proc to ACPI table parsers).
> 
> Fixes: d81056b5278 (Handle apic/x2apic entries in MADT in correct order)
> Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>

Applied, thanks!

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

#1383997

FromLukasz Anaczkowski <lukasz.anaczkowski@intel.com>
Date2016-04-21 11:30 +0200
Message-ID<rqj5T-6Uw-3@gated-at.bofh.it>
In reply to#1383805
From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
Sent: Thursday, April 21, 2016 2:08 AM
> The changelog describes the problem (good), but it doesn't say anything
> about the mitigation (not good).  You should say something about what the
> patch does to address the issue, like "To address that problem, do X".

Thanks, Rafael. Sending v2 of the patch with extended commit message.

> Looking at the code, the idea is to check each entry against both LAPIC and
> X2APIC in table order so as to cause them to be mapped in that order too, right?

Yes, that is correct.

Thanks,
Lukasz

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web