Re: [RFC] Observations on x86 process.c

Re: [RFC] Observations on x86 process.c

[Jan Engelhardt]

>A) dump_thread() and dump_task_regs() are in the middle of the file, but
>will be infrequently used. With default 16 byte alignment, this may cause
>bits of them to wind up polluting the L1 on anything with L1 lines > 16
>bytes.  L2 lines could be similarly polluted too of course.

C compilers are free to reorder functions (are they?), especially GCC when it 
is passed -funit-at-a-time (which currently is not in CFLAGS).



Jan Engelhardt                                                               
--                                                                            
| Gesellschaft fuer Wissenschaftliche Datenverarbeitung Goettingen,
| Am Fassberg, 37077 Goettingen, www.gwdg.de

[RFC] Observations on x86 process.c

[cutaway]

A) dump_thread() and dump_task_regs() are in the middle of the file, but
will be infrequently used. With default 16 byte alignment, this may cause
bits of them to wind up polluting the L1 on anything with L1 lines > 16
bytes.  L2 lines could be similarly polluted too of course.

Moving these two routines to the bottom would probably be a better deal.

B) elf_core_copy_regs() macro (which resolves to ELF_CORE_COPY_REGS macro)
just copies largely similar (but not quite identical) structures with a bit
of difference in the middle for seg reg handling using a long sequence of "a
= b" type assignments.  It would seem this could be tweaked a bit with a
couple of REP MOV's on either side of the seg reg dissimilarity.  Fast crash
dump handling code isn't as desirable as compact crash dump handling code.

C) dump_task_regs() can be shortened up a tad by zeroing the high words of
the seg reg vars with a bit of inline that uses a word AND with imm8 zero.
Right now the compiler is generating 4 MOVZX's and 4 MOV's to clip off the
trash bits. Again, not being a high performance path, the better compactness
of (4) AND mem16,imm8 would be more desirable over the 8 MOVZX/MOV
instructions that get generated now.

Re: [RFC] Observations on x86 process.c

[Denis Vlasenko]

On Monday 13 June 2005 16:02, cutaway@bellsouth.net wrote:
> A) dump_thread() and dump_task_regs() are in the middle of the file, but
> will be infrequently used. With default 16 byte alignment, this may cause
> bits of them to wind up polluting the L1 on anything with L1 lines > 16
> bytes.  L2 lines could be similarly polluted too of course.
> Moving these two routines to the bottom would probably be a better deal.

What about having a __speed macro:

int very_frequently_user_func() __speed {
...
}

Unconditionally aligning all fns to 16 bytes is a waste of cache
in lieu of 80/20 rule ("80% of execution time is spent in 20% of the code")...

> B) elf_core_copy_regs() macro (which resolves to ELF_CORE_COPY_REGS macro)
> just copies largely similar (but not quite identical) structures with a bit
> of difference in the middle for seg reg handling using a long sequence of "a
> = b" type assignments.  It would seem this could be tweaked a bit with a
> couple of REP MOV's on either side of the seg reg dissimilarity.  Fast crash
> dump handling code isn't as desirable as compact crash dump handling code.

http://lxr.linux.no/source/include/asm-i386/elf.h#L78
75 /* regs is struct pt_regs, pr_reg is elf_gregset_t (which is
76    now struct_user_regs, they are different) */
77 
78 #define ELF_CORE_COPY_REGS(pr_reg, regs)                \
79         pr_reg[0] = regs->ebx;                          \
80         pr_reg[1] = regs->ecx;                          \
81         pr_reg[2] = regs->edx;                          \
82         pr_reg[3] = regs->esi;                          \
83         pr_reg[4] = regs->edi;                          \
84         pr_reg[5] = regs->ebp;                          \
85         pr_reg[6] = regs->eax;                          \
86         pr_reg[7] = regs->xds;                          \
87         pr_reg[8] = regs->xes;                          \
88         savesegment(fs,pr_reg[9]);                      \
89         savesegment(gs,pr_reg[10]);                     \
90         pr_reg[11] = regs->orig_eax;                    \
91         pr_reg[12] = regs->eip;                         \
92         pr_reg[13] = regs->xcs;                         \
93         pr_reg[14] = regs->eflags;                      \
94         pr_reg[15] = regs->esp;                         \
95         pr_reg[16] = regs->xss;

