[PATCH] x86: Clean up mwait_play_dead

[PATCH] x86: Clean up mwait_play_dead

[Frank Arnold]

From: Frank Arnold <frank.arnold@amd.com>

Commit 93789b32dbf355e70f18b17a82e8661677a7f7fb introduced
cpuinfo_x86 *c for brevity. It is used to check for the
mwait feature flag. The same can be done with a subsequent
check for clflsh and save us a second per-cpu pointer
dereference.

Signed-off-by: Frank Arnold <frank.arnold@amd.com>
Acked-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/smpboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index c2871d3..93a0066 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1334,7 +1334,7 @@ static inline void mwait_play_dead(void)
 
 	if (!(cpu_has(c, X86_FEATURE_MWAIT) && mwait_usable(c)))
 		return;
-	if (!cpu_has(__this_cpu_ptr(&cpu_info), X86_FEATURE_CLFLSH))
+	if (!cpu_has(c, X86_FEATURE_CLFLSH))
 		return;
 	if (__this_cpu_read(cpu_info.cpuid_level) < CPUID_MWAIT_LEAF)
 		return;
-- 
1.7.3.4

Re: [PATCH] x86: Clean up mwait_play_dead

[H. Peter Anvin]

On 04/13/2011 09:11 AM, Frank Arnold wrote:
> From: Frank Arnold <frank.arnold@amd.com>
> 
> Commit 93789b32dbf355e70f18b17a82e8661677a7f7fb introduced
> cpuinfo_x86 *c for brevity. It is used to check for the
> mwait feature flag. The same can be done with a subsequent
> check for clflsh and save us a second per-cpu pointer
> dereference.
> 
> Signed-off-by: Frank Arnold <frank.arnold@amd.com>
> Acked-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  arch/x86/kernel/smpboot.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 

This should be using this_cpu_has()... if that's not in the tree yet we
should add it.

Creating an absolute pointer for a percpu references is a big lose.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Clean up mwait_play_dead

[Borislav Petkov]

On Wed, Apr 13, 2011 at 09:52:40AM -0700, H. Peter Anvin wrote:
> On 04/13/2011 09:11 AM, Frank Arnold wrote:
> > From: Frank Arnold <frank.arnold@amd.com>
> > 
> > Commit 93789b32dbf355e70f18b17a82e8661677a7f7fb introduced
> > cpuinfo_x86 *c for brevity. It is used to check for the
> > mwait feature flag. The same can be done with a subsequent
> > check for clflsh and save us a second per-cpu pointer
> > dereference.
> > 
> > Signed-off-by: Frank Arnold <frank.arnold@amd.com>
> > Acked-by: Borislav Petkov <borislav.petkov@amd.com>
> > ---
> >  arch/x86/kernel/smpboot.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> 
> This should be using this_cpu_has()... if that's not in the tree yet we
> should add it.

Looks like it is still in tip/x86/mm.

We could either redo the patch properly when this_cpu_has() is merged or
add the change against that branch.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] x86: Clean up mwait_play_dead

[H. Peter Anvin]

On 04/13/2011 10:36 AM, Borislav Petkov wrote:
>>
>> This should be using this_cpu_has()... if that's not in the tree yet we
>> should add it.
> 
> Looks like it is still in tip/x86/mm.
> 
> We could either redo the patch properly when this_cpu_has() is merged or
> add the change against that branch.
> 

It really should probably have been in tip:x86/cpu not x86/mm, but since
this is so simple we can just add it to the branch.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86: Clean up mwait_play_dead

[Ingo Molnar]

* Borislav Petkov <bp@amd64.org> wrote:

> On Wed, Apr 13, 2011 at 09:52:40AM -0700, H. Peter Anvin wrote:
> > On 04/13/2011 09:11 AM, Frank Arnold wrote:
> > > From: Frank Arnold <frank.arnold@amd.com>
> > > 
> > > Commit 93789b32dbf355e70f18b17a82e8661677a7f7fb introduced
> > > cpuinfo_x86 *c for brevity. It is used to check for the
> > > mwait feature flag. The same can be done with a subsequent
> > > check for clflsh and save us a second per-cpu pointer
> > > dereference.
> > > 
> > > Signed-off-by: Frank Arnold <frank.arnold@amd.com>
> > > Acked-by: Borislav Petkov <borislav.petkov@amd.com>
> > > ---
> > >  arch/x86/kernel/smpboot.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > 
> > This should be using this_cpu_has()... if that's not in the tree yet we
> > should add it.
> 
> Looks like it is still in tip/x86/mm.

Yeah - that patch was sent at around the beginning of the merge window, so it 
was too late for .39.

> We could either redo the patch properly when this_cpu_has() is merged or add 
> the change against that branch.

Yeah - please do one against tip:master and we'll put it into the right topic 
branch.

Thanks,

	Ingo

Re: [PATCH] x86: Clean up mwait_play_dead

[Frank Arnold]

On Wed, 2011-04-13 at 13:49 -0400, Ingo Molnar wrote:
> * Borislav Petkov <bp@amd64.org> wrote:
> 
> > On Wed, Apr 13, 2011 at 09:52:40AM -0700, H. Peter Anvin wrote:
> > > On 04/13/2011 09:11 AM, Frank Arnold wrote:
> > > > From: Frank Arnold <frank.arnold@amd.com>
> > > > 
> > > > Commit 93789b32dbf355e70f18b17a82e8661677a7f7fb introduced
> > > > cpuinfo_x86 *c for brevity. It is used to check for the
> > > > mwait feature flag. The same can be done with a subsequent
> > > > check for clflsh and save us a second per-cpu pointer
> > > > dereference.
> > > > 
> > > > Signed-off-by: Frank Arnold <frank.arnold@amd.com>
> > > > Acked-by: Borislav Petkov <borislav.petkov@amd.com>
> > > > ---
> > > >  arch/x86/kernel/smpboot.c |    2 +-
> > > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > > 
> > > 
> > > This should be using this_cpu_has()... if that's not in the tree yet we
> > > should add it.
> > 
> > Looks like it is still in tip/x86/mm.
> 
> Yeah - that patch was sent at around the beginning of the merge window, so it 
> was too late for .39.
> 
> > We could either redo the patch properly when this_cpu_has() is merged or add 
> > the change against that branch.
> 
> Yeah - please do one against tip:master and we'll put it into the right topic 
> branch.

It's already fixed in tip:master with the proper use of this_cpu_has().
commit 349c004e3d31fda23ad225b61861be38047fff16

Sorry for the noise.

-- 
Frank.

Advanced Micro Devices GmbH
Sitz: Dornach, Gemeinde Aschheim,
Landkreis München Registergericht München,
HRB Nr. 43632 - WEEE Registrierungsnummer 129 19551
Geschäftsführer: Alberto Bozzo, Andrew Bowd