Skip to contentSkip to conversation

x86: copy srat table and unmap in acpi_parse_table

PATCH In review Yinghai Lu 2 participants 4 messages Jan 17, 2008

Subsystems (3)
x86 ACPI x86 MM
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 x86: copy srat table and unmap in acpi_parse_table

[PATCH] x86: copy srat table and unmap in acpi_parse_table

the old acpi_numa_slit_init was saving old address in early stage acpi_slit and acpi_parse_table can not unmap address that.
the patch copy the slit in the callback,
so we could unmap table in acpi_parse_table instead of outside track it.

need to revert
"
commit d8d28f25f33c6a035cdfb1d421c79293d16e5c58

Author: Ingo Molnar <[email protected]>
Date:   Thu Jan 17 15:26:42 2008 +0100

x86: ACPI: fix mapping leaks

ioremap_early() is stateful, hence we cannot tolerate mapping leaks.
"

before appling this patch

Signed-off-by Yinghai Lu <[email protected]>
arch/x86/mm/srat_64.c +16 -5
Index: linux-2.6/arch/x86/mm/srat_64.c
===================================================================
@@ -23,7 +23,9 @@
 
 int acpi_numa __initdata;
 
-static struct acpi_table_slit *acpi_slit;
+static int slit_copied;
+static u64 slit_locality_count;
+static u8 slit_entry[MAX_NUMNODES * MAX_NUMNODES];
 
 static nodemask_t nodes_parsed __initdata;
 static struct bootnode nodes[MAX_NUMNODES] __initdata;

@@ -130,7 +132,16 @@ void __init acpi_numa_slit_init(struct a
 		printk(KERN_INFO "ACPI: SLIT table looks invalid. Not used.\n");
 		return;
 	}
-	acpi_slit = slit;
+
+	if (slit->locality_count > MAX_NUMNODES)
+		return;
+
+	slit_locality_count = slit->locality_count;
+
+	memcpy(slit_entry, slit->entry,
+	       slit_locality_count * slit_locality_count);
+
+	slit_copied = 1;
 }
 
 /* Callback for Proximity Domain -> LAPIC mapping */

@@ -502,11 +513,11 @@ int __node_distance(int a, int b)
 {
 	int index;
 
-	if (!acpi_slit)
+	if (!slit_copied)
 		return null_slit_node_compare(a, b) ? LOCAL_DISTANCE :
 						      REMOTE_DISTANCE;
-	index = acpi_slit->locality_count * node_to_pxm(a);
-	return acpi_slit->entry[index + node_to_pxm(b)];
+	index = slit_locality_count * node_to_pxm(a);
+	return slit_entry[index + node_to_pxm(b)];
 }
 
 EXPORT_SYMBOL(__node_distance);
drivers/acpi/tables.c +1 -0
Index: linux-2.6/drivers/acpi/tables.c
===================================================================
@@ -260,6 +260,7 @@ int __init acpi_table_parse(char *id, ac
 
 	if (table) {
 		handler(table);
+		acpi_os_unmap_memory(table, table->length);
 		return 0;
 	} else
 		return 1;
#1 lore raw
Quoted 14 lines · Yinghai Lu

[PATCH] x86: copy srat table and unmap in acpi_parse_table

the old acpi_numa_slit_init was saving old address in early stage acpi_slit and acpi_parse_table can not unmap address that. the patch copy the slit in the callback, so we could unmap table in acpi_parse_table instead of outside track it.

need to revert
"
commit d8d28f25f33c6a035cdfb1d421c79293d16e5c58

Author: Ingo Molnar <[email protected]>
Date:   Thu Jan 17 15:26:42 2008 +0100
Quoted 6 lines

x86: ACPI: fix mapping leaks

ioremap_early() is stateful, hence we cannot tolerate mapping leaks.
"

before appling this patch

do you mean your patch should be applied first, then the d8d28f25f33c6a03 patch should applied as second?

Or if d8d28f25f33c6a03 really needs to be reverted to get your system to boot properly, which particular bit of it was causing trouble for you? (or the whole thing?)

Ingo

author
#2 lore raw
Quoted 19 lines · Ingo Molnar
Quoted 16 lines · Yinghai Lu

[PATCH] x86: copy srat table and unmap in acpi_parse_table

the old acpi_numa_slit_init was saving old address in early stage acpi_slit and acpi_parse_table can not unmap address that. the patch copy the slit in the callback, so we could unmap table in acpi_parse_table instead of outside track it.

need to revert
"
commit d8d28f25f33c6a035cdfb1d421c79293d16e5c58

Author: Ingo Molnar <[email protected]>
Date:   Thu Jan 17 15:26:42 2008 +0100

x86: ACPI: fix mapping leaks

Quoted 7 lines
Quoted 4 lines

ioremap_early() is stateful, hence we cannot tolerate mapping leaks.
"

before appling this patch

do you mean your patch should be applied first, then the d8d28f25f33c6a03 patch should applied as second?

d8d28f25f33c6a03 is not needed

Quoted 4 lines

Or if d8d28f25f33c6a03 really needs to be reverted to get your system to boot properly, which particular bit of it was causing trouble for you? (or the whole thing?)

we need to call acpi_os_unmap_memory in acpi_table_parse or just after it.

call it in acpi_table_parse is much cleaner

the problem is that acpi_numa_slit_init do bad assumaption that it still can use address after it's callback function. so we need to copy the slit.

YH

#3 lore raw
Quoted 8 lines · Yinghai Lu
Quoted 3 lines

Or if d8d28f25f33c6a03 really needs to be reverted to get your system to boot properly, which particular bit of it was causing trouble for you? (or the whole thing?)

we need to call acpi_os_unmap_memory in acpi_table_parse or just after it.

call it in acpi_table_parse is much cleaner

agreed - i had a second look at your patch and it's much cleaner.

Quoted 3 lines

the problem is that acpi_numa_slit_init do bad assumaption that it still can use address after it's callback function. so we need to copy the slit.

ok. I've applied your patch and boot-tested it already - it should show up soon in the next x86.git drop.

Ingo