2006-01-21 00:41:49

by Ravikiran G Thirumalai

[permalink] [raw]
Subject: [bug] __meminit breaks cpu hotplug

Recent __meminit additions broke cpu hotplug when the kernel is configured
with HOTPLUG_CPU but not HOTPLUG_MEMORY.
Although __meminit replaced __devinit functions many places, all those functions
looked like they should have been marked with __cpuinit to begin with. That
is the reason I have changed the below to __cpuinit. I leave it to Matt if he
wants to use __devinit here instead. (Depending on where he sees __meminit
would be used.)

Signed-off-by: Ravikiran Thirumalai <[email protected]>
Signed-off-by: Shai Fultheim <[email protected]>

Index: linux-2.6.16-rc1/include/linux/init.h
===================================================================
--- linux-2.6.16-rc1.orig/include/linux/init.h 2006-01-17 14:12:16.000000000 -0800
+++ linux-2.6.16-rc1/include/linux/init.h 2006-01-20 12:24:44.000000000 -0800
@@ -247,10 +247,10 @@ void __init parse_early_param(void);
#define __memexit
#define __memexitdata
#else
-#define __meminit __init
-#define __meminitdata __initdata
-#define __memexit __exit
-#define __memexitdata __exitdata
+#define __meminit __cpuinit
+#define __meminitdata __cpuinitdata
+#define __memexit __cpuexit
+#define __memexitdata __cpuexitdata
#endif

/* Functions marked as __devexit may be discarded at kernel link time, depending


2006-01-21 00:54:39

by Andrew Morton

[permalink] [raw]
Subject: Re: [bug] __meminit breaks cpu hotplug

Ravikiran G Thirumalai <[email protected]> wrote:
>
> Recent __meminit additions broke cpu hotplug when the kernel is configured
> with HOTPLUG_CPU but not HOTPLUG_MEMORY.
> Although __meminit replaced __devinit functions many places, all those functions
> looked like they should have been marked with __cpuinit to begin with. That
> is the reason I have changed the below to __cpuinit. I leave it to Matt if he
> wants to use __devinit here instead. (Depending on where he sees __meminit
> would be used.)
>
> Signed-off-by: Ravikiran Thirumalai <[email protected]>
> Signed-off-by: Shai Fultheim <[email protected]>
>
> Index: linux-2.6.16-rc1/include/linux/init.h
> ===================================================================
> --- linux-2.6.16-rc1.orig/include/linux/init.h 2006-01-17 14:12:16.000000000 -0800
> +++ linux-2.6.16-rc1/include/linux/init.h 2006-01-20 12:24:44.000000000 -0800
> @@ -247,10 +247,10 @@ void __init parse_early_param(void);
> #define __memexit
> #define __memexitdata
> #else
> -#define __meminit __init
> -#define __meminitdata __initdata
> -#define __memexit __exit
> -#define __memexitdata __exitdata
> +#define __meminit __cpuinit
> +#define __meminitdata __cpuinitdata
> +#define __memexit __cpuexit
> +#define __memexitdata __cpuexitdata

This looks wrong. The __meminit and __cpuinit definitions we have now are
OK, aren't they? Surely the problem is that some functions/variables are
incorrectly tagged?

If some function is needed by both HOTPLUG_CPU and HOTPLUG_MEMORY then
we're rather messed up - I guess it'll need to be put inside #if
defined(..) || defined(..) and put into plain old .text.

2006-01-21 01:28:28

by Ravikiran G Thirumalai

[permalink] [raw]
Subject: Re: [bug] __meminit breaks cpu hotplug

On Fri, Jan 20, 2006 at 04:55:21PM -0800, Andrew Morton wrote:
> Ravikiran G Thirumalai <[email protected]> wrote:
> > -#define __meminitdata __initdata
> > -#define __memexit __exit
> > -#define __memexitdata __exitdata
> > +#define __meminit __cpuinit
> > +#define __meminitdata __cpuinitdata
> > +#define __memexit __cpuexit
> > +#define __memexitdata __cpuexitdata
>
> This looks wrong. The __meminit and __cpuinit definitions we have now are
> OK, aren't they? Surely the problem is that some functions/variables are
> incorrectly tagged?

I hit the bug on pageset_cpuup_callback, which is obviously __cpuinit, but
has been marked __meminit. Yeah .. bad patch duh!

For some reason I thought all other functions marked with __meminit looked
like __cpuinit candidates....while just pageset_cpuup_callback should be
changed to __cpuinit


Index: linux-2.6.16-rc1/mm/page_alloc.c
===================================================================
--- linux-2.6.16-rc1.orig/mm/page_alloc.c 2006-01-17 14:12:17.000000000 -0800
+++ linux-2.6.16-rc1/mm/page_alloc.c 2006-01-20 17:21:03.000000000 -0800
@@ -1923,7 +1923,7 @@ static inline void free_zone_pagesets(in
}
}

-static int __meminit pageset_cpuup_callback(struct notifier_block *nfb,
+static int __cpuinit pageset_cpuup_callback(struct notifier_block *nfb,
unsigned long action,
void *hcpu)
{

2006-01-21 01:49:16

by Ashok Raj

[permalink] [raw]
Subject: Re: [bug] __meminit breaks cpu hotplug

On Fri, Jan 20, 2006 at 05:27:09PM -0800, Ravikiran G Thirumalai wrote:
>
>
> I hit the bug on pageset_cpuup_callback, which is obviously __cpuinit,
> but
> has been marked __meminit. Yeah .. bad patch duh!
>
> For some reason I thought all other functions marked with __meminit
> looked
> like __cpuinit candidates....while just pageset_cpuup_callback should
> be
> changed to __cpuinit
>

Sorry i missed this while posting the other patch. There are other functions
called by process_zones() from withing pageset_cpuup_callback() that
are also marked __meminit. I sent a patch just now to cover that.

but as i mentioned i still have trouble getting cpuup to work right.
cpu_down() seems to work ok so far.

--
Cheers,
Ashok Raj
- Open Source Technology Center