branch hint profiling of my nehalem machine, showed 96% incorrect branch
hints:
6548732 174664120 96 __switch_to process_64.c
406
6548745 174565593 96 __switch_to process_64.c
410
Signed-off-by: Tim Blechmann <[email protected]>
---
arch/x86/kernel/process_64.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index c8d0ece..1e2e1fe 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -403,11 +403,11 @@ __switch_to(struct task_struct *prev_p, struct
task_struct *next_p)
* This won't pick up thread selector changes, but I guess that is ok.
*/
savesegment(es, prev->es);
- if (unlikely(next->es | prev->es))
+ if (next->es | prev->es)
loadsegment(es, next->es);
savesegment(ds, prev->ds);
- if (unlikely(next->ds | prev->ds))
+ if (next->ds | prev->ds)
loadsegment(ds, next->ds);
-- 1.6.4.2
Commit-ID: a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
Gitweb: http://git.kernel.org/tip/a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
Author: Tim Blechmann <[email protected]>
AuthorDate: Tue, 24 Nov 2009 11:55:15 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 24 Nov 2009 12:20:04 +0100
sched, x86: Optimize branch hint in __switch_to()
Branch hint profiling on my nehalem machine showed 96%
incorrect branch hints:
6548732 174664120 96 __switch_to process_64.c
406
6548745 174565593 96 __switch_to process_64.c
410
Signed-off-by: Tim Blechmann <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/process_64.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index ad535b6..d9db104 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -406,11 +406,10 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
* This won't pick up thread selector changes, but I guess that is ok.
*/
savesegment(es, prev->es);
- if (unlikely(next->es | prev->es))
+ if (next->es | prev->es)
loadsegment(es, next->es);
-
savesegment(ds, prev->ds);
- if (unlikely(next->ds | prev->ds))
+ if (next->ds | prev->ds)
loadsegment(ds, next->ds);
On Tue, Nov 24, 2009 at 11:57 AM, tip-bot for Tim Blechmann
<[email protected]> wrote:
> Commit-ID: a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
> Gitweb: http://git.kernel.org/tip/a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
> Author: Tim Blechmann <[email protected]>
> AuthorDate: Tue, 24 Nov 2009 11:55:15 +0100
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Tue, 24 Nov 2009 12:20:04 +0100
>
> sched, x86: Optimize branch hint in __switch_to()
>
> Branch hint profiling on my nehalem machine showed 96%
> incorrect branch hints:
>
> 6548732 174664120 96 __switch_to process_64.c
> 406
> 6548745 174565593 96 __switch_to process_64.c
> 410
>
> Signed-off-by: Tim Blechmann <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Mike Galbraith <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> arch/x86/kernel/process_64.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index ad535b6..d9db104 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -406,11 +406,10 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
> * This won't pick up thread selector changes, but I guess that is ok.
> */
> savesegment(es, prev->es);
> - if (unlikely(next->es | prev->es))
> + if (next->es | prev->es)
> loadsegment(es, next->es);
> -
> savesegment(ds, prev->ds);
> - if (unlikely(next->ds | prev->ds))
> + if (next->ds | prev->ds)
> loadsegment(ds, next->ds);
>
>
64-bit tasks should have %ds and %es set to null selectors. The only
time they should be different is for 32-bit tasks. It lookx like some
versions of GCC just aren't using the hint. I've tested it with 4.4.2
and the generated assembly is the same with or without the hint.
--
Brian Gerst
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 11/24/2009 06:28 PM, Brian Gerst wrote:
> On Tue, Nov 24, 2009 at 11:57 AM, tip-bot for Tim Blechmann
> <[email protected]> wrote:
>> Commit-ID: a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
>> Gitweb: http://git.kernel.org/tip/a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
>> Author: Tim Blechmann <[email protected]>
>> AuthorDate: Tue, 24 Nov 2009 11:55:15 +0100
>> Committer: Ingo Molnar <[email protected]>
>> CommitDate: Tue, 24 Nov 2009 12:20:04 +0100
>>
>> sched, x86: Optimize branch hint in __switch_to()
>>
>> Branch hint profiling on my nehalem machine showed 96%
>> incorrect branch hints:
>>
>> 6548732 174664120 96 __switch_to process_64.c
>> 406
>> 6548745 174565593 96 __switch_to process_64.c
>> 410
>>
>> Signed-off-by: Tim Blechmann <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Mike Galbraith <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Arnaldo Carvalho de Melo <[email protected]>
>> Cc: Frederic Weisbecker <[email protected]>
>> LKML-Reference: <[email protected]>
>> Signed-off-by: Ingo Molnar <[email protected]>
>> ---
>> arch/x86/kernel/process_64.c | 5 ++---
>> 1 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
>> index ad535b6..d9db104 100644
>> --- a/arch/x86/kernel/process_64.c
>> +++ b/arch/x86/kernel/process_64.c
>> @@ -406,11 +406,10 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
>> * This won't pick up thread selector changes, but I guess that is ok.
>> */
>> savesegment(es, prev->es);
>> - if (unlikely(next->es | prev->es))
>> + if (next->es | prev->es)
>> loadsegment(es, next->es);
>> -
>> savesegment(ds, prev->ds);
>> - if (unlikely(next->ds | prev->ds))
>> + if (next->ds | prev->ds)
>> loadsegment(ds, next->ds);
>>
>>
>
> 64-bit tasks should have %ds and %es set to null selectors. The only
> time they should be different is for 32-bit tasks.
this doesn't seem to be the case on my machine for 96% of the calls ...
i am just running 64bit binaries on this machine, no 32bit programs
(that i know of (ubuntu karmic amd64))
tim
- --
[email protected]
http://tim.klingt.org
Relying on the government to protect your privacy is like asking a
peeping tom to install your window blinds.
John Perry Barlow
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iEYEARECAAYFAksM8s0ACgkQdL+4qsZfVsscjwCffcvQz7/VTtIfH1XXgJ2SxJxT
xWYAnjZ9aBD/OWmroVn68oEKqJZ7Pm3U
=C7kB
-----END PGP SIGNATURE-----
On Wed, Nov 25, 2009 at 4:03 AM, Tim Blechmann <[email protected]> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11/24/2009 06:28 PM, Brian Gerst wrote:
>> On Tue, Nov 24, 2009 at 11:57 AM, tip-bot for Tim Blechmann
>> <[email protected]> wrote:
>>> Commit-ID: a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
>>> Gitweb: http://git.kernel.org/tip/a3a1de0c34de6f5f8332cd6151c46af7813c0fcb
>>> Author: Tim Blechmann <[email protected]>
>>> AuthorDate: Tue, 24 Nov 2009 11:55:15 +0100
>>> Committer: Ingo Molnar <[email protected]>
>>> CommitDate: Tue, 24 Nov 2009 12:20:04 +0100
>>>
>>> sched, x86: Optimize branch hint in __switch_to()
>>>
>>> Branch hint profiling on my nehalem machine showed 96%
>>> incorrect branch hints:
>>>
>>> 6548732 174664120 96 __switch_to process_64.c
>>> 406
>>> 6548745 174565593 96 __switch_to process_64.c
>>> 410
>>>
>>> Signed-off-by: Tim Blechmann <[email protected]>
>>> Cc: Peter Zijlstra <[email protected]>
>>> Cc: Mike Galbraith <[email protected]>
>>> Cc: Paul Mackerras <[email protected]>
>>> Cc: Arnaldo Carvalho de Melo <[email protected]>
>>> Cc: Frederic Weisbecker <[email protected]>
>>> LKML-Reference: <[email protected]>
>>> Signed-off-by: Ingo Molnar <[email protected]>
>>> ---
>>> arch/x86/kernel/process_64.c | 5 ++---
>>> 1 files changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
>>> index ad535b6..d9db104 100644
>>> --- a/arch/x86/kernel/process_64.c
>>> +++ b/arch/x86/kernel/process_64.c
>>> @@ -406,11 +406,10 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
>>> * This won't pick up thread selector changes, but I guess that is ok.
>>> */
>>> savesegment(es, prev->es);
>>> - if (unlikely(next->es | prev->es))
>>> + if (next->es | prev->es)
>>> loadsegment(es, next->es);
>>> -
>>> savesegment(ds, prev->ds);
>>> - if (unlikely(next->ds | prev->ds))
>>> + if (next->ds | prev->ds)
>>> loadsegment(ds, next->ds);
>>>
>>>
>>
>> 64-bit tasks should have %ds and %es set to null selectors. The only
>> time they should be different is for 32-bit tasks.
>
> this doesn't seem to be the case on my machine for 96% of the calls ...
> i am just running 64bit binaries on this machine, no 32bit programs
> (that i know of (ubuntu karmic amd64))
>
> tim
Some early kernel threads inherited KERNEL_DS from the boot process.
Patch coming soon.
--
Brian Gerst
- /* set up data segments. actually 0 would do too */
- movl $__KERNEL_DS,%eax
+ /* set up data segments */
+ xorl %eax,%eax
movl %eax,%ds
movl %eax,%ss
movl %eax,%es
This prevents kernel threads from inheriting non-null segment selectors,
and causing optimizations in __switch_to() to be ineffective.
Signed-off-by: Brian Gerst <[email protected]>
---
arch/x86/kernel/head_64.S | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 780cd92..22db86a 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -212,8 +212,8 @@ ENTRY(secondary_startup_64)
*/
lgdt early_gdt_descr(%rip)
- /* set up data segments. actually 0 would do too */
- movl $__KERNEL_DS,%eax
+ /* set up data segments */
+ xorl %eax,%eax
movl %eax,%ds
movl %eax,%ss
movl %eax,%es
--
1.6.5.2
Commit-ID: 8ec6993d9f7d961014af970ded57542961fe9ad9
Gitweb: http://git.kernel.org/tip/8ec6993d9f7d961014af970ded57542961fe9ad9
Author: Brian Gerst <[email protected]>
AuthorDate: Wed, 25 Nov 2009 11:17:36 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 26 Nov 2009 10:44:30 +0100
x86, 64-bit: Set data segments to null after switching to 64-bit mode
This prevents kernel threads from inheriting non-null segment
selectors, and causing optimizations in __switch_to() to be
ineffective.
Signed-off-by: Brian Gerst <[email protected]>
Cc: Tim Blechmann <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Jeremy Fitzhardinge <[email protected]>
Cc: Jan Beulich <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/head_64.S | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 2640660..17ba9ec 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -212,8 +212,8 @@ ENTRY(secondary_startup_64)
*/
lgdt early_gdt_descr(%rip)
- /* set up data segments. actually 0 would do too */
- movl $__KERNEL_DS,%eax
+ /* set up data segments */
+ xorl %eax,%eax
movl %eax,%ds
movl %eax,%ss
movl %eax,%es
On 11/25/2009 05:17 PM, Brian Gerst wrote:
> This prevents kernel threads from inheriting non-null segment selectors,
> and causing optimizations in __switch_to() to be ineffective.
>
> Signed-off-by: Brian Gerst <[email protected]>
with this patch, the branch hint is actually correct, giving 100%
correct predictions. so please revert my earlier patch (commit
a3a1de0c34de6f5f8332cd6151c46af7813c0fcb)
thnx, tim
--
[email protected]
http://tim.klingt.org
Desperation is the raw material of drastic change. Only those who can
leave behind everything they have ever believed in can hope to escape.
William S. Burroughs
This reverts commit a3a1de0c34de6f5f8332cd6151c46af7813c0fcb.
Commit 8ec6993d9f7d961014af970ded57542961fe9ad9 cleared the es and ds
selectors, so the original branch hints are correct now. Therefore
the branch hint doesn't need to be removed.
Signed-off-by: Tim Blechmann <[email protected]>
---
arch/x86/kernel/process_64.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
Commit-ID: be8147e68625a1adb111acfd6b98a492be4b74d0
Gitweb: http://git.kernel.org/tip/be8147e68625a1adb111acfd6b98a492be4b74d0
Author: Tim Blechmann <[email protected]>
AuthorDate: Wed, 2 Dec 2009 12:32:10 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 2 Dec 2009 12:38:03 +0100
Revert "sched, x86: Optimize branch hint in __switch_to()"
This reverts commit a3a1de0c34de6f5f8332cd6151c46af7813c0fcb.
Commit 8ec6993d9f7d961014af970ded57542961fe9ad9 cleared the es
and ds selectors, so the original branch hints are correct now.
Therefore the branch hint doesn't need to be removed.
Signed-off-by: Tim Blechmann <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/process_64.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 93c501d..eb62cbc 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -406,10 +406,11 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
* This won't pick up thread selector changes, but I guess that is ok.
*/
savesegment(es, prev->es);
- if (next->es | prev->es)
+ if (unlikely(next->es | prev->es))
loadsegment(es, next->es);
+
savesegment(ds, prev->ds);
- if (next->ds | prev->ds)
+ if (unlikely(next->ds | prev->ds))
loadsegment(ds, next->ds);