[patch 0/2] x86: vmlinux.lds cleanup

[patch 0/2] x86: vmlinux.lds cleanup

[Cyrill Gorcunov]

This patch series replaces numeric constants in vmlinux.lds scripts
to appropriate symbolic names.

Any comments are highly appreciated.

-- 

[patch 1/2] x86: lds - Use PAGE_SIZE instead of numeric constant

[Cyrill Gorcunov]

[-- Attachment #1: ps-x86 --]
[-- Type: text/plain, Size: 5607 bytes --]

It's much better to use PAGE_SIZE then magic 4096
(though it's almost synonym in most cases on x86 but
not for *all* cases ;)

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Index: linux-2.6.git/arch/x86/kernel/vmlinux_64.lds.S
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/vmlinux_64.lds.S	2008-02-09 22:27:13.000000000 +0300
+++ linux-2.6.git/arch/x86/kernel/vmlinux_64.lds.S	2008-02-17 18:08:57.000000000 +0300
@@ -37,7 +37,7 @@ SECTIONS
 	KPROBES_TEXT
 	*(.fixup)
 	*(.gnu.warning)
-	_etext = .;			/* End of text section */
+	_etext = .;		/* End of text section */
   } :text = 0x9090
 
   . = ALIGN(16);		/* Exception table */
@@ -60,7 +60,7 @@ SECTIONS
   	__tracedata_end = .;
   }
 
-  . = ALIGN(PAGE_SIZE);        /* Align data segment to page size boundary */
+  . = ALIGN(PAGE_SIZE);		/* Align data segment to page size boundary */
 				/* Data */
   .data : AT(ADDR(.data) - LOAD_OFFSET) {
 	DATA_DATA
@@ -119,7 +119,7 @@ SECTIONS
   .vsyscall_3 ADDR(.vsyscall_0) + 3072: AT(VLOAD(.vsyscall_3))
 		{ *(.vsyscall_3) }
 
-  . = VSYSCALL_VIRT_ADDR + 4096;
+  . = VSYSCALL_VIRT_ADDR + PAGE_SIZE;
 
 #undef VSYSCALL_ADDR
 #undef VSYSCALL_PHYS_ADDR
@@ -134,23 +134,23 @@ SECTIONS
 	*(.data.init_task)
   }:data.init
 
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   .data.page_aligned : AT(ADDR(.data.page_aligned) - LOAD_OFFSET) {
 	*(.data.page_aligned)
   }
 
   /* might get freed after init */
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   __smp_alt_begin = .;
   __smp_locks = .;
   .smp_locks : AT(ADDR(.smp_locks) - LOAD_OFFSET) {
 	*(.smp_locks)
   }
   __smp_locks_end = .;
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   __smp_alt_end = .;
 
-  . = ALIGN(4096);		/* Init code and data */
+  . = ALIGN(PAGE_SIZE);		/* Init code and data */
   __init_begin = .;
   .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) {
 	_sinittext = .;
@@ -191,7 +191,7 @@ SECTIONS
   .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) {
 	*(.altinstructions)
   }
-  __alt_instructions_end = .; 
+  __alt_instructions_end = .;
   .altinstr_replacement : AT(ADDR(.altinstr_replacement) - LOAD_OFFSET) {
 	*(.altinstr_replacement)
   }
@@ -207,25 +207,25 @@ SECTIONS
 /* vdso blob that is mapped into user space */
   vdso_start = . ;
   .vdso  : AT(ADDR(.vdso) - LOAD_OFFSET) { *(.vdso) }
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   vdso_end = .;
 
 #ifdef CONFIG_BLK_DEV_INITRD
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   __initramfs_start = .;
   .init.ramfs : AT(ADDR(.init.ramfs) - LOAD_OFFSET) { *(.init.ramfs) }
   __initramfs_end = .;
 #endif
 
-  PERCPU(4096)
+  PERCPU(PAGE_SIZE)
 
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   __init_end = .;
 
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   __nosave_begin = .;
   .data_nosave : AT(ADDR(.data_nosave) - LOAD_OFFSET) { *(.data.nosave) }
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   __nosave_end = .;
 
   __bss_start = .;		/* BSS */
