[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151028003418.GA7802@nazgul.tnic>
Date: Wed, 28 Oct 2015 01:34:18 +0100
From: Borislav Petkov <bp@...e.de>
To: Huang Rui <ray.huang@....com>
Cc: Guenter Roeck <linux@...ck-us.net>,
Peter Zijlstra <peterz@...radead.org>,
Jean Delvare <jdelvare@...e.de>,
Andy Lutomirski <luto@...capital.net>,
Andreas Herrmann <herrmann.der.user@...il.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...nel.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Len Brown <lenb@...nel.org>,
John Stultz <john.stultz@...aro.org>,
Frédéric Weisbecker <fweisbec@...il.com>,
lm-sensors@...sensors.org, linux-kernel@...r.kernel.org,
x86@...nel.org,
Andreas Herrmann <herrmann.der.user@...glemail.com>,
Aravind Gopalakrishnan <Aravind.Gopalakrishnan@....com>,
Fengguang Wu <fengguang.wu@...el.com>,
Aaron Lu <aaron.lu@...el.com>, Tony Li <tony.li@....com>
Subject: Re: [PATCH v2 05/10] hwmon: (fam15h_power) Add compute unit
accumulated power
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, 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...
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists