[patch 2/8] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts by using smp_call_function_any()

[patch 2/8] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts by using smp_call_function_any()

[akpm]

From: Andrew Morton <akpm@linux-foundation.org>

Presently acpi-cpufreq will perform the MSR read on the first CPU in the
mask.  That's inefficient if that CPU differs from the current CPU. 
Because we have to perform a cross-CPU call, but we could have run the
rdmsr on the current CPU.

So switch to using the new smp_call_function_any(), whcih will perform the
call on the current CPU if that CPU is present in the mask (it is).

Cc: "Zhang, Yanmin" <yanmin_zhang@linux.intel.com>
Cc: Dave Jones <davej@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jaswinder Singh Rajput <jaswinder@kernel.org>
Cc: Len Brown <len.brown@intel.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Cc: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff -puN arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c~arch-x86-kernel-cpu-cpufreq-acpi-cpufreqc-avoid-cross-cpu-interrupts-by-using-smp_call_function_any arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c~arch-x86-kernel-cpu-cpufreq-acpi-cpufreqc-avoid-cross-cpu-interrupts-by-using-smp_call_function_any
+++ a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -192,9 +192,11 @@ static void do_drv_write(void *_cmd)
 
 static void drv_read(struct drv_cmd *cmd)
 {
+	int err;
 	cmd->val = 0;
 
-	smp_call_function_single(cpumask_any(cmd->mask), do_drv_read, cmd, 1);
+	err = smp_call_function_any(cmd->mask, do_drv_read, cmd, 1);
+	WARN_ON_ONCE(err);	/* smp_call_function_any() was buggy? */
 }
 
 static void drv_write(struct drv_cmd *cmd)
_

Re: [patch 2/8] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts by using smp_call_function_any()

[Dave Jones]

On Fri, Sep 18, 2009 at 12:41:05PM -0700, Andrew Morton wrote:

 > diff -puN arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c~arch-x86-kernel-cpu-cpufreq-acpi-cpufreqc-avoid-cross-cpu-interrupts-by-using-smp_call_function_any arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
 > --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c~arch-x86-kernel-cpu-cpufreq-acpi-cpufreqc-avoid-cross-cpu-interrupts-by-using-smp_call_function_any
 > +++ a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
 > @@ -192,9 +192,11 @@ static void do_drv_write(void *_cmd)
 >  
 >  static void drv_read(struct drv_cmd *cmd)
 >  {
 > +	int err;
 >  	cmd->val = 0;
 >  
 > -	smp_call_function_single(cpumask_any(cmd->mask), do_drv_read, cmd, 1);
 > +	err = smp_call_function_any(cmd->mask, do_drv_read, cmd, 1);
 > +	WARN_ON_ONCE(err);	/* smp_call_function_any() was buggy? */
 >  }
 >  
 >  static void drv_write(struct drv_cmd *cmd)

I'm ok with this going in, but I still wonder if we could have done it
all a lot easier, by making cpumask_any pick the current cpu if it was
in the mask instead of introducing yet another variant to an already
enormous api.

	Dave

Re: [patch 2/8] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts by using smp_call_function_any()

[Andrew Morton]

On Fri, 18 Sep 2009 15:59:53 -0400
Dave Jones <davej@redhat.com> wrote:

> On Fri, Sep 18, 2009 at 12:41:05PM -0700, Andrew Morton wrote:
> 
>  > diff -puN arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c~arch-x86-kernel-cpu-cpufreq-acpi-cpufreqc-avoid-cross-cpu-interrupts-by-using-smp_call_function_any arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
>  > --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c~arch-x86-kernel-cpu-cpufreq-acpi-cpufreqc-avoid-cross-cpu-interrupts-by-using-smp_call_function_any
>  > +++ a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
>  > @@ -192,9 +192,11 @@ static void do_drv_write(void *_cmd)
>  >  
>  >  static void drv_read(struct drv_cmd *cmd)
>  >  {
>  > +	int err;
>  >  	cmd->val = 0;
>  >  
>  > -	smp_call_function_single(cpumask_any(cmd->mask), do_drv_read, cmd, 1);
>  > +	err = smp_call_function_any(cmd->mask, do_drv_read, cmd, 1);
>  > +	WARN_ON_ONCE(err);	/* smp_call_function_any() was buggy? */
>  >  }
>  >  
>  >  static void drv_write(struct drv_cmd *cmd)
> 
> I'm ok with this going in, but I still wonder if we could have done it
> all a lot easier, by making cpumask_any pick the current cpu if it was
> in the mask instead of introducing yet another variant to an already
> enormous api.
> 

Yeah, I wondered about that.  But iirc Rusty had a good-sounding reason
for adding a new function but that reason didn't get into the changelog so
it's lost.

Re: [patch 2/8] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid cross-CPU interrupts by using smp_call_function_any()

[Rusty Russell]

On Sat, 19 Sep 2009 05:29:53 am Dave Jones wrote:
> I'm ok with this going in, but I still wonder if we could have done it
> all a lot easier, by making cpumask_any pick the current cpu if it was
> in the mask instead of introducing yet another variant to an already
> enormous api.

That was the initial proposal, but I'm uncomfortable with that, too.  "any"
really implies "doesn't matter"; if we overload it to have a preference, it
becomes really hard to tell which ones really care and which ones don't.

Then refactoring becomes harder (performance regression when you alter code).

So yeah, not ideal, but this seemed like the minimal API extension.

Cheers,
Rusty.