x86/delay: Do not use cpu_tss in preemptible ctxt in delay_mwaitx()
Borislav Petkov
maintainerRFC PATCH 1/1 x86/delay: Do not use cpu_tss in preemptible ctxt in delay_mwaitx()
From: Borislav Petkov <[email protected]>
So Andy had a good idea about using a cacheline-aligned, seldomly used per-cpu var as the MONITORX target but we can't use it in preemptible context. The first simple idea I have is to disable preemption around us dereffing it.
Better ideas?
--- arch/x86/lib/delay.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
arch/x86/lib/delay.c +10 -5
@@ -92,17 +92,22 @@ static void delay_tsc(unsigned long __loops)static void delay_mwaitx (unsigned long __loops) {u64 start, end, delay, loops = __loops; +struct tss_struct *t; + +/* +* Use cpu_tss as a cacheline-aligned, seldomly accessed per-cpu +* variable as the monitor target. +*/ +preempt_disable (); + t =this_cpu_ptr (&cpu_tss); +preempt_enable (); start =rdtsc_ordered ();for (;;) { delay =min_t (u64,MWAITX_MAX_LOOPS , loops); -/* -* Use cpu_tss as a cacheline-aligned, seldomly -* accessed per-cpu variable as the monitor target. -*/ -__monitorx (this_cpu_ptr (&cpu_tss),0 ,0 ); +__monitorx (t,0 ,0 );/* * AMD, like Intel, supports the EAX hint and EAX=0xf
Signature · 1 line
2.3.5
Quoted 7 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
So Andy had a good idea about using a cacheline-aligned, seldomly used per-cpu var as the MONITORX target but we can't use it in preemptible context. The first simple idea I have is to disable preemption around us dereffing it.
What's the actual problem? Is it the preempt warnings and, if so, would raw_cpu_ptr fix it?
It may pay to move it into the loop, though.
--Andy
Borislav Petkov
maintainerQuoted 11 lines · Andy Lutomirski
Quoted 7 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
So Andy had a good idea about using a cacheline-aligned, seldomly used per-cpu var as the MONITORX target but we can't use it in preemptible context. The first simple idea I have is to disable preemption around us dereffing it.
What's the actual problem? Is it the preempt warnings and, if so, would
raw_cpu_ptrfix it?
Yeah, it is the warning:
[ 1.565876] BUG: using smp_processor_id() in preemptible [00000000] code: udevd/312 [ 1.566123] caller is delay_mwaitx+0x40/0xa0
and yes, I think so. I don't think we care about being in preemptible context since we're going idle anyway and doesn't matter which cpu_tss we touch.
Yeah, I'll use raw_cpu_ptr...
Quoted 1 line
It may pay to move it into the loop, though.
... and won't need to do that.
Thanks for the idea.
Signature · 4 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.Borislav Petkov
maintainerPATCH 1/1 x86/delay: Avoid preemptible context checks in delay_mwaitx()
We do use this_cpu_ptr(&cpu_tss) as a cacheline-aligned, seldomly accessed per-cpu var as the MONITORX target in delay_mwaitx(). However, when called in preemptible context, this_cpu_ptr -> smp_processor_id() -> debug_smp_processor_id() fires:
BUG: using smp_processor_id() in preemptible [00000000] code: udevd/312 caller is delay_mwaitx+0x40/0xa0
But we don't care about that check - we only need cpu_tss as a MONITORX target and it doesn't really matter which CPU's var we're touching as we're going idle anyway. Fix that.
--- arch/x86/lib/delay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/lib/delay.c +1 -1
@@ -102,7 +102,7 @@ static void delay_mwaitx(unsigned long __loops)* Use cpu_tss as a cacheline-aligned, seldomly * accessed per-cpu variable as the monitor target. */ -__monitorx (this_cpu_ptr (&cpu_tss),0 ,0 ); +__monitorx (raw_cpu_ptr (&cpu_tss),0 ,0 );/* * AMD, like Intel, supports the EAX hint and EAX=0xf
Signature · 8 lines
2.3.5
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.tip-bot for Borislav Petkov
merge bot[tip:x86/urgent] x86/delay: Avoid preemptible context checks in delay_mwaitx()
x86/delay: Avoid preemptible context checks in delay_mwaitx()
Commit-ID: 84477336ec03f8061ffd6908da341e063e5d6d1f Gitweb: http://git.kernel.org/tip/84477336ec03f8061ffd6908da341e063e5d6d1f Author: Borislav Petkov <[email protected]> AuthorDate: Wed, 9 Mar 2016 21:56:22 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 10 Mar 2016 11:27:12 +0100
x86/delay: Avoid preemptible context checks in delay_mwaitx()
We do use this_cpu_ptr(&cpu_tss) as a cacheline-aligned, seldomly accessed per-cpu var as the MONITORX target in delay_mwaitx(). However, when called in preemptible context, this_cpu_ptr -> smp_processor_id() -> debug_smp_processor_id() fires:
BUG: using smp_processor_id() in preemptible [00000000] code: udevd/312 caller is delay_mwaitx+0x40/0xa0
But we don't care about that check - we only need cpu_tss as a MONITORX target and it doesn't really matter which CPU's var we're touching as we're going idle anyway. Fix that.
--- arch/x86/lib/delay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/lib/delay.c +1 -1
@@ -102,7 +102,7 @@ static void delay_mwaitx(unsigned long __loops)* Use cpu_tss as a cacheline-aligned, seldomly * accessed per-cpu variable as the monitor target. */ -__monitorx (this_cpu_ptr (&cpu_tss),0 ,0 ); +__monitorx (raw_cpu_ptr (&cpu_tss),0 ,0 );/* * AMD, like Intel, supports the EAX hint and EAX=0xf