You are basically proposing micro-optimizing it with asm().
It it *that* critical?

> C) dump_task_regs() can be shortened up a tad by zeroing the high words of
> the seg reg vars with a bit of inline that uses a word AND with imm8 zero.
> Right now the compiler is generating 4 MOVZX's and 4 MOV's to clip off the
> trash bits. Again, not being a high performance path, the better compactness
> of (4) AND mem16,imm8 would be more desirable over the 8 MOVZX/MOV
> instructions that get generated now.

What's wrong with 16bit MOVs? Anyway.
You shouldn't tailor code for specific compiler peculiarities
or rewrite code with asm().

If you really want gcc to generate better code - try to come up
with gcc patch so that it notices such optimization opportunities:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21329
--
vda

Re: [RFC] Observations on x86 process.c

[cutaway]

They are free to, but I've rarely(never?) seen GCC actually do it with the
default build setup.  Don't trust me - look at a generated ASM listing and
convince yourself what I'm saying is true.  GCC does a lot of massive
reordering within functions though.

Throw a "CFLAGS += -Wa,-al=$<.lst" at the top of the Makefile.  That'll
generate a "process.c.lst" assembler listing file.

IAC, it would be harmless if one did, and goodness if one doesn't.  That
sounds like a win to me :)

 Tony

----- Original Message ----- 
From: "Jan Engelhardt" <jengelh@linux01.gwdg.de>
>
> C compilers are free to reorder functions (are they?), especially GCC when
it
> is passed -funit-at-a-time (which currently is not in CFLAGS).

Re: [RFC] Observations on x86 process.c

[Denis Vlasenko]

> > What about having a __speed macro:
> >
> > int very_frequently_user_func() __speed {
> > ...
> > }
> >
> > Unconditionally aligning all fns to 16 bytes is a waste of cache
> > in lieu of 80/20 rule ("80% of execution time is spent in 20% of the
> code")...
> 
> This was going to be a follow up observation<g>...  the sleep/wake inc/dec
> functions at the top of the file could/should also be relocated.  Going into
> a coma and coming out of one probably aren't happen often enough to
> potentially be burning L1/L2 lines on them.  Probably not very useful to
> have (2) instruction functions being 16 byte aligned either when both would
> comfortably fit in a single line.

I am not affected. I compile my kernels with zero alignment.

Not my itch to scratch.

> > http://lxr.linux.no/source/include/asm-i386/elf.h#L78
> > 75 /* regs is struct pt_regs, pr_reg is elf_gregset_t (which is
> > 76    now struct_user_regs, they are different) */
> > 77
> > 78 #define ELF_CORE_COPY_REGS(pr_reg, regs)                \
> > 79         pr_reg[0] = regs->ebx;                          \
> > 80         pr_reg[1] = regs->ecx;                          \
> > 81         pr_reg[2] = regs->edx;                          \
> > 82         pr_reg[3] = regs->esi;                          \
> > 83         pr_reg[4] = regs->edi;                          \
> > 84         pr_reg[5] = regs->ebp;                          \
> > 85         pr_reg[6] = regs->eax;                          \
> > 86         pr_reg[7] = regs->xds;                          \
> > 87         pr_reg[8] = regs->xes;                          \
> > 88         savesegment(fs,pr_reg[9]);                      \
> > 89         savesegment(gs,pr_reg[10]);                     \
> > 90         pr_reg[11] = regs->orig_eax;                    \
> > 91         pr_reg[12] = regs->eip;                         \
> > 92         pr_reg[13] = regs->xcs;                         \
> > 93         pr_reg[14] = regs->eflags;                      \
> > 94         pr_reg[15] = regs->esp;                         \
> > 95         pr_reg[16] = regs->xss;
> >
> > You are basically proposing micro-optimizing it with asm().
> > It it *that* critical?
> 
> For speed? No.  Speed of crashing isn't important<g>.  Actually, functions
> like these should be taken out of mainline pages completely and relegated to
> a page(s) dedicated to rarely used but necessary routines.  This has more to
> do with cache pollution than anything else.

Do you realize how large linux kernel is? Are you going to optimize all of it
by hand?!

> > You shouldn't tailor code for specific compiler peculiarities
> > or rewrite code with asm().
> >
> > If you really want gcc to generate better code - try to come up
> > with gcc patch so that it notices such optimization opportunities:
> 
> We have no power to "unship" existing compilers in the field, and it seems
> harsh to penalize people using them IMO.  Tuning NOW for everything in the
> field gives advantage NOW.  If GCC ever becomes more intelligent about its
> code gen that's nice, but it doesn't help the several flavors in common use
> floating around right now.

People tend to gradually update their systems. Make gcc better, and it will
pay back with time. If you want that benefit _now_, then use your better gcc
immediately instead of stone age one. Others will take care of themselves.

If you feel like sprinkling tons of asm() over zillions of lines of code instead
- good luck :)
--
vda

