Re: [patch for 2.6.30 2/2] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts

[Andrew Morton <akpm@linux-foundation.org>]

On Sun, 19 Apr 2009 22:57:24 -0400 (EDT) Len Brown <lenb@kernel.org> wrote:

> 
> 
> > +++ a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > @@ -197,9 +197,22 @@ static void do_drv_write(void *_cmd)
> >  
> >  static void drv_read(struct drv_cmd *cmd)
> >  {
> > -	cmd->val = 0;
> > +	int target_cpu;		/* The CPU on which to perform thr rdmsr() */
> > +	int this_cpu;
> > +
> > +	/*
> > +	 * If the current CPU is in cmd->mask then run the rdmsr() on this
> > +	 * CPU to avoid the cross-cpu interrupt.
> > +	 */
> > +	this_cpu = get_cpu();
> > +	if (cpu_isset(this_cpu, *(cmd->mask)))
> > +		target_cpu = this_cpu;
> > +	else
> > +		target_cpu = cpumask_any(cmd->mask);
> >  
> > -	smp_call_function_single(cpumask_any(cmd->mask), do_drv_read, cmd, 1);
> > +	cmd->val = 0;
> > +	smp_call_function_single(target_cpu, do_drv_read, cmd, 1);
> > +	put_cpu();
> >  }
> >  
> >  static void drv_write(struct drv_cmd *cmd)
> > _
> 
> Rather than this patch I would expect we would want to either:
> 
> A. as we went to the trouble to detect the local case
>    in drv_read, why call smp_call_function at all for that case?

Sure, that would work.

I felt it was a little cleaner to always delegate the call to
smp_call_function() rather than open-coding smp_call_function()'s
internal implementation details at this site.  We'd need to do:

	local_irq_disable();		/* Because this is what smp_call_function_single() does */
	do_drv_read(...);
	local_irq_enable();

> or
> 
> B. optimize smp_call_function_single to beneift all users
>    instead of just this customer.

Yep.  That would be a new smp_call_function_any() which takes a cpumask
rather than a single CPU number.  I think Rusty was cooking something
up..