[PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Tim Abbott]

alpha is the only architecture that uses the section name
.data.init_thread instead of .data.init_task.  So convert alpha to use
.data.init_task like everything else.

.data.init_task does not need a separate output section; this change
also moves it into the .data output section.

Signed-off-by: Tim Abbott <tabbott@ksplice.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/init_task.c   |    5 ++---
 arch/alpha/kernel/vmlinux.lds.S |    7 ++-----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/alpha/kernel/init_task.c b/arch/alpha/kernel/init_task.c
index 19b8632..6f80ca4 100644
--- a/arch/alpha/kernel/init_task.c
+++ b/arch/alpha/kernel/init_task.c
@@ -13,6 +13,5 @@ static struct sighand_struct init_sighand = INIT_SIGHAND(init_sighand);
 struct task_struct init_task = INIT_TASK(init_task);
 EXPORT_SYMBOL(init_task);
 
-union thread_union init_thread_union
-	__attribute__((section(".data.init_thread")))
-	= { INIT_THREAD_INFO(init_task) };
+union thread_union init_thread_union __init_task_data =
+	{ INIT_THREAD_INFO(init_task) };
diff --git a/arch/alpha/kernel/vmlinux.lds.S b/arch/alpha/kernel/vmlinux.lds.S
index b9d6568..a4bb75e 100644
--- a/arch/alpha/kernel/vmlinux.lds.S
+++ b/arch/alpha/kernel/vmlinux.lds.S
@@ -1,5 +1,6 @@
 #include <asm-generic/vmlinux.lds.h>
 #include <asm/page.h>
+#include <asm/thread_info.h>
 
 OUTPUT_FORMAT("elf64-alpha")
 OUTPUT_ARCH(alpha)
@@ -92,11 +93,6 @@ SECTIONS
 	__init_end = .;
 	/* Freed after init ends here */
 
