| Message ID | 1241135719-9286-4-git-send-email-tabbott@mit.edu (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | kyle mcmartin |
| Headers | show |
Tim Abbott wrote: > .data.init_task should not need a separate output section; this change > moves it into the .data section. > > Signed-off-by: Tim Abbott <tabbott@mit.edu> > Cc: Kyle McMartin <kyle@mcmartin.ca> > Cc: Helge Deller <deller@gmx.de> > Cc: linux-parisc@vger.kernel.org I think this patch is wrong, although it is theoretically correct. IIRC, gcc on hppa is not able to provide an alignment >= 8k, which is why we have done the 16k alignment inside the linker script. So, I think this change will prevent the parisc kernel to boot up. Needs testing. Helge > --- > arch/parisc/kernel/init_task.c | 2 +- > arch/parisc/kernel/vmlinux.lds.S | 10 ++-------- > 2 files changed, 3 insertions(+), 9 deletions(-) > > diff --git a/arch/parisc/kernel/init_task.c b/arch/parisc/kernel/init_task.c > index 1e25a45..8ee17ea 100644 > --- a/arch/parisc/kernel/init_task.c > +++ b/arch/parisc/kernel/init_task.c > @@ -48,7 +48,7 @@ EXPORT_SYMBOL(init_mm); > * "init_task" linker map entry.. > */ > union thread_union init_thread_union > - __attribute__((aligned(128))) __attribute__((__section__(".data.init_task"))) = > + __attribute__((aligned(128))) __init_task_data = > { INIT_THREAD_INFO(init_task) }; > > #if PT_NLEVELS == 3 > diff --git a/arch/parisc/kernel/vmlinux.lds.S b/arch/parisc/kernel/vmlinux.lds.S > index b5936c9..c8a528d 100644 > --- a/arch/parisc/kernel/vmlinux.lds.S > +++ b/arch/parisc/kernel/vmlinux.lds.S > @@ -103,6 +103,8 @@ SECTIONS > . = ALIGN(L1_CACHE_BYTES); > /* Data */ > .data : { > + /* assembler code expects init_task to be 16k aligned */ > + INIT_TASK_DATA(16384) > NOSAVE_DATA > CACHELINE_ALIGNED_DATA(L1_CACHE_BYTES) > DATA_DATA > @@ -133,14 +135,6 @@ SECTIONS > } > __bss_stop = .; > > - > - /* assembler code expects init_task to be 16k aligned */ > - . = ALIGN(16384); > - /* init_task */ > - .data.init_task : { > - *(.data.init_task) > - } > - > #ifdef CONFIG_64BIT > . = ALIGN(16); > /* Linkage tables */ -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, May 02, 2009 at 07:13:37AM +0200, Helge Deller wrote: > I think this patch is wrong, although it is theoretically correct. > > IIRC, gcc on hppa is not able to provide an alignment >= 8k, which is > why we have done the 16k alignment inside the linker script. > So, I think this change will prevent the parisc kernel to boot up. > Needs testing. > I think you're confusing this with the 8-byte maximum alignment from kmalloc and on-stack that prevents us from just using a 16-byte aligned word as a lock on pa1.1? The patch I trimmed from this mail looks correct to me. regards, Kyle -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Kyle McMartin wrote: > On Sat, May 02, 2009 at 07:13:37AM +0200, Helge Deller wrote: >> I think this patch is wrong, although it is theoretically correct. >> >> IIRC, gcc on hppa is not able to provide an alignment >= 8k, which is >> why we have done the 16k alignment inside the linker script. >> So, I think this change will prevent the parisc kernel to boot up. >> Needs testing. >> > > I think you're confusing this with the 8-byte maximum alignment from > kmalloc and on-stack that prevents us from just using a 16-byte aligned > word as a lock on pa1.1? No, I was not confusing it with the 8byte-alignment. I really meant that a > 8k alignment was not possible. I tried exactly the same stuff once and failed. I think the restriction came from hpux compatibility or some old gas... Anyway, I just tried some assembly and it seems to work. > The patch I trimmed from this mail looks correct to me. If you apply it and it boots OK for you, I'm fine. Helge -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sam Ravnborg wrote: > On Sat, May 02, 2009 at 07:13:37AM +0200, Helge Deller wrote: >> Tim Abbott wrote: >>> .data.init_task should not need a separate output section; this change >>> moves it into the .data section. >>> >>> Signed-off-by: Tim Abbott <tabbott@mit.edu> >>> Cc: Kyle McMartin <kyle@mcmartin.ca> >>> Cc: Helge Deller <deller@gmx.de> >>> Cc: linux-parisc@vger.kernel.org >> >> I think this patch is wrong, although it is theoretically correct. >> >> IIRC, gcc on hppa is not able to provide an alignment >= 8k, which is >> why we have done the 16k alignment inside the linker script. >> So, I think this change will prevent the parisc kernel to boot up. >> Needs testing. > > The patch does not do much... > >> Helge >> >>> --- >>> arch/parisc/kernel/init_task.c | 2 +- >>> arch/parisc/kernel/vmlinux.lds.S | 10 ++-------- >>> 2 files changed, 3 insertions(+), 9 deletions(-) >>> >>> diff --git a/arch/parisc/kernel/init_task.c b/arch/parisc/kernel/init_task.c >>> index 1e25a45..8ee17ea 100644 >>> --- a/arch/parisc/kernel/init_task.c >>> +++ b/arch/parisc/kernel/init_task.c >>> @@ -48,7 +48,7 @@ EXPORT_SYMBOL(init_mm); >>> * "init_task" linker map entry.. >>> */ >>> union thread_union init_thread_union >>> - __attribute__((aligned(128))) __attribute__((__section__(".data.init_task"))) = >>> + __attribute__((aligned(128))) __init_task_data = >>> { INIT_THREAD_INFO(init_task) }; > This is a simple replacement with a nicer way to say "this belongs to > the .data.init_task section - no functional difference. > > >>> >>> #if PT_NLEVELS == 3 >>> diff --git a/arch/parisc/kernel/vmlinux.lds.S b/arch/parisc/kernel/vmlinux.lds.S >>> index b5936c9..c8a528d 100644 >>> --- a/arch/parisc/kernel/vmlinux.lds.S >>> +++ b/arch/parisc/kernel/vmlinux.lds.S >>> @@ -103,6 +103,8 @@ SECTIONS >>> . = ALIGN(L1_CACHE_BYTES); >>> /* Data */ >>> .data : { >>> + /* assembler code expects init_task to be 16k aligned */ >>> + INIT_TASK_DATA(16384) >>> NOSAVE_DATA >>> CACHELINE_ALIGNED_DATA(L1_CACHE_BYTES) >>> DATA_DATA >>> @@ -133,14 +135,6 @@ SECTIONS >>> } >>> __bss_stop = .; >>> >>> - >>> - /* assembler code expects init_task to be 16k aligned */ >>> - . = ALIGN(16384); >>> - /* init_task */ >>> - .data.init_task : { >>> - *(.data.init_task) >>> - } >>> - > This part moves away from a specific output section to including this in the > .data output section - with the _same_ alignmnet. > So the only issue here is that we move init_task before NOSAVE_DATA etc. > > I do not see why you think this changes alignmnet? Ugh. Of course you are correct. It doesn't change anything. Patch is OK for me. I missed the 16384 in INIT_TASK_DATA(16384). Thanks, Helge -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/parisc/kernel/init_task.c b/arch/parisc/kernel/init_task.c index 1e25a45..8ee17ea 100644 --- a/arch/parisc/kernel/init_task.c +++ b/arch/parisc/kernel/init_task.c @@ -48,7 +48,7 @@ EXPORT_SYMBOL(init_mm); * "init_task" linker map entry.. */ union thread_union init_thread_union - __attribute__((aligned(128))) __attribute__((__section__(".data.init_task"))) = + __attribute__((aligned(128))) __init_task_data = { INIT_THREAD_INFO(init_task) }; #if PT_NLEVELS == 3 diff --git a/arch/parisc/kernel/vmlinux.lds.S b/arch/parisc/kernel/vmlinux.lds.S index b5936c9..c8a528d 100644 --- a/arch/parisc/kernel/vmlinux.lds.S +++ b/arch/parisc/kernel/vmlinux.lds.S @@ -103,6 +103,8 @@ SECTIONS . = ALIGN(L1_CACHE_BYTES); /* Data */ .data : { + /* assembler code expects init_task to be 16k aligned */ + INIT_TASK_DATA(16384) NOSAVE_DATA CACHELINE_ALIGNED_DATA(L1_CACHE_BYTES) DATA_DATA @@ -133,14 +135,6 @@ SECTIONS } __bss_stop = .; - - /* assembler code expects init_task to be 16k aligned */ - . = ALIGN(16384); - /* init_task */ - .data.init_task : { - *(.data.init_task) - } - #ifdef CONFIG_64BIT . = ALIGN(16); /* Linkage tables */
.data.init_task should not need a separate output section; this change moves it into the .data section. Signed-off-by: Tim Abbott <tabbott@mit.edu> Cc: Kyle McMartin <kyle@mcmartin.ca> Cc: Helge Deller <deller@gmx.de> Cc: linux-parisc@vger.kernel.org --- arch/parisc/kernel/init_task.c | 2 +- arch/parisc/kernel/vmlinux.lds.S | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-)