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
On Thu, 2008-07-03 at 13:39 +0200, Ingo Molnar wrote:
> * Richard Kennedy <[email protected]> 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
* Richard Kennedy <[email protected]> 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
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 <[email protected]>
---
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);
* Richard Kennedy <[email protected]> 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