The change:
+#ifdef CONFIG_HOTPLUG_CPU
+#define __INIT .section ".text","ax"
+#define __INITDATA .section ".data","aw"
+#else
#define __INIT .section ".init.text","ax"
-#define __FINIT .previous
#define __INITDATA .section ".init.data","aw"
+#endif
+#define __FINIT .previous
breaks ARM, since the ARM startup code moves from the start of the image
to somewhere in the middle of the kernel image. (That's trivially fixable.)
Besides that, this change is also wrong because not all assembly marked
__INIT needs to be moved into the standard .text segment when hotplug CPU
is enabled. Ditto for __INITDATA.
One is left wondering why the original patch proposers didn't:
1. Invent __CPUINIT and __CPUINITDATA, to be consistent with __init vs
__cpuinit.
2. copy other architecture maintainers.
Shall I invent __CPUINIT to mark data to always be thrown away, or are
the x86 folk going to withdraw this patch and do it properly?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
Russell King wrote:
> The change:
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +#define __INIT .section ".text","ax"
> +#define __INITDATA .section ".data","aw"
> +#else
> #define __INIT .section ".init.text","ax"
> -#define __FINIT .previous
> #define __INITDATA .section ".init.data","aw"
> +#endif
> +#define __FINIT .previous
>
>
Something is not right here.... was this the patch that I submitted? If
so then I definitely screwed up pretty badly.
I thought I had done (sorry for the cut-and-paste):
+#ifdef CONFIG_HOTPLUG_CPU
+#define __CPUINIT .section ".text","ax"
+#define __CPUINITDATA .section ".data","aw"
+#else
+#define __CPUINIT __INIT
+#define __CPUINITDATA __INITDATA
+#endif
Maybe some wires got crossed along the submit route... Russell, I'll
submit a fix against the latest tree.
P.
On Tue, 8 May 2007, Russell King wrote:
>
> Shall I invent __CPUINIT to mark data to always be thrown away, or are
> the x86 folk going to withdraw this patch and do it properly?
I can certainly revert the patch, but it would probably be better if you
were to just start adding the infrastructure you need and send it as a new
patch instead (where the revert is part of it, and the __CPUINIT is
additional).
Linus
Linus Torvalds wrote:
> On Tue, 8 May 2007, Russell King wrote:
>
>> Shall I invent __CPUINIT to mark data to always be thrown away, or are
>> the x86 folk going to withdraw this patch and do it properly?
>>
>
> I can certainly revert the patch, but it would probably be better if you
> were to just start adding the infrastructure you need and send it as a new
> patch instead (where the revert is part of it, and the __CPUINIT is
> additional).
>
>
I've sent Russell a new patch to test off-list. He or I will submit a
fix here after he tests.
P.
> Linus
>
On Tue, May 08, 2007 at 11:19:46AM -0400, Prarit Bhargava wrote:
> Linus Torvalds wrote:
> >On Tue, 8 May 2007, Russell King wrote:
> >
> >>Shall I invent __CPUINIT to mark data to always be thrown away, or are
> >>the x86 folk going to withdraw this patch and do it properly?
> >>
> >
> >I can certainly revert the patch, but it would probably be better if you
> >were to just start adding the infrastructure you need and send it as a new
> >patch instead (where the revert is part of it, and the __CPUINIT is
> >additional).
>
> I've sent Russell a new patch to test off-list. He or I will submit a
> fix here after he tests.
I'm not sure it'd make much sense for me to test it - I don't have x86
based test boxes here.
Assuming your intention is to revert this commit and replace it with your
patch which doesn't change the semantics of __INIT, it shouldn't cause me
any problems.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
> I'm not sure it'd make much sense for me to test it - I don't have x86
> based test boxes here.
>
> Assuming your intention is to revert this commit and replace it with your
> patch which doesn't change the semantics of __INIT, it shouldn't cause me
> any problems.
>
>
Okay -- I'll generate a new patch later this afternoon, test, and submit.
P.
On Tue, May 08, 2007 at 02:49:28PM +0100, Russell King wrote:
> The change:
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +#define __INIT .section ".text","ax"
> +#define __INITDATA .section ".data","aw"
> +#else
> #define __INIT .section ".init.text","ax"
> -#define __FINIT .previous
> #define __INITDATA .section ".init.data","aw"
> +#endif
> +#define __FINIT .previous
>
> breaks ARM, since the ARM startup code moves from the start of the image
> to somewhere in the middle of the kernel image. (That's trivially fixable.)
Although I've worked around this, I still would like this fixed.
Prarit's produced a patch (mail attached) which should fix it,
can we get it merged please?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: