Re: [PATCH] [x86, msr]: execute on the correct CPU subset (was:Re: [PATCH] [x86, msr]: remove code duplication)

[Borislav Petkov]

Hi,

On Wed, Jul 08, 2009 at 01:19:11PM +0200, Borislav Petkov wrote:
> On Tue, Jul 07, 2009 at 08:56:00AM -0700, H. Peter Anvin wrote:
> > Borislav Petkov wrote:
> > > 
> > > Actually, the more important question is why am I executing anything on
> > > my own CPU without first checking if it is in the cpumask _at_ _all_?!
> > > /me ducks behind the sofa.
> > > 
> > > The right thing to do should be something like the following:
> > > 
> > > preempt_disable();
> > > this_cpu = raw_smp_processor_id();
> > > 
> > > if (cpumask_test_cpu(this_cpu, mask)) {
> > > 	local_irq_disable();
> > > 	msr_func(&rv);
> > > 	local_irq_enable();
> > > }
> > > 
> > > smp_call_function_many(mask, msr_func, &rv, 1);
> > > preempt_enable();
> > > 
> > 
> > I don't see why you're disabling local IRQs.
> 
> I guess I was trying to be overly careful but can't seem to think of a
> case when this would be appropriate. Hmm...
> 
> --
> From: Borislav Petkov <borislav.petkov@xxxxxxx>
> Date: Mon, 6 Jul 2009 16:08:34 +0200
> Subject: [PATCH] [x86, msr]: execute on the correct CPU subset
> 
> rdmsr_on_cpus/wrmsr_on_cpus were erroneously executing on the current
> CPU even in the case where it wasn't in the supplied bitmask. Add a
> check for that and handle accordingly.
> 
> While at it, since rdmsr_on_cpus and wrmsr_on_cpus are almost identical,
> fold them into a common __rwmsr_on_cpus helper passing a function
> pointer arg to the actual MSR operation.
> 
> Signed-off-by: Borislav Petkov <borislav.petkov@xxxxxxx>
> ---
>  arch/x86/lib/msr.c |   53 +++++++++++++++++++--------------------------------
>  1 files changed, 20 insertions(+), 33 deletions(-)

any comments on that one, NAK/AK? Since it is a fix and not changing the
interface to external users, it might be a good idea to include it in
2.6.31, IMHO, no?

-- 
Regards/Gruss,
Boris.

Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632

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