Re: [PATCH v5 2/6] hwmon: (fam15h_power) Add compute unit accumulated power

[Huang Rui <ray.huang@amd.com>]

On Mon, Mar 28, 2016 at 11:29:52AM +0200, Borislav Petkov wrote:
> On Mon, Mar 28, 2016 at 01:32:12PM +0800, Huang Rui wrote:
> > +
> > +	get_online_cpus();
> > +	this_cpu = get_cpu();
> 
> What now?
> 
> get_online_cpus() is enough.
> 

Will remove get_cpu().

> > +
> > +	/*
> > +	 * Choose the first online core of each compute unit, and then
> > +	 * read their MSR value of power and ptsc in a single IPI,
> > +	 * because the MSR value of CPU core represent the compute
> > +	 * unit's.
> > +	 */
> > +	for_each_online_cpu(cpu) {
> > +		target = cpumask_first(topology_sibling_cpumask(cpu));
> > +		if (!cpumask_test_cpu(target, mask))
> > +			cpumask_set_cpu(target, mask);
> > +	}
> 
> I think you want something like this: iterate over each core and put one
> of them into the mask.
> 
> 	core = -1;
> 
> 	for_each_online_cpu(cpu) {
> 		this_core = topology_core_id(cpu);
> 
> 		if (this_core == core)
> 			continue;
> 
> 		core = this_core;
> 
> 		/* get any CPU on this compute unit */
> 		cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask);
> 	}
> 

Yep, with new x86 topology for core on AMD, using this way should be
more clear.

Thanks,
Rui