Index: linux-2.6.git/arch/x86/kernel/vmlinux_32.lds.S
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/vmlinux_32.lds.S	2008-02-09 22:27:13.000000000 +0300
+++ linux-2.6.git/arch/x86/kernel/vmlinux_32.lds.S	2008-02-16 16:34:36.000000000 +0300
@@ -38,7 +38,7 @@ SECTIONS
 
   /* read-only */
   .text : AT(ADDR(.text) - LOAD_OFFSET) {
-	. = ALIGN(4096); /* not really needed, already page aligned */
+	. = ALIGN(PAGE_SIZE); /* not really needed, already page aligned */
 	*(.text.page_aligned)
 	TEXT_TEXT
 	SCHED_TEXT
@@ -70,21 +70,21 @@ SECTIONS
   RODATA
 
   /* writeable */
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   .data : AT(ADDR(.data) - LOAD_OFFSET) {	/* Data */
 	DATA_DATA
 	CONSTRUCTORS
 	} :data
 
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   .data_nosave : AT(ADDR(.data_nosave) - LOAD_OFFSET) {
   	__nosave_begin = .;
 	*(.data.nosave)
-  	. = ALIGN(4096);
+  	. = ALIGN(PAGE_SIZE);
   	__nosave_end = .;
   }
 
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   .data.page_aligned : AT(ADDR(.data.page_aligned) - LOAD_OFFSET) {
 	*(.data.page_aligned)
 	*(.data.idt)
@@ -108,7 +108,7 @@ SECTIONS
   }
 
   /* might get freed after init */
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   .smp_locks : AT(ADDR(.smp_locks) - LOAD_OFFSET) {
   	__smp_locks = .;
 	*(.smp_locks)
@@ -120,10 +120,10 @@ SECTIONS
    * after boot. Always make sure that ALIGN() directive is present after
    * the section which contains __smp_alt_end.
    */
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
 
   /* will be freed after init */
-  . = ALIGN(4096);		/* Init code and data */
+  . = ALIGN(PAGE_SIZE);		/* Init code and data */
   .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) {
   	__init_begin = .;
 	_sinittext = .;
@@ -174,23 +174,23 @@ SECTIONS
 	EXIT_DATA
   }
 #if defined(CONFIG_BLK_DEV_INITRD)
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   .init.ramfs : AT(ADDR(.init.ramfs) - LOAD_OFFSET) {
 	__initramfs_start = .;
 	*(.init.ramfs)
 	__initramfs_end = .;
   }
 #endif
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   .data.percpu  : AT(ADDR(.data.percpu) - LOAD_OFFSET) {
 	__per_cpu_start = .;
 	*(.data.percpu)
 	*(.data.percpu.shared_aligned)
 	__per_cpu_end = .;
   }
-  . = ALIGN(4096);
+  . = ALIGN(PAGE_SIZE);
   /* freed after init ends here */
-	
+
   .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
 	__init_end = .;
 	__bss_start = .;		/* BSS */
@@ -200,7 +200,7 @@ SECTIONS
 	__bss_stop = .;
   	_end = . ;
 	/* This is where the kernel creates the early boot page tables */
-	. = ALIGN(4096);
+	. = ALIGN(PAGE_SIZE);
 	pg0 = . ;
   }
 

-- 

Re: [patch 1/2] x86: lds - Use PAGE_SIZE instead of numeric constant

[Sam Ravnborg]

On Sun, Feb 17, 2008 at 06:17:17PM +0300, Cyrill Gorcunov wrote:
> It's much better to use PAGE_SIZE then magic 4096
> (though it's almost synonym in most cases on x86 but
> not for *all* cases ;)

Thanks Cyrill.

> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

Re: [patch 1/2] x86: lds - Use PAGE_SIZE instead of numeric constant

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> On Sun, Feb 17, 2008 at 06:17:17PM +0300, Cyrill Gorcunov wrote:
> > It's much better to use PAGE_SIZE then magic 4096
> > (though it's almost synonym in most cases on x86 but
> > not for *all* cases ;)
> 
> Thanks Cyrill.
> 
> > 
> > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

thanks - applied.

	Ingo

[patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Cyrill Gorcunov]