Re: [RFC] Observations on x86 process.c

[cutaway]

----- Original Message ----- 
From: "Denis Vlasenko" <vda@ilport.com.ua>
To: <cutaway@bellsouth.net>; <linux-kernel@vger.kernel.org>
Sent: Monday, June 13, 2005 09:18
Subject: Re: [RFC] Observations on x86 process.c


> On Monday 13 June 2005 16:02, cutaway@bellsouth.net wrote:
> > A) dump_thread() and dump_task_regs() are in the middle of the file, but
> > will be infrequently used. With default 16 byte alignment, this may
cause
> > bits of them to wind up polluting the L1 on anything with L1 lines > 16
> > bytes.  L2 lines could be similarly polluted too of course.
> > Moving these two routines to the bottom would probably be a better deal.
>
> What about having a __speed macro:
>
> int very_frequently_user_func() __speed {
> ...
> }
>
> Unconditionally aligning all fns to 16 bytes is a waste of cache
> in lieu of 80/20 rule ("80% of execution time is spent in 20% of the
code")...

This was going to be a follow up observation<g>...  the sleep/wake inc/dec
functions at the top of the file could/should also be relocated.  Going into
a coma and coming out of one probably aren't happen often enough to
potentially be burning L1/L2 lines on them.  Probably not very useful to
have (2) instruction functions being 16 byte aligned either when both would
comfortably fit in a single line.

>
> > B) elf_core_copy_regs() macro (which resolves to ELF_CORE_COPY_REGS
macro)
> > just copies largely similar (but not quite identical) structures with a
bit
> > of difference in the middle for seg reg handling using a long sequence
of "a
> > = b" type assignments.  It would seem this could be tweaked a bit with a
> > couple of REP MOV's on either side of the seg reg dissimilarity.  Fast
crash
> > dump handling code isn't as desirable as compact crash dump handling
code.
>
> http://lxr.linux.no/source/include/asm-i386/elf.h#L78
> 75 /* regs is struct pt_regs, pr_reg is elf_gregset_t (which is
> 76    now struct_user_regs, they are different) */
> 77
> 78 #define ELF_CORE_COPY_REGS(pr_reg, regs)                \
> 79         pr_reg[0] = regs->ebx;                          \
> 80         pr_reg[1] = regs->ecx;                          \
> 81         pr_reg[2] = regs->edx;                          \
> 82         pr_reg[3] = regs->esi;                          \
> 83         pr_reg[4] = regs->edi;                          \
> 84         pr_reg[5] = regs->ebp;                          \
> 85         pr_reg[6] = regs->eax;                          \
> 86         pr_reg[7] = regs->xds;                          \
> 87         pr_reg[8] = regs->xes;                          \
> 88         savesegment(fs,pr_reg[9]);                      \
> 89         savesegment(gs,pr_reg[10]);                     \
> 90         pr_reg[11] = regs->orig_eax;                    \
> 91         pr_reg[12] = regs->eip;                         \
> 92         pr_reg[13] = regs->xcs;                         \
> 93         pr_reg[14] = regs->eflags;                      \
> 94         pr_reg[15] = regs->esp;                         \
> 95         pr_reg[16] = regs->xss;
>
> You are basically proposing micro-optimizing it with asm().
> It it *that* critical?

For speed? No.  Speed of crashing isn't important<g>.  Actually, functions
like these should be taken out of mainline pages completely and relegated to
a page(s) dedicated to rarely used but necessary routines.  This has more to
do with cache pollution than anything else.

