| 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
| ||
|
Message-ID: <alpine.LFD.2.01.0908011214330.3304@localhost.localdomain>
Date: Sat, 1 Aug 2009 12:28:01 -0700 (PDT)
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Ingo Molnar <mingo@...e.hu>
cc: "H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Tejun Heo <tj@...nel.org>
Subject: Re: [GIT PULL] Additional x86 fixes for 2.6.31-rc5
Hmm.
I just noticed another issue on x86 code generation, since I was looking
at assembly language generation due to the do_sigaltstack() kernel stack
info leak thing.
Our "get_current()" seriously sucks now that it's a per-cpu variable.
Look at the code generated for something like
current->sas_ss_sp = (unsigned long) ss_sp;
current->sas_ss_size = ss_size;
and notice how the code really really sucks:
movq %gs:per_cpu__current_task,%rcx
movq %rdx, 1152(%rcx)
movq %gs:per_cpu__current_task,%rdx
movq %rax, 1160(%rdx)
because it reloads that silly per-cpu variable every time, because the
assembler has a constraint of
"m" (per_cpu__current_task)
and so gcc is worried that the stores will invalidate the result of the
load from the per-cpu variable.
I don't know how to fix that _well_, but here's a not-so-very-pretty patch
that seems to shave off 4.5kB from my kernel, and gives gcc much better
scheduling for 'current' and 'thread_info' because now it can load them
early - and cache them - even in the presense of stores.
It uses a "p" (&var) constraint instead of a "m" (var) one, to make gcc
think there is no actual "load" from memory. This obviously _only_ works
for percpu variables that are stable within a thread, but 'current' and
'kernel_stack' should be that way.
End result: the above horror becomes a more reasonable
movq %gs:per_cpu__current_task,%rax
movq %rcx, 1152(%rax)
movq %rdx, 1160(%rax)
instead (it still doesn't cache it over the whole function, but it's
certainly better).
NOTE! I did not test that it all worked. I only looked at the asm, and
checked out the improvements. All the ones I looked at looked reasonable.
Worthwhile? You be the judge.
There's another detail that may be worth looking at: we often get
'current' and 'thread_info' together, and they are _not_ in the same
cache-line. It might be worth defining them together in the per-cpu data,
and making sure they are in the same cacheline too. In general, we should
probably look at which per-pcu variables are hot and read-only, and try to
gathe them all together.
Linus
---
From: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Sat, 1 Aug 2009 11:50:54 -0700
Subject: [PATCH] x86-64: Add 'percpu_read_stable()' interface for cacheable accesses
This is very useful for some common things like 'get_current()' and
'get_thread_info()', which can be used multiple times in a function, and
where the result is cacheable.
Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
---
arch/x86/include/asm/current.h | 2 +-
arch/x86/include/asm/percpu.h | 13 +++++++------
arch/x86/include/asm/thread_info.h | 2 +-
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
index c68c361..4d447b7 100644
--- a/arch/x86/include/asm/current.h
+++ b/arch/x86/include/asm/current.h
@@ -11,7 +11,7 @@ DECLARE_PER_CPU(struct task_struct *, current_task);
static __always_inline struct task_struct *get_current(void)
{
- return percpu_read(current_task);
+ return percpu_read_stable(current_task);
}
#define current get_current()
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 103f1dd..3fd619b 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -104,36 +104,37 @@ do { \
} \
} while (0)
-#define percpu_from_op(op, var) \
+#define percpu_from_op(op, var, constraint) \
({ \
typeof(var) ret__; \
switch (sizeof(var)) { \
case 1: \
asm(op "b "__percpu_arg(1)",%0" \
: "=q" (ret__) \
- : "m" (var)); \
+ : constraint); \
break; \
case 2: \
asm(op "w "__percpu_arg(1)",%0" \
: "=r" (ret__) \
- : "m" (var)); \
+ : constraint); \
break; \
case 4: \
asm(op "l "__percpu_arg(1)",%0" \
: "=r" (ret__) \
- : "m" (var)); \
+ : constraint); \
break; \
case 8: \
asm(op "q "__percpu_arg(1)",%0" \
: "=r" (ret__) \
- : "m" (var)); \
+ : constraint); \
break; \
default: __bad_percpu_size(); \
} \
ret__; \
})
-#define percpu_read(var) percpu_from_op("mov", per_cpu__##var)
+#define percpu_read(var) percpu_from_op("mov", per_cpu__##var,"m" (per_cpu__##var))
+#define percpu_read_stable(var) percpu_from_op("mov", per_cpu__##var,"p" (&per_cpu__##var))
#define percpu_write(var, val) percpu_to_op("mov", per_cpu__##var, val)
#define percpu_add(var, val) percpu_to_op("add", per_cpu__##var, val)
#define percpu_sub(var, val) percpu_to_op("sub", per_cpu__##var, val)
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index fad7d40..a1bb5a1 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -213,7 +213,7 @@ DECLARE_PER_CPU(unsigned long, kernel_stack);
static inline struct thread_info *current_thread_info(void)
{
struct thread_info *ti;
- ti = (void *)(percpu_read(kernel_stack) +
+ ti = (void *)(percpu_read_stable(kernel_stack) +
KERNEL_STACK_OFFSET - THREAD_SIZE);
return ti;
}
--
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/
Powered by blists - more mailing lists