When building with memory hotplug enabled and cpu hotplug disabled, we
end up with the following section mismatch:
WARNING: mm/built-in.o(.text+0x4e58): Section mismatch: reference to
.init.text: (between 'free_area_init_node' and '__build_all_zonelists')
This happens as a result of:
-> free_area_init_node()
-> free_area_init_core()
-> zone_pcp_init() <-- all __meminit up to this point
-> zone_batchsize() <-- marked as __cpuinit
This happens because CONFIG_HOTPLUG_CPU=n sets __cpuinit to __init, but
CONFIG_MEMORY_HOTPLUG=y unsets __meminit.
Changing zone_batchsize() to __init_refok fixes this.
Signed-off-by: Paul Mundt <[email protected]>
--
mm/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bd8e335..5c312d6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1968,7 +1968,7 @@ void zone_init_free_lists(struct pglist_data *pgdat, struct zone *zone,
memmap_init_zone((size), (nid), (zone), (start_pfn), MEMMAP_EARLY)
#endif
-static int __cpuinit zone_batchsize(struct zone *zone)
+static int __init_refok zone_batchsize(struct zone *zone)
{
int batch;
On Mon, 11 Jun 2007 13:35:43 +0900
Paul Mundt <[email protected]> wrote:
> When building with memory hotplug enabled and cpu hotplug disabled, we
> end up with the following section mismatch:
>
> WARNING: mm/built-in.o(.text+0x4e58): Section mismatch: reference to
> .init.text: (between 'free_area_init_node' and '__build_all_zonelists')
>
> This happens as a result of:
>
> -> free_area_init_node()
> -> free_area_init_core()
> -> zone_pcp_init() <-- all __meminit up to this point
> -> zone_batchsize() <-- marked as __cpuinit
>
> This happens because CONFIG_HOTPLUG_CPU=n sets __cpuinit to __init, but
> CONFIG_MEMORY_HOTPLUG=y unsets __meminit.
>
> Changing zone_batchsize() to __init_refok fixes this.
>
It seems this zone_batchsize() is called by cpu-hotplug and memory-hotplug.
So, __init_refok doesn't look good, here.
maybe we can use __devinit here. (Because HOTPLUG_CPU and MEMORY_HOTPLUG are
depend on CONFIG_HOTPLUG.)
-Kame
On Mon, Jun 11, 2007 at 02:01:45PM +0900, KAMEZAWA Hiroyuki wrote:
> On Mon, 11 Jun 2007 13:35:43 +0900
> Paul Mundt <[email protected]> wrote:
>
> > When building with memory hotplug enabled and cpu hotplug disabled, we
> > end up with the following section mismatch:
> >
> > WARNING: mm/built-in.o(.text+0x4e58): Section mismatch: reference to
> > .init.text: (between 'free_area_init_node' and '__build_all_zonelists')
> >
> > This happens as a result of:
> >
> > -> free_area_init_node()
> > -> free_area_init_core()
> > -> zone_pcp_init() <-- all __meminit up to this point
> > -> zone_batchsize() <-- marked as __cpuinit
> >
> > This happens because CONFIG_HOTPLUG_CPU=n sets __cpuinit to __init, but
> > CONFIG_MEMORY_HOTPLUG=y unsets __meminit.
> >
> > Changing zone_batchsize() to __init_refok fixes this.
> >
>
> It seems this zone_batchsize() is called by cpu-hotplug and memory-hotplug.
> So, __init_refok doesn't look good, here.
>
> maybe we can use __devinit here. (Because HOTPLUG_CPU and MEMORY_HOTPLUG are
> depend on CONFIG_HOTPLUG.)
>
Yes, that's probably a more reasonable way to go. The __devinit name is a
bit misleading, though..
--
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bd8e335..05ace44 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1968,7 +1968,7 @@ void zone_init_free_lists(struct pglist_data *pgdat, struct zone *zone,
memmap_init_zone((size), (nid), (zone), (start_pfn), MEMMAP_EARLY)
#endif
-static int __cpuinit zone_batchsize(struct zone *zone)
+static int __devinit zone_batchsize(struct zone *zone)
{
int batch;
On Mon, 11 Jun 2007 14:09:55 +0900 Paul Mundt wrote:
> On Mon, Jun 11, 2007 at 02:01:45PM +0900, KAMEZAWA Hiroyuki wrote:
> > On Mon, 11 Jun 2007 13:35:43 +0900
> > Paul Mundt <[email protected]> wrote:
> >
> > > When building with memory hotplug enabled and cpu hotplug disabled, we
> > > end up with the following section mismatch:
> > >
> > > WARNING: mm/built-in.o(.text+0x4e58): Section mismatch: reference to
> > > .init.text: (between 'free_area_init_node' and '__build_all_zonelists')
> > >
> > > This happens as a result of:
> > >
> > > -> free_area_init_node()
> > > -> free_area_init_core()
> > > -> zone_pcp_init() <-- all __meminit up to this point
> > > -> zone_batchsize() <-- marked as __cpuinit
> > >
> > > This happens because CONFIG_HOTPLUG_CPU=n sets __cpuinit to __init, but
> > > CONFIG_MEMORY_HOTPLUG=y unsets __meminit.
> > >
> > > Changing zone_batchsize() to __init_refok fixes this.
> > >
> >
> > It seems this zone_batchsize() is called by cpu-hotplug and memory-hotplug.
> > So, __init_refok doesn't look good, here.
> >
> > maybe we can use __devinit here. (Because HOTPLUG_CPU and MEMORY_HOTPLUG are
> > depend on CONFIG_HOTPLUG.)
> >
> Yes, that's probably a more reasonable way to go. The __devinit name is a
> bit misleading, though..
__meminit does not fit/work here?
> --
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index bd8e335..05ace44 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1968,7 +1968,7 @@ void zone_init_free_lists(struct pglist_data *pgdat, struct zone *zone,
> memmap_init_zone((size), (nid), (zone), (start_pfn), MEMMAP_EARLY)
> #endif
>
> -static int __cpuinit zone_batchsize(struct zone *zone)
> +static int __devinit zone_batchsize(struct zone *zone)
> {
> int batch;
>
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
On Mon, Jun 11, 2007 at 08:27:32AM -0700, Randy Dunlap wrote:
> On Mon, 11 Jun 2007 14:09:55 +0900 Paul Mundt wrote:
> > On Mon, Jun 11, 2007 at 02:01:45PM +0900, KAMEZAWA Hiroyuki wrote:
> > > On Mon, 11 Jun 2007 13:35:43 +0900
> > > Paul Mundt <[email protected]> wrote:
> > > > This happens because CONFIG_HOTPLUG_CPU=n sets __cpuinit to __init, but
> > > > CONFIG_MEMORY_HOTPLUG=y unsets __meminit.
> > >
> > > It seems this zone_batchsize() is called by cpu-hotplug and
> > > memory-hotplug. So, __init_refok doesn't look good, here.
> > >
> > > maybe we can use __devinit here. (Because HOTPLUG_CPU and
> > > MEMORY_HOTPLUG are depend on CONFIG_HOTPLUG.)
> > >
> > Yes, that's probably a more reasonable way to go. The __devinit name is a
> > bit misleading, though..
>
> __meminit does not fit/work here?
>
No, for the reasons already noted.
If CONFIG_MEMORY_HOTPLUG=n __meminit == __init, and if
CONFIG_HOTPLUG_CPU=n __cpuinit == __init. However, with one set and the
other disabled, you end up with a reference between __init and a regular
non-init function.
CONFIG_HOTPLUG is the only thing that they both have in common, so only
__devinit will gaurantee the proper behaviour. __init_refok is the
opposite of the behaviour that is desired, as Kamezawa-san was quick to
point out.
Simply switching to __meminit will cause zone_batchlist() to emit a
section mismatch on CONFIG_HOTPLUG_CPU=y and CONFIG_MEMORY_HOTPLUG=n
configurations instead of the other way around.
>
> If CONFIG_MEMORY_HOTPLUG=n __meminit == __init, and if
> CONFIG_HOTPLUG_CPU=n __cpuinit == __init. However, with one set and the
> other disabled, you end up with a reference between __init and a regular
> non-init function.
My plan is to define dedicated sections for both __devinit and __meminit.
Then we can apply the checks no matter the definition of CONFIG_HOTPLUG*
But we are a few steps away form doing so:
1) All harcoded uses of .init.text needs to go (at least done in assembler files)
2) The arch lds files needs to be unified a bit too.
Then we can during the final link stage decide if __devinit shall be merged
into .text or .init.text (after applying the modpost checks).
But do not hold your breath.
The even more important precondition is to sort out all the current
section mismatch warnings. But here we are getting close.
Sam
> >
> > If CONFIG_MEMORY_HOTPLUG=n __meminit == __init, and if
> > CONFIG_HOTPLUG_CPU=n __cpuinit == __init. However, with one set and the
> > other disabled, you end up with a reference between __init and a regular
> > non-init function.
>
> My plan is to define dedicated sections for both __devinit and __meminit.
> Then we can apply the checks no matter the definition of CONFIG_HOTPLUG*
I prefer defining "__nodeinit" for __cpuinit and __meminit case to
__devinit. __devinit is used many devices like I/O, and it is
useful for many desktop users. But, cpu/memory hotpluggable box
is very rare. And it should be in init section for many people.
This kind of issue is caused by initialization of pgdat/zone.
I think __nodeinit is enough and desirable.
Bye.
--
Yasunori Goto
On Tue, Jun 12, 2007 at 10:50:33AM +0900, Yasunori Goto wrote:
> > >
> > > If CONFIG_MEMORY_HOTPLUG=n __meminit == __init, and if
> > > CONFIG_HOTPLUG_CPU=n __cpuinit == __init. However, with one set and the
> > > other disabled, you end up with a reference between __init and a regular
> > > non-init function.
> >
> > My plan is to define dedicated sections for both __devinit and __meminit.
> > Then we can apply the checks no matter the definition of CONFIG_HOTPLUG*
>
> I prefer defining "__nodeinit" for __cpuinit and __meminit case to
> __devinit. __devinit is used many devices like I/O, and it is
> useful for many desktop users. But, cpu/memory hotpluggable box
> is very rare. And it should be in init section for many people.
>
> This kind of issue is caused by initialization of pgdat/zone.
> I think __nodeinit is enough and desirable.
>
A #define __nodeinit __devinit is probably reasonable for clarity
purposes. But whatever we want to call it, the current __cpuinit for
zone_batchsize() has to be changed, as it will be freed with the rest of
the init code if CPU hotplug is disabled. If we want to do something
cleaner in the long run, that's fine, but changing to __devinit now at
least gets the semantics right for both the memory and cpu hotplug cases.