x86_64: tss_struct layout does not match comments !?

x86_64: tss_struct layout does not match comments !?

[Richard Kennedy]

Hi Ingo,

the comments in the definition of tss_struct suggests is should be
cacheline aligned ( or 256 byte aligned ? ) :- 

|struct tss_struct {
|....
|	/*
|	 * Pad the TSS to be cacheline-aligned (size is 0x100):
|	 */
|	unsigned long		__cacheline_filler[35];
|	/*
|	 * .. and then another 0x100 bytes for the emergency kernel stack:
|	 */
|	unsigned long		stack[64];
|
|} __attribute__((packed));

However on a 64 bit build the size of tss_struct is 9136,
cacheline_filler is 280 and stack size is 512 at offset 8624.
None of which are cacheline aligned.

I'm guessing this isn't what was intended.

do you know what the original intention was ?

1. struct tss_struct{...} __cacheline_aligned;
or just
2. struct tss_struct {
...
long stack[64] __cacheline_aligned;
}

Richard

Re: x86_64: tss_struct layout does not match comments !?

[Ingo Molnar]

* Richard Kennedy <richard@rsk.demon.co.uk> wrote:

> Hi Ingo,
> 
> the comments in the definition of tss_struct suggests is should be
> cacheline aligned ( or 256 byte aligned ? ) :- 
> 
> |struct tss_struct {
> |....
> |	/*
> |	 * Pad the TSS to be cacheline-aligned (size is 0x100):
> |	 */
> |	unsigned long		__cacheline_filler[35];
> |	/*
> |	 * .. and then another 0x100 bytes for the emergency kernel stack:
> |	 */
> |	unsigned long		stack[64];
> |
> |} __attribute__((packed));
> 
> However on a 64 bit build the size of tss_struct is 9136,
> cacheline_filler is 280 and stack size is 512 at offset 8624.
> None of which are cacheline aligned.
> 
> I'm guessing this isn't what was intended.
> 
> do you know what the original intention was ?
> 
> 1. struct tss_struct{...} __cacheline_aligned;
> or just
> 2. struct tss_struct {
> ...
> long stack[64] __cacheline_aligned;
> }

#1 is the intent - because each CPU has a separate TSS. init_tss.stack 
is really just an emergency static stack we have in place for very early 
exceptions.

i think the __cacheline_filler could be removed safely. Mind sending a 
patch for that?

	Ingo

Re: x86_64: tss_struct layout does not match comments !?

[Richard Kennedy]

On Thu, 2008-07-03 at 13:39 +0200, Ingo Molnar wrote:
> * Richard Kennedy <richard@rsk.demon.co.uk> wrote:
> 
> > Hi Ingo,
> > 
> > the comments in the definition of tss_struct suggests is should be
> > cacheline aligned ( or 256 byte aligned ? ) :- 
> > 
> > |struct tss_struct {
> > |....
> > |	/*
> > |	 * Pad the TSS to be cacheline-aligned (size is 0x100):
> > |	 */
> > |	unsigned long		__cacheline_filler[35];
> > |	/*
> > |	 * .. and then another 0x100 bytes for the emergency kernel stack:
> > |	 */
> > |	unsigned long		stack[64];
> > |
> > |} __attribute__((packed));
> > 
> > However on a 64 bit build the size of tss_struct is 9136,
> > cacheline_filler is 280 and stack size is 512 at offset 8624.
> > None of which are cacheline aligned.
> > 
> > I'm guessing this isn't what was intended.
> > 
> > do you know what the original intention was ?
> > 
> > 1. struct tss_struct{...} __cacheline_aligned;
> > or just
> > 2. struct tss_struct {
> > ...
> > long stack[64] __cacheline_aligned;
> > }
> 
> #1 is the intent - because each CPU has a separate TSS. init_tss.stack 
> is really just an emergency static stack we have in place for very early 
> exceptions.
> 
> i think the __cacheline_filler could be removed safely. Mind sending a 
> patch for that?
> 
> 	Ingo

No problem.
Richard 

[PATCH] x86: cacheline_align tss_struct

[Richard Kennedy]

The manual padding to align on cacheline size only worked in 32 bit
In 64 bit the structure was not aligned and contained wasted space.
    
use the compiler ____cachline_aligned to save space & properly align
this structure.
    
x86_64_default size goes from 9136 -> 8960
x86_64_AMD     size goes from 9136 -> 8896
    
    
Signed-off-by: Richard Kennedy <richard@rsk.demon.co.uk>
---

Hi Ingo
here's the patch.
built & running on 2.6.26-rc8.

Richard



diff --git a/include/asm-x86/processor.h b/include/asm-x86/processor.h
index 5591052..4ab2ede 100644
--- a/include/asm-x86/processor.h
+++ b/include/asm-x86/processor.h
@@ -263,15 +263,11 @@ struct tss_struct {
 	struct thread_struct	*io_bitmap_owner;
 
 	/*
-	 * Pad the TSS to be cacheline-aligned (size is 0x100):
-	 */
-	unsigned long		__cacheline_filler[35];
-	/*
 	 * .. and then another 0x100 bytes for the emergency kernel stack:
 	 */
 	unsigned long		stack[64];
 
-} __attribute__((packed));
+} ____cacheline_aligned;
 
 DECLARE_PER_CPU(struct tss_struct, init_tss);
 

Re: [PATCH] x86: cacheline_align tss_struct

[Ingo Molnar]

* Richard Kennedy <richard@rsk.demon.co.uk> wrote:

> The manual padding to align on cacheline size only worked in 32 bit
> In 64 bit the structure was not aligned and contained wasted space.
>     
> use the compiler ____cachline_aligned to save space & properly align
> this structure.
>     
> x86_64_default size goes from 9136 -> 8960
> x86_64_AMD     size goes from 9136 -> 8896

applied to tip/x86/cleanups - thanks Richard!

	Ingo