linux-kernel - Re: [PATCH v2 05/10] hwmon: (fam15h_power) Add compute unit accumulated power

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 28 Oct 2015 13:06:36 +0800
From:	Huang Rui <ray.huang@....com>
To:	Borislav Petkov <bp@...e.de>
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 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
--
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/