RE: [Xen-devel][PATCH] Change cpufreq_driver->get so that it can get other cpu's real physical freq

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

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