>
> > C) dump_task_regs() can be shortened up a tad by zeroing the high words
of
> > the seg reg vars with a bit of inline that uses a word AND with imm8
zero.
> > Right now the compiler is generating 4 MOVZX's and 4 MOV's to clip off
the
> > trash bits. Again, not being a high performance path, the better
compactness
> > of (4) AND mem16,imm8 would be more desirable over the 8 MOVZX/MOV
> > instructions that get generated now.
>
> What's wrong with 16bit MOVs? Anyway.
> You shouldn't tailor code for specific compiler peculiarities
> or rewrite code with asm().
>
> If you really want gcc to generate better code - try to come up
> with gcc patch so that it notices such optimization opportunities:

We have no power to "unship" existing compilers in the field, and it seems
harsh to penalize people using them IMO.  Tuning NOW for everything in the
field gives advantage NOW.  If GCC ever becomes more intelligent about its
code gen that's nice, but it doesn't help the several flavors in common use
floating around right now.

Re: [RFC] Observations on x86 process.c

[cutaway]

----- Original Message ----- 
From: "Denis Vlasenko" <vda@ilport.com.ua>
>
> I am not affected. I compile my kernels with zero alignment.
>
> Not my itch to scratch.

Does some clueless user who needs a custom kernel for something even
understand what you just said?  Probably not...

>
> Do you realize how large linux kernel is? Are you going to optimize all of
it
> by hand?!

Its big, but not as big as some I've worked on.  I'm willing to tune the
"arch" specific stuff where I can do better than the compiler.  A few bytes
here, a few there, pretty soon you got a page back.  BTW, there's just under
100K of fluff zero padding in locked pages because of the way kernel
messages are being 32 byte aligned.  Do you think getting 100K of locked
kernel memory back essentially "for free" is worth taking a shot at?  I do.

>
> People tend to gradually update their systems. Make gcc better, and it
will
> pay back with time. If you want that benefit _now_, then use your better
gcc
> immediately instead of stone age one. Others will take care of themselves.
>
Pretty harsh attitude IMO.  People use compilers because they DO want
someone to take care of some things for them.

Re: [RFC] Observations on x86 process.c

[Ian Campbell]

On Mon, 2005-06-13 at 12:32 -0400, cutaway@bellsouth.net wrote:
> Where __attribute__((slowcode)) is defined in some macro.

I think you should probably checkout the likely() and unlikely() macros
which are already defined for use in the kernel.
Ian.

-- 
Ian Campbell
Current Noise: Metallica - Fuel

When the cup is full, carry it level.

Re: [RFC] Observations on x86 process.c

[cutaway]

This is a good idea BTW.

It could be expanded upon further by adding something else to designate
error case and infrequent oddball situation code blocks.  Even "fast"
functions often include large sections of stuff that doesn't need speed
optimizations.  That stuff just winds up puffing up the L1/L2 when bits of
it get touched.  A compiler smart enough to emit function code to two
different .text regions could take advantage of this.

ex.

int __fast fast1(whatever...)
{
    if (rare_case) {
    ...large blob of non-speed critical code...
    }
    ...fast code...
}

int __fast fast2(whatever...)

The way it happens today is GCC (usually) relocates the blob of speed
insensitive code towards the bottom of the function, which means you're less
likely to pick up a bit of fast2()'s prolog as you're exiting fast1, and
more likely to pollute the caches with worthless stuff.

If blocks can be designated as infrequent/error paths and relocatable to a
different .text section you stand a much better chance of picking up
something useful as functions exit.  ex:

int __fast fast1(whatever...)
{
    if (rare_case) __attribute__((slowcode)) {
    ...large blob of non-speed critical code...
    }
    ...fast code...
}

Where __attribute__((slowcode)) is defined in some macro.


----- Original Message ----- 
From: "Denis Vlasenko" <vda@ilport.com.ua>
>
> What about having a __speed macro:
>
> int very_frequently_user_func() __speed {
> ...
> }

Re: [RFC] Observations on x86 process.c

[cutaway]

I'm well aware of them Ian.  What I'm talking about is a somewhat different
broader notion that can be applied to things other than conditionals.

ex.

switch(whatever)
    {
    case blah:
        __attribute__((slowcode)) {
        /*
          An infrequently used, but plump block of code.
        */
        }

    case blahblah:
    ...etc...
    }


----- Original Message ----- 
From: "Ian Campbell" <ijc@hellion.org.uk>
To: <cutaway@bellsouth.net>
>
> I think you should probably checkout the likely() and unlikely() macros
> which are already defined for use in the kernel.
> Ian.