-	/* Note 2 page alignment above.  */
-	.data.init_thread : {
-		*(.data.init_thread)
-	}
-
 	. = ALIGN(PAGE_SIZE);
 	.data.page_aligned : {
 		*(.data.page_aligned)
@@ -110,6 +106,7 @@ SECTIONS
 	_data = .;
 	/* Data */
 	.data : {
+		INIT_TASK_DATA(THREAD_SIZE)
 		DATA_DATA
 		CONSTRUCTORS
 	}
-- 
1.6.3.3

Re: [PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Richard Henderson]

On 07/31/2009 01:56 PM, Tim Abbott wrote:
> -	/* Note 2 page alignment above.  */
> -	.data.init_thread : {
> -		*(.data.init_thread)
> -	}

NACK.

You can change the section name, sure, but you cannot remove the 2 page 
alignment that we had via the alignment at the end of the init sections. 
  You'll break current_thread_info which is always computed as 
(kernel-stack-pointer & -(2*PAGE_SIZE)).

Similarly it is *not* a bug that the page_aligned sections were before 
data, because we already knew we had 2 page alignment from the end of 
init + 2 pages of init_thread.


r~

Re: [PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Richard Henderson]

On 07/31/2009 02:23 PM, Richard Henderson wrote:
> Similarly it is *not* a bug that the page_aligned sections were before
> data, because we already knew we had 2 page alignment from the end of
> init + 2 pages of init_thread.

Indeed, I'll go further and say that the common definition of 
RW_DATA_SECTION is buggy.

> #define RW_DATA_SECTION(cacheline, nosave, pagealigned, inittask)       \
>         . = ALIGN(PAGE_SIZE);                                           \
>         .data : AT(ADDR(.data) - LOAD_OFFSET) {                         \
>                 INIT_TASK(inittask)                                     \
>                 CACHELINE_ALIGNED_DATA(cacheline)                       \
>                 READ_MOSTLY_DATA(cacheline)                             \
>                 DATA_DATA                                               \
>                 CONSTRUCTORS                                            \
>                 NOSAVE_DATA(nosave)                                     \
>                 PAGE_ALIGNED_DATA(pagealigned)                          \
>         }

Given that we align the entire .data section why have interior padding 
to re-align for page-aligned data?  Surely a better ordering would be

	. = ALIGN(PAGE_SIZE);
	.data : AT(ADDR(.data) - LOAD_OFFSET) {
		INIT_TASK(inittask)
		NOSAVE_DATA
		PAGE_ALIGNED_DATA(pagealigned)
		CACHELINE_ALIGNED_DATA(cacheline)
		READ_MOSTLY_DATA(cacheline)
		DATA_DATA
		CONSTRUCTORS
	}

with that change, and without

> -	. = ALIGN(2 * PAGE_SIZE);
> +	. = ALIGN(PAGE_SIZE);
>  	__init_end = .;

the patch looks like it'd be ok.  All you're doing with this fragment is 
failing to free a page of padding.  You could change it to

	. = ALIGN(THREAD_SIZE)

if it makes you feel better.


r~

Re: [PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Tim Abbott]

On Fri, 31 Jul 2009, Richard Henderson wrote:

> On 07/31/2009 02:23 PM, Richard Henderson wrote:
> > Similarly it is *not* a bug that the page_aligned sections were before
> > data, because we already knew we had 2 page alignment from the end of
> > init + 2 pages of init_thread.
> 
> Indeed, I'll go further and say that the common definition of RW_DATA_SECTION
> is buggy.
[...]
> Given that we align the entire .data section why have interior padding to
> re-align for page-aligned data?  Surely a better ordering would be
[...]

Agreed.  I actually commented on this issue in one of the drafts of 
RW_DATA_SECTION, but forgot to check again whether Sam had addressed it 
when reviewing the final version.  Fortunately we're talking about a small 
inefficiency, not something more serious.  Below is your proposed change 
in patch form; I hope Sam will take a look at it.

	-Tim Abbott

Optimize the ordering of sections in RW_DATA_SECTION.

The current RW_DATA_SECTION macro doesn't place the various 
PAGE_SIZE-aligned sections next to each other.  This could result in up to 
a page of memory being wasted realigning to PAGE_SIZE twice.

Signed-off-by: Tim Abbott <tabbott@ksplice.com>
---
 include/asm-generic/vmlinux.lds.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 6ad76bf..146815d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -706,12 +706,12 @@
 	. = ALIGN(PAGE_SIZE);						\
 	.data : AT(ADDR(.data) - LOAD_OFFSET) {				\
 		INIT_TASK_DATA(inittask)				\
+		NOSAVE_DATA						\
+		PAGE_ALIGNED_DATA(pagealigned)				\
 		CACHELINE_ALIGNED_DATA(cacheline)			\
 		READ_MOSTLY_DATA(cacheline)				\
 		DATA_DATA						\
 		CONSTRUCTORS						\
-		NOSAVE_DATA						\
-		PAGE_ALIGNED_DATA(pagealigned)				\
 	}
 
 #define INIT_TEXT_SECTION(inittext_align)				\
-- 
1.6.3.3

Re: [PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Tim Abbott]

On Fri, 31 Jul 2009, Richard Henderson wrote:

> On 07/31/2009 01:56 PM, Tim Abbott wrote:
> > -	/* Note 2 page alignment above.  */
> > -	.data.init_thread : {
> > -		*(.data.init_thread)
> > -	}
> 
> NACK.
> 
> You can change the section name, sure, but you cannot remove the 2 page
> alignment that we had via the alignment at the end of the init sections.
> You'll break current_thread_info which is always computed as
> (kernel-stack-pointer & -(2*PAGE_SIZE)).

The INIT_TASK_DATA(THREAD_SIZE) macro call aligns to THREAD_SIZE (= 
2*PAGE_SIZE).  So I'm not removing the 2 page alignment; I'm just moving 
it along with the code that needs to be aligned.

This change:

-	. = ALIGN(2 * PAGE_SIZE);
+	. = ALIGN(PAGE_SIZE);
 	__init_end = .;

removes the now-unnecessary (2 * PAGE_SIZE) alignment for __init_end 
caused by moving .data.init_task (it should have been in the first patch).

	-Tim Abbott

Re: [PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Richard Henderson]

On 07/31/2009 03:02 PM, Tim Abbott wrote:
> The INIT_TASK_DATA(THREAD_SIZE) macro call aligns to THREAD_SIZE (=
> 2*PAGE_SIZE).  So I'm not removing the 2 page alignment; I'm just moving
> it along with the code that needs to be aligned.

Sure.

>
> This change:
>
> -	. = ALIGN(2 * PAGE_SIZE);
> +	. = ALIGN(PAGE_SIZE);
>   	__init_end = .;
>
> removes the now-unnecessary (2 * PAGE_SIZE) alignment for __init_end
> caused by moving .data.init_task (it should have been in the first patch).

While it's technically unnecessary, it's also very much desired.

Think about it.  Suppose we have 9 pages of init, followed by the
two-page-aligned INIT_TASK_DATA.  So we get a page worth of padding
added.  It's better to have the two-page-alignment within the init
section so as to get 10 pages of init followed by no padding.  In
this way the page of padding gets freed with the rest of init.

Are you following me at all here?


r~

Re: [PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Tim Abbott]

On Fri, 31 Jul 2009, Richard Henderson wrote:

> On 07/31/2009 03:02 PM, Tim Abbott wrote:
> > 
> > This change:
> > 
> > -	. = ALIGN(2 * PAGE_SIZE);
> > +	. = ALIGN(PAGE_SIZE);
> >   	__init_end = .;
> > 
> > removes the now-unnecessary (2 * PAGE_SIZE) alignment for __init_end
> > caused by moving .data.init_task (it should have been in the first patch).
> 
> While it's technically unnecessary, it's also very much desired.
> 
> Think about it.  Suppose we have 9 pages of init, followed by the
> two-page-aligned INIT_TASK_DATA.  So we get a page worth of padding
> added.  It's better to have the two-page-alignment within the init
> section so as to get 10 pages of init followed by no padding.  In
> this way the page of padding gets freed with the rest of init.
> 
> Are you following me at all here?

Yeah, okay, that makes total sense (nice optimization).  New version of 
the patch below, with the alignment changed to THREAD_SIZE (and I added a 
comment explaining why).

	-Tim Abbott

From: Geoffrey Thomas <geofft@ksplice.com>

alpha: Clean up linker script using new linker script macros.

Note that .data.page_aligned and .data.cacheline_aligned are now after
_data; it was probably a bug that they were before it.

Also, some explicit ALIGN(8)'s between various initcall sections were
removed; this should be harmless as the implicit alignment of
initcall_t was already 8.

Signed-off-by: Geoffrey Thomas <geofft@ksplice.com>
Signed-off-by: Tim Abbott <tabbott@ksplice.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
 arch/alpha/kernel/vmlinux.lds.S |   90 ++++-----------------------------------
 1 files changed, 9 insertions(+), 81 deletions(-)

diff --git a/arch/alpha/kernel/vmlinux.lds.S b/arch/alpha/kernel/vmlinux.lds.S
index a4bb75e..a4f9001 100644
--- a/arch/alpha/kernel/vmlinux.lds.S
+++ b/arch/alpha/kernel/vmlinux.lds.S
@@ -32,84 +32,21 @@ SECTIONS
 	} :kernel
 
 	RODATA
