2008-02-17 15:20:44

by Cyrill Gorcunov

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

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 <[email protected]>
---

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

--


2008-02-17 18:59:35

by Sam Ravnborg

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

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

2008-02-17 19:08:39

by Cyrill Gorcunov

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

[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 -

2008-02-17 19:08:55

by Ingo Molnar

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


* Sam Ravnborg <[email protected]> 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

2008-02-17 19:26:24

by Sam Ravnborg

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

On Sun, Feb 17, 2008 at 08:08:29PM +0100, Ingo Molnar wrote:
>
> * Sam Ravnborg <[email protected]> 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 <[email protected]>

Sam

2008-02-17 19:49:36

by H. Peter Anvin

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

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

2008-02-17 20:07:32

by Cyrill Gorcunov

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

[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 -

2008-02-17 20:12:49

by H. Peter Anvin

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

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

2008-02-17 20:16:33

by Cyrill Gorcunov

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

[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 -

2008-02-18 14:27:38

by Andi Kleen

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


> 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