On Sun, 19 Apr 2009 22:57:24 -0400 (EDT) Len Brown <lenb@xxxxxxxxxx> 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..
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
