lkml.org 
[lkml]   [2015]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 05/10] hwmon: (fam15h_power) Add compute unit accumulated power
On Wed, Oct 28, 2015 at 01:34:18AM +0100, Borislav Petkov wrote:
> On Tue, Oct 27, 2015 at 10:53:40AM +0800, Huang Rui wrote:
> > Preemption must be disabled when calling smp_call_function_many,
> > get_cpu would did that. Will get_online_cpus have the same behavior
> > like that?
>
> Well, get_online_cpus() protects you against CPU hotplug operations in
> general. If you want to protect yourself against CPUs going away only,
> then I guess get_cpu()/put_cpu() is fine.
>
> But since we're going to work with the masks as below, prohibiting any
> changes to cpu_online_mask is probably the better/safe thing to do, i.e.
>
> get_online_cpus();
> preempt_disable();
>
> smp_call_function_many( ... );
>
> preempt_enable();
> put_online_cpus();
>
> > That means "the value(cu_acc_power) of the compute unit", which does
> > not represent the value of one CPU core.
>
> No, I mean this: "This behavior can decrease IPI numbers between the
> unit's."
>
> I'm wondering whether it is really needed at all ...
>

OK, The real words are "This behavior can decrease IPI numbers between
the cores." Actually, the meaning also can be reflected from the
codes. So I could remove this sentence.

> > OK, how about below codes:
> >
> > ---
> > for (i = 0; i <= cores_per_cu / BITS_PER_LONG; i++) {
> > offset = cores_per_cu % BITS_PER_LONG;
> > if (i == cores_per_cu / BITS_PER_LONG) {
> > cpumask_bits(src_mask)[i] = GENMASK(offset -1, 0);
> > break;
> > }
> > cpumask_bits(src_mask)[i] = GENMASK(BITS_PER_LONG - 1, 0);
> > }
> >
> > for (i = 0; i < cu_num; i++) {
> > cpumask_shift_left(dst, src_mask, cores_per_cu * i);
> > cpumask_and(res, dst, cpu_online_mask);
> > cpumask_set_cpu(cpumask_any(res), mask);
> > }
>
> I think you can make it even simpler:
>
> /* prepare CU temp mask */
> for (i = 0; i < cores_per_cu; i++)
> cpumask_set_cpu(i, tmp_mask);
>
> for (i = 0; i < cu_num; i++) {
> /* WARN_ON for empty CU masks */
> WARN_ON(!cpumask_and(res_mask, tmp_mask, cpu_online_mask));
> cpumask_set(cpumask_any(res_mask), call_mask);
> cpumask_shift_right(tmp_mask, tmp_mask, cores_per_cu);
> }
>
> smp_call_function_many(call_mask, .... );
>
> Something like that...
>

Looks better. :)

Thanks,
Rui


\
 
 \ /
  Last update: 2015-10-28 06:21    [W:0.345 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site