Tian, Kevin wrote:
>> From: Liu, Jinsong
>> Sent: Thursday, December 11, 2008 10:42 AM
>>
>> Change cpufreq_driver->get so that it can get other cpu's real
>> physical freq.
>>
>> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
>
> @@ -174,7 +177,11 @@ static void drv_read(struct drv_cmd *cmd
> {
> cmd->val = 0;
>
> - do_drv_read(cmd);
> + /* to reduce IPI for the sake of performance */
> + if (first_cpu(cmd->mask) == smp_processor_id())
> + do_drv_read((void *)cmd);
> + else
> + on_selected_cpus( cmd->mask, do_drv_read, (void *)cmd, 0, 1);
> }
>
> Above is a bad change where you kicks multiple cpus to update
> same variable without any coordination. Also why can short path only
> be used with strict condition that current cpu must be first bit in
> mask? As long as current cpu is on mask, you can always read
> directly.
No need any coordination here, please notice under whatever situation, there is
only 1 bit set at cpumask here (we can only read the value of 1 cpu 1 time).
The first bit of cpumask here is not to set strict condition for short path. In
fact, becuase of only 1 bit set at cpumask, the first_cpu() is only used to get
its cpuid. If cpuid is the running cpu, go short path, otherwise go longer path.
In fact, get_cur_val() was called by 2 paths (driver path and check_freq path),
we design here is used to provide a unified interface to satisfied the
requirement of both paths.
>
> @@ -205,7 +220,7 @@ static u32 get_cur_val(cpumask_t mask)
> return 0;
> }
>
> - cmd.mask = mask;
> + cmd.mask = cpumask_of_cpu(cpu);
>
> drv_read(&cmd);
> return cmd.val;
>
> Since do_drv_read can figure out which cpu to read from the mask,
> why do you need above limitation then?
do_drv_read get the msr value of the cpu which it runs upon.
the cpumask limitation here is used to let drv_read() only read 1 cpu msr
value, otherwise it will be confused, on_selected_cpu will send IPI to multi
cpus and read msr value will mistake.
Thanks,
Jinsong
>
> Thanks,
> Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|