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

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

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

> [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 <mingo@elte.hu>
> 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

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

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

[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 <mingo@elte.hu>
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 <yinghai.lu@sun.com>

Index: linux-2.6/arch/x86/mm/srat_64.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/srat_64.c
+++ 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);
Index: linux-2.6/drivers/acpi/tables.c
===================================================================
--- linux-2.6.orig/drivers/acpi/tables.c
+++ 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;

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

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

> > 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.

> 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

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

[Yinghai Lu]

On Thursday 17 January 2008 12:43:43 pm Ingo Molnar wrote:
> 
> * Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
> 
> > [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 <mingo@elte.hu>
> > 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
> 
> do you mean your patch should be applied first, then the 
> d8d28f25f33c6a03 patch should applied as second?

d8d28f25f33c6a03  is not needed

> 
> 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