2008-01-17 20:39:56

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH] 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]>

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;


2008-01-17 20:44:26

by Ingo Molnar

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


* Yinghai Lu <[email protected]> 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 <[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

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

2008-01-17 22:03:59

by Yinghai Lu

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

On Thursday 17 January 2008 12:43:43 pm Ingo Molnar wrote:
>
> * Yinghai Lu <[email protected]> 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 <[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
>
> 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

2008-01-17 22:06:58

by Ingo Molnar

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


* Yinghai Lu <[email protected]> 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