linux-kernel - Re: [PATCH 0/5] branch hint tweaks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 24 Nov 2009 15:13:02 -0500
From:	Brian Gerst <brgerst@...il.com>
To:	Tim Blechmann <tim@...ngt.org>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/5] branch hint tweaks

On Tue, Nov 24, 2009 at 1:21 PM, Tim Blechmann <tim@...ngt.org> wrote:
>> Did you run profiling tests again after making these changes to see if
>> they had any effect?  likely() and unlikely() are only hints.  GCC
>> doesn't have to follow them, or it could be broken in recent GCC
>> versions.
>
> i know, the compiler doesn't have to follow the hint ... but with the
> likely/unlikely profiling, not the execution time is profiled, but
> whether the branch hint is pointing to the right direction on my machine
> ... if the assembly is actually affected does probably depend on the
> compiler version, instruction set, cpu tuning ...

The only branch "hint" in the final assembly output is whether a
branch points forward or backward.  unlikely() should tell GCC to put
the code at the end of the function, and use a forward branch to it.

Take for instance the code in arch/x86/kernel/process_64.c:

        savesegment(es, prev->es);
        if (unlikely(next->es | prev->es))
                loadsegment(es, next->es);

 5eb:   66 8c c1                mov    %es,%cx
 5ee:   66 89 48 30             mov    %cx,0x30(%rax)
 5f2:   66 83 bb a8 04 00 00    cmpw   $0x0,0x4a8(%rbx)
 5f9:   00
 5fa:   41 8b 8d a8 04 00 00    mov    0x4a8(%r13),%ecx
 601:   75 05                   jne    608 <__switch_to+0x86>
 603:   66 85 c9                test   %cx,%cx
 606:   74 04                   je     60c <__switch_to+0x8a>
 608:   31 f6                   xor    %esi,%esi
 60a:   8e c1                   mov    %ecx,%es

Both of those branches are forward, which will be statically predicted
as not taken.  Removing the unlikely() did not change the generated
code in this case with GCC 4.4.2.  So this patch does nothing for that
compiler version, but will lead to worse code on compilers that do
respect the hint.

Note that there is some weirdness in how the actual test is generated.
 It's not doing a bitwise-or operation like the C code describes.
This could be throwing off the optimizer.

--
Brian Gerst
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/