[-- Attachment #1: x86-thread-size --]
[-- Type: text/plain, Size: 1331 bytes --]

Though we use PDA for regular task stack but that
is not acceptable for init_task wich is special
one. We still have to allocate init_task's stack
in that manner.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Ingo, Peter, I can be wrong but take a look please
to x86/lgest/i386_head.S:41

	/* Set up the initial stack so we can run C code. */
	movl $(init_thread_union+THREAD_SIZE),%esp

if we just eliminate this string from lds scripts - I bet
we get bad situation here. Xen has a similar trick.
And even head_32.S and head_64.S both have

32bits
======
ENTRY(stack_start)
	.long init_thread_union+THREAD_SIZE

64bits
======
	ENTRY(init_rsp)
	.quad  init_thread_union+THREAD_SIZE-8

So if we just remove that string from lds file - we will
be screwed up I think ;)

Index: linux-2.6.git/arch/x86/kernel/vmlinux_64.lds.S
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/vmlinux_64.lds.S	2008-02-17 13:56:54.000000000 +0300
+++ linux-2.6.git/arch/x86/kernel/vmlinux_64.lds.S	2008-02-17 14:10:21.000000000 +0300
@@ -129,7 +129,7 @@ SECTIONS
 #undef VVIRT_OFFSET
 #undef VVIRT
 
-  . = ALIGN(8192);		/* init_task */
+  . = ALIGN(THREAD_SIZE);	/* init_task */
   .data.init_task : AT(ADDR(.data.init_task) - LOAD_OFFSET) {
 	*(.data.init_task)
   }:data.init

-- 

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Sam Ravnborg]

On Sun, Feb 17, 2008 at 06:17:18PM +0300, Cyrill Gorcunov wrote:
> Though we use PDA for regular task stack but that
> is not acceptable for init_task wich is special
> one. We still have to allocate init_task's stack
> in that manner.

hpa had some comments about this particular alignment.
If we keep the alignmnet then Cyrill's patch looks good.

But maybe we should check what is the real alignmment
requirement (outside the scope of wha you do - someone
more intiminate with this coide should do it).

	Sam

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Cyrill Gorcunov]

[Sam Ravnborg - Sun, Feb 17, 2008 at 07:59:36PM +0100]
| On Sun, Feb 17, 2008 at 06:17:18PM +0300, Cyrill Gorcunov wrote:
| > Though we use PDA for regular task stack but that
| > is not acceptable for init_task wich is special
| > one. We still have to allocate init_task's stack
| > in that manner.
| 
| hpa had some comments about this particular alignment.
| If we keep the alignmnet then Cyrill's patch looks good.
| 
| But maybe we should check what is the real alignmment
| requirement (outside the scope of wha you do - someone
| more intiminate with this coide should do it).
| 
| 	Sam
| 

Yes Sam, I've read Peter and Ingo comments on this patch
(actually I sent the same patch maybe week ago and Peter
and Ingo told me that we use %fs,%gs now for stack) but
as I pointed in comment - this alignment is still using
by Xen and lguest and even x86. So - it would be really
usefull for me if someone give me the last point - YES we
use such an alignment for stack, NO - we don't use it at all.
I was trying to hadle this myself - but I'm not a specialist
in this area you know ;)

		- Cyrill -

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[H. Peter Anvin]

Cyrill Gorcunov wrote:
> 
> Yes Sam, I've read Peter and Ingo comments on this patch
> (actually I sent the same patch maybe week ago and Peter
> and Ingo told me that we use %fs,%gs now for stack) but
> as I pointed in comment - this alignment is still using
> by Xen and lguest and even x86. So - it would be really
> usefull for me if someone give me the last point - YES we
> use such an alignment for stack, NO - we don't use it at all.
> I was trying to hadle this myself - but I'm not a specialist
> in this area you know ;)
> 

It bugs me quite a bit that paravirt clients contain this hack 
open-coded, but that's not *your* fault.  There probably should be a 
macro to encapsulate the stack pointer location for a specific thread.

	-hpa

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Cyrill Gorcunov]

[H. Peter Anvin - Sun, Feb 17, 2008 at 11:48:58AM -0800]
> Cyrill Gorcunov wrote:
>> Yes Sam, I've read Peter and Ingo comments on this patch
>> (actually I sent the same patch maybe week ago and Peter
>> and Ingo told me that we use %fs,%gs now for stack) but
>> as I pointed in comment - this alignment is still using
>> by Xen and lguest and even x86. So - it would be really
>> usefull for me if someone give me the last point - YES we
>> use such an alignment for stack, NO - we don't use it at all.
>> I was trying to hadle this myself - but I'm not a specialist
>> in this area you know ;)
>
> It bugs me quite a bit that paravirt clients contain this hack open-coded, 
> but that's not *your* fault.  There probably should be a macro to 
> encapsulate the stack pointer location for a specific thread.
>
> 	-hpa
>