-
-	/* Exception table */
-	. = ALIGN(16);
-	__ex_table : {
-		__start___ex_table = .;
-		*(__ex_table)
-		__stop___ex_table = .;
-	}
+	EXCEPTION_TABLE(16)
 
 	/* Will be freed after init */
-	. = ALIGN(PAGE_SIZE);
-	/* Init code and data */
-	__init_begin = .;
-	.init.text : {
-		_sinittext = .;
-		INIT_TEXT
-		_einittext = .;
-	}
-	.init.data : {
-		INIT_DATA
-	}
-
-	. = ALIGN(16);
-	.init.setup : {
-		__setup_start = .;
-		*(.init.setup)
-		__setup_end = .;
-	}
-
-	. = ALIGN(8);
-	.initcall.init : {
-		__initcall_start = .;
-		INITCALLS
-		__initcall_end = .;
-	}
-
-#ifdef CONFIG_BLK_DEV_INITRD
-	. = ALIGN(PAGE_SIZE);
-	.init.ramfs : {
-		__initramfs_start = .;
-		*(.init.ramfs)
-		__initramfs_end = .;
-	}
-#endif
-
-	. = ALIGN(8);
-	.con_initcall.init : {
-		__con_initcall_start = .;
-		*(.con_initcall.init)
-		__con_initcall_end = .;
-	}
-
-	. = ALIGN(8);
-	SECURITY_INIT
-
+	__init_begin = ALIGN(PAGE_SIZE);
+	INIT_TEXT_SECTION(PAGE_SIZE)
+	INIT_DATA_SECTION(16)
 	PERCPU(PAGE_SIZE)
-
-	. = ALIGN(2 * PAGE_SIZE);
+	/* Align to THREAD_SIZE rather than PAGE_SIZE here so any padding page
+	   needed for the THREAD_SIZE aligned init_task gets freed after init */
+	. = ALIGN(THREAD_SIZE);
 	__init_end = .;
 	/* Freed after init ends here */
 
-	. = ALIGN(PAGE_SIZE);
-	.data.page_aligned : {
-		*(.data.page_aligned)
-	}
-
-	. = ALIGN(64);
-	.data.cacheline_aligned : {
-		*(.data.cacheline_aligned)
-	}
-
 	_data = .;
-	/* Data */
-	.data : {
-		INIT_TASK_DATA(THREAD_SIZE)
-		DATA_DATA
-		CONSTRUCTORS
-	}
+	RW_DATA_SECTION(64, PAGE_SIZE, THREAD_SIZE)
 
 	.got : {
 		*(.got)
@@ -119,16 +56,7 @@ SECTIONS
 	}
 	_edata = .;	/* End of data section */
 
-	__bss_start = .;
-	.sbss : {
-		*(.sbss)
-		*(.scommon)
-	}
-	.bss : {
-		*(.bss)
-		*(COMMON)
-	}
-	__bss_stop = .;
+	BSS_SECTION(0, 0, 0)
 	_end = .;
 
 	/* Sections to be discarded */
-- 
1.6.3.3

Re: [PATCH 1/2] alpha: use .data.init_task instead of .data.init_thread.

[Richard Henderson]

On 07/31/2009 04:56 PM, Tim Abbott wrote:
> Also, some explicit ALIGN(8)'s between various initcall sections were
> removed; this should be harmless as the implicit alignment of
> initcall_t was already 8.
>
> Signed-off-by: Geoffrey Thomas<geofft@ksplice.com>
> Signed-off-by: Tim Abbott<tabbott@ksplice.com>
> Cc: Richard Henderson<rth@twiddle.net>
> Cc: Ivan Kokshaysky<ink@jurassic.park.msu.ru>
> Cc: Sam Ravnborg<sam@ravnborg.org>

Signed-off-by: Richard Henderson <rth@twiddle.net>


r~