2007-05-05 00:04:33

by Andrew Morton

[permalink] [raw]
Subject: Re: [Patch] Fix section mismatch of memory hotplug related code.

On Thu, 05 Apr 2007 17:01:02 +0900
Yasunori Goto <[email protected]> wrote:

> Hello.
>
> This is to fix many section mismatches of code related to memory hotplug.
> I checked compile with memory hotplug on/off on ia64 and x86-64 box.
>
> ..
>
> ===================================================================
> --- meminit.orig/drivers/acpi/numa.c 2007-04-04 20:15:58.000000000 +0900
> +++ meminit/drivers/acpi/numa.c 2007-04-04 20:56:34.000000000 +0900
> @@ -228,7 +228,7 @@ int __init acpi_numa_init(void)
> return 0;
> }
>
> -int acpi_get_pxm(acpi_handle h)
> +int __meminit acpi_get_pxm(acpi_handle h)
> {
> unsigned long pxm;
> acpi_status status;
> @@ -246,7 +246,7 @@ int acpi_get_pxm(acpi_handle h)
> }
> EXPORT_SYMBOL(acpi_get_pxm);
>
> -int acpi_get_node(acpi_handle *handle)
> +int __meminit acpi_get_node(acpi_handle *handle)
> {
> int pxm, node = -1;

It doesn't make a lot of sense to export an __init symbol to modules. I
guess it's OK in this case, but we get warnings:

WARNING: drivers/built-in.o - Section mismatch: reference to .init.text:acpi_get_node from __ksymtab between '__ksymtab_acpi_get_node' (at offset 0x1040) and '__ksymtab_acpi_get_pxm'
WARNING: drivers/built-in.o - Section mismatch: reference to .init.text:acpi_get_pxm from __ksymtab between '__ksymtab_acpi_get_pxm' (at offset 0x1050) and '__ksymtab_acpi_unlock_battery_dir'

One fix would be to statically link them again. Another fix would be to
wrap the exports in #ifdef CONFIG_ACPI_HOTPLUG_MEMORY_MODULE. Neither is
very attractive.

Coold you have a think about it please? The config is at
http://userweb.kernel.org/~akpm/config-akpm2.txt

Thanks.


2007-05-06 17:52:33

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [Patch] Fix section mismatch of memory hotplug related code.

On Fri, May 04, 2007 at 05:04:21PM -0700, Andrew Morton wrote:
> On Thu, 05 Apr 2007 17:01:02 +0900
> Yasunori Goto <[email protected]> wrote:
>
> > Hello.
> >
> > This is to fix many section mismatches of code related to memory hotplug.
> > I checked compile with memory hotplug on/off on ia64 and x86-64 box.
> >
> > ..
> >
> > ===================================================================
> > --- meminit.orig/drivers/acpi/numa.c 2007-04-04 20:15:58.000000000 +0900
> > +++ meminit/drivers/acpi/numa.c 2007-04-04 20:56:34.000000000 +0900
> > @@ -228,7 +228,7 @@ int __init acpi_numa_init(void)
> > return 0;
> > }
> >
> > -int acpi_get_pxm(acpi_handle h)
> > +int __meminit acpi_get_pxm(acpi_handle h)
> > {
> > unsigned long pxm;
> > acpi_status status;
> > @@ -246,7 +246,7 @@ int acpi_get_pxm(acpi_handle h)
> > }
> > EXPORT_SYMBOL(acpi_get_pxm);
> >
> > -int acpi_get_node(acpi_handle *handle)
> > +int __meminit acpi_get_node(acpi_handle *handle)
> > {
> > int pxm, node = -1;
>
> It doesn't make a lot of sense to export an __init symbol to modules. I
> guess it's OK in this case, but we get warnings:

It seems wrong to me to first tell linker to discard the code after init and
next to export the symbol to make it available for any module anytime.

Both function are relatively small so better avoid playing games and
drop the __meminit tag.

Sam

2007-05-07 13:17:31

by Yasunori Goto

[permalink] [raw]
Subject: Re: [Patch] Fix section mismatch of memory hotplug related code.

> On Fri, May 04, 2007 at 05:04:21PM -0700, Andrew Morton wrote:
> > On Thu, 05 Apr 2007 17:01:02 +0900
> > Yasunori Goto <[email protected]> wrote:
> >
> > > Hello.
> > >
> > > This is to fix many section mismatches of code related to memory hotplug.
> > > I checked compile with memory hotplug on/off on ia64 and x86-64 box.
> > >
> > > ..
> > >
> > > ===================================================================
> > > --- meminit.orig/drivers/acpi/numa.c 2007-04-04 20:15:58.000000000 +0900
> > > +++ meminit/drivers/acpi/numa.c 2007-04-04 20:56:34.000000000 +0900
> > > @@ -228,7 +228,7 @@ int __init acpi_numa_init(void)
> > > return 0;
> > > }
> > >
> > > -int acpi_get_pxm(acpi_handle h)
> > > +int __meminit acpi_get_pxm(acpi_handle h)
> > > {
> > > unsigned long pxm;
> > > acpi_status status;
> > > @@ -246,7 +246,7 @@ int acpi_get_pxm(acpi_handle h)
> > > }
> > > EXPORT_SYMBOL(acpi_get_pxm);
> > >
> > > -int acpi_get_node(acpi_handle *handle)
> > > +int __meminit acpi_get_node(acpi_handle *handle)
> > > {
> > > int pxm, node = -1;
> >
> > It doesn't make a lot of sense to export an __init symbol to modules. I
> > guess it's OK in this case, but we get warnings:

Oops, I might do unnecessary things. :(

>
> It seems wrong to me to first tell linker to discard the code after init and
> next to export the symbol to make it available for any module anytime.
>
> Both function are relatively small so better avoid playing games and
> drop the __meminit tag.

It sounds reasonable. I'll do it.

Thanks.


--
Yasunori Goto


2007-05-08 12:07:16

by Yasunori Goto

[permalink] [raw]
Subject: [Patch] Fix unnecesary meminit

> > It doesn't make a lot of sense to export an __init symbol to modules. I
> > guess it's OK in this case, but we get warnings:
>
> It seems wrong to me to first tell linker to discard the code after init and
> next to export the symbol to make it available for any module anytime.
>
> Both function are relatively small so better avoid playing games and
> drop the __meminit tag.

Ok. This is the patch.


-------
This is to fix unnecessary __meminit definition.
These are exported for kernel modules.

I compiled on ia64/x86-64 with memory hotplug on/off.

Signed-off-by: Yasunori Goto <[email protected]>

drivers/acpi/numa.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.21-mm1/drivers/acpi/numa.c
===================================================================
--- linux-2.6.21-mm1.orig/drivers/acpi/numa.c 2007-05-08 19:33:05.000000000 +0900
+++ linux-2.6.21-mm1/drivers/acpi/numa.c 2007-05-08 19:33:12.000000000 +0900
@@ -228,7 +228,7 @@ int __init acpi_numa_init(void)
return 0;
}

-int __meminit acpi_get_pxm(acpi_handle h)
+int acpi_get_pxm(acpi_handle h)
{
unsigned long pxm;
acpi_status status;
@@ -246,7 +246,7 @@ int __meminit acpi_get_pxm(acpi_handle h
}
EXPORT_SYMBOL(acpi_get_pxm);

-int __meminit acpi_get_node(acpi_handle *handle)
+int acpi_get_node(acpi_handle *handle)
{
int pxm, node = -1;


--
Yasunori Goto