Thanks Peter for comments. Peter could you clarify for me a bit
more on the string:

x86/kernel/head_32.S:339

	/* Set up the stack pointer */
	lss stack_start,%esp

but stack_start is defined as head_32.S:647

.data
ENTRY(stack_start)
	.long init_thread_union+THREAD_SIZE

so stack_start *must* be aligned with THREAD_SIZE in vmlinux.lds
at compiling time. There is no PDA at this booting time. Am I wrong?
If you're too busy - just reply me like "Read the code" ;)

		- Cyrill -

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[H. Peter Anvin]

Cyrill Gorcunov wrote:
> 
> Thanks Peter for comments. Peter could you clarify for me a bit
> more on the string:
> 
> x86/kernel/head_32.S:339
> 
> 	/* Set up the stack pointer */
> 	lss stack_start,%esp
> 
> but stack_start is defined as head_32.S:647
> 
> .data
> ENTRY(stack_start)
> 	.long init_thread_union+THREAD_SIZE
> 
> so stack_start *must* be aligned with THREAD_SIZE in vmlinux.lds
> at compiling time. There is no PDA at this booting time. Am I wrong?
> If you're too busy - just reply me like "Read the code" ;)
> 

That doesn't follow.

You're saying that it *must* be aligned, I don't think that's true 
anymore; I think nowadays it's more accurate to say that it *is* 
aligned, but I don't think that's fundamental.

	-hpa

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Cyrill Gorcunov]

[H. Peter Anvin - Sun, Feb 17, 2008 at 12:11:45PM -0800]
> Cyrill Gorcunov wrote:
>> Thanks Peter for comments. Peter could you clarify for me a bit
>> more on the string:
>> x86/kernel/head_32.S:339
>> 	/* Set up the stack pointer */
>> 	lss stack_start,%esp
>> but stack_start is defined as head_32.S:647
>> .data
>> ENTRY(stack_start)
>> 	.long init_thread_union+THREAD_SIZE
>> so stack_start *must* be aligned with THREAD_SIZE in vmlinux.lds
>> at compiling time. There is no PDA at this booting time. Am I wrong?
>> If you're too busy - just reply me like "Read the code" ;)
>
> That doesn't follow.
>
> You're saying that it *must* be aligned, I don't think that's true anymore; 
> I think nowadays it's more accurate to say that it *is* aligned, but I 
> don't think that's fundamental.
>
> 	-hpa
>

Thanks a lot, Peter.

		- Cyrill -

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Andi Kleen]

> You're saying that it *must* be aligned, I don't think that's true 
> anymore; I think nowadays it's more accurate to say that it *is* 
> aligned, but I don't think that's fundamental.

At least in .25 mainline current_thread_info() is still using
[re]sp arithmetic.  Also I think the stack backtracers also
rely on suitable alignment of the stack top. So right now it is still 
fundamental. Now that could be all fixed by using segment
register references and might be, but so far that doesn't
seem to be finished yet.


-Andi

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> On Sun, Feb 17, 2008 at 06:17:18PM +0300, Cyrill Gorcunov wrote:
> > Though we use PDA for regular task stack but that
> > is not acceptable for init_task wich is special
> > one. We still have to allocate init_task's stack
> > in that manner.
> 
> hpa had some comments about this particular alignment. If we keep the 
> alignmnet then Cyrill's patch looks good.

ok. I've picked up Cyrill's cleanup - can i add your acked-by line?

	Ingo

Re: [patch 2/2] x86: lds - Use THREAD_SIZE instead of numeric constant

[Sam Ravnborg]

On Sun, Feb 17, 2008 at 08:08:29PM +0100, Ingo Molnar wrote:
> 
> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Sun, Feb 17, 2008 at 06:17:18PM +0300, Cyrill Gorcunov wrote:
> > > Though we use PDA for regular task stack but that
> > > is not acceptable for init_task wich is special
> > > one. We still have to allocate init_task's stack
> > > in that manner.
> > 
> > hpa had some comments about this particular alignment. If we keep the 
> > alignmnet then Cyrill's patch looks good.
> 
> ok. I've picked up Cyrill's cleanup - can i add your acked-by line?
Yes - I should have been specific and not just said "looks good".
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam