From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753301AbbJ0Czw (ORCPT ); Mon, 26 Oct 2015 22:55:52 -0400 Received: from mail-by2on0097.outbound.protection.outlook.com ([207.46.100.97]:64273 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751594AbbJ0Czu (ORCPT ); Mon, 26 Oct 2015 22:55:50 -0400 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=amd.com; amacapital.net; dkim=none (message not signed) header.d=none;amacapital.net; dmarc=permerror action=none header.from=amd.com; X-WSS-ID: 0NWUYSU-08-B8I-02 X-M-MSG: Date: Tue, 27 Oct 2015 10:53:40 +0800 From: Huang Rui To: Borislav Petkov CC: Guenter Roeck , Peter Zijlstra , Jean Delvare , Andy Lutomirski , Andreas Herrmann , Thomas Gleixner , Ingo Molnar , "Rafael J. Wysocki" , Len Brown , John Stultz , =?iso-8859-1?Q?Fr=E9d=E9ric?= Weisbecker , , , , Andreas Herrmann , Aravind Gopalakrishnan , Fengguang Wu , Aaron Lu , Tony Li Subject: Re: [PATCH v2 05/10] hwmon: (fam15h_power) Add compute unit accumulated power Message-ID: <20151027025339.GF8036@hr-amur2> References: <1445308109-17970-1-git-send-email-ray.huang@amd.com> <1445308109-17970-6-git-send-email-ray.huang@amd.com> <20151023132702.GC2844@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20151023132702.GC2844@pd.tnic> User-Agent: Mutt/1.5.21 (2010-09-15) X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(428002)(199003)(189002)(164054003)(24454002)(46406003)(86362001)(87936001)(5007970100001)(83506001)(33716001)(97736004)(92566002)(106466001)(189998001)(11100500001)(105586002)(4001350100001)(110136002)(2950100001)(77096005)(19580395003)(97756001)(47776003)(33656002)(19580405001)(23726002)(50986999)(50466002)(101416001)(54356999)(76176999)(5008740100001)(107986001);DIR:OUT;SFP:1101;SCL:1;SRVR:DM3PR12MB0857;H:atltwp02.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0857;2:gIKVAP06KN22LFuqMN2zketwfqZFt0lCQuIvTbOePPupNZpznew1jVlAY67FhchyOFrdJs5J6wMZ9VMzyp3HunGM3g84J0/DOCN3v4X21nqj47How+vxOF3tajjOoZ705jLILfzRVI7d2Ch+jr5Pg0ZClgbVOWOoDx9sdSlzdyo=;3:mfuqsTQGMvIJUSsPTwvOzL3t05SoYWZODeQvqSDPVadBYkycBBPv6XESn4UI1XGdL/+C6vjnVM4XlFokUGrPm4xYix6GuyYn3ki1qn9zkw6hzO1uIVu+LX3xrnMw1ImxXhDHYYTAE5OJn2f2/sNOslaVeFWOD7K6pnmsYhWNDAkAaeBPcgagayFNn9o/NHgL3OBZ3A1nMoQzntMyivEmcAPPVUgDPhlEaOpSmNROJ0JECSAntshBpShf7azZU/tZ;25:TkUh6CRNfWnqaYJEKTiRBXupWf+nDR/zvqhdICG2XvcpVoIjukdsHfm+QEQfarDTjGufVcttRIfAw7stjd10fOCabIZWAyT9WSpASdCwAJupyJ+VWV+Rw/UYEVZ/Eo6iH7uIveVFufUXfkG9cvBEuvM1fdRAW8BV3fakA6SFLjDPA9wKnhiq9nCz3xCKol/cciipZfoNadd0PcNhDYiEkVvLC8aAu/wGeBuIvchOtBmguKuXqV7IeeUqekKuBraF X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM3PR12MB0857; X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0857;20:ddYhQsbu412Svq8dEk4QTtLJ7NPXuF4cKaZHQpbC1iZoy4hHKmSv1E7EnPMubLsEB1HzBVC9sg707nq0t92JZ/tPv80XFojnaP+tfbVTv+csq48wI9CCfXPvPmDxTcWKmTm4CumTuvjLas16iKubNKppOnr4fGygYdFN8WmfzwgrKIpJWXm9ADd4RodtoOPVE3FAABZH04D5DMf4VtCcv4w6DZstLyBSwxATPITnlcns7cwoY+pT6qM8zu6nQj8OcLYk0ynBtPlh8yw6cHZef820Y1aOkj2mjqwXnyyRimOZ8rIHnZ6fgh5sU1f67RCqCP4n95Yt4WYvflj07w/7nvjPau4U1OvijjcG16mRWw6EdXOHQ5obcP1AoKh4VGnr4iuWDlvGapFACMljP3duYmimxKucYTP1UN/QrzZT3aNaEK8LcbkEYo0Nz+cTKPB81IPvs2yBTwaky+4roX0rNHBOCshV26AlyoGwSwc/d6rndQcKlFMjzKZhZ88GH4i3;4:csm/ualT1x1Xh7hQ3exT4AKQ+n2EXuuWjXd3pRJjeWyX7MhjyNw+Lu64Z4AoKw7xOrJZKwDc+mJVJvs8ch/BjO0xT4wPWKAGMMqky+WpSL4JifSFdkXKaqDfsyMaF/PfLc5brNVX3qivFT5mHl/ZMO6SnOfluBc7BMYUrz+paLK63xftb/8XRS0Ocb32hyGVC3K/FYeOTrq8cs8y8AdWChRg5Ga0oJB665sUl4RPezt0gyEz6lJDpLhhM/9FBp6i2vA/y6jcut4TGkKknz7SwH/olKDJ2tBPXOHXzMw+NprPb5US18dcqNbAuRdD3PckqJ3RnG0LOSvT2gOme0yo6/QbhrbQFnVRhd5ycTTRtX6RW/83oLCuoQ6HY7ogWT5g X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(520078)(8121501046)(3002001)(102215026);SRVR:DM3PR12MB0857;BCL:0;PCL:0;RULEID:;SRVR:DM3PR12MB0857; X-Forefront-PRVS: 0742443479 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DM3PR12MB0857;23:P8rOjqtszTQWyE0qsqzT6Z9dYyioRTIh6vDYgIP0L?= =?us-ascii?Q?xYL/aE1klpSAXElaYC37ETIvEe5xT8mkUgOsqMjt5mkINKIF3nGvNpHwdGNi?= =?us-ascii?Q?FzCBgy1DfBp1DmfRY8MQiGpsEoIzZoSlkir8EENUdJCZpxCOq9C5dSQenCCi?= =?us-ascii?Q?qDqMcI5314TuYzKxtcHT2yz9/wcS6lsqfSRLd/Ox9XW6Gp1mecU5sBtQrrIi?= =?us-ascii?Q?8B7B5FgoVjTRzDkVXP20SNy/OhdI4elE04oEf4EWW6F4WIRkEgvsUm/PO6Eu?= =?us-ascii?Q?H4gKF9vVDzKwhtmiNhXxM3UxhhQYaBCOT9nNorJH8C+8MrnLaCiSP9XVXD/i?= =?us-ascii?Q?KEJdfL7Mh9BLEgZT3jwldSOhPg/OhSwP4K4Sjre0P6C9s93ye8CVaVxbbWIh?= =?us-ascii?Q?MXrP3byXHGgNUJkxZWXv7DQY1+zaTMFypTi2PtokcVoBTxRtJ61dIJfOxAUP?= =?us-ascii?Q?ILa1q4IggrqZaX53wC8Z+4VHjAzYJbcyqv01i9IlsPOKYttO6br1P681RyU+?= =?us-ascii?Q?zm3j0fvQAoMHM6sPago0dfBxkjDAh6mR//DvFonSOEePh+p36UzgXpGoKIw+?= =?us-ascii?Q?IswBdklqzxpWXkwvHcqSBednJatnjymqBbXV30owsgoJ5jpEIqxUnvflY2zJ?= =?us-ascii?Q?bIKSbAXddmy/G1uU/+ZJFxOgm4FuD92DAwlakLExzVJ+HKjdTQlsKjQE7xiP?= =?us-ascii?Q?WA68n8+OprTqoU6+IED3mtKm29e1GsOm/iD2X3qF9h1u2RrSl2q0NMGJLgZv?= =?us-ascii?Q?buXd/T/85nxVKqK++1Jq+ns8r4pb/6HqrfVOkD1EfqUjs9HZZY/Qs6BrykNP?= =?us-ascii?Q?rn6XqyYL1oP4QDM7GIOzS4KWCME8lkVPojUYpvfQ+h/MiWP3OQHLN7O9et+P?= =?us-ascii?Q?TNiO0DQyd3heUG8y4wRBbcKCwmCp+dxnDT3UqNtxRtap9Xbj42abu8sMS/X8?= =?us-ascii?Q?AdvsuB5u229B2XTtka7ccN9kKx3J0KgJyV3N6zM7Q+CYC/HO+QeXnw0Y3lKD?= =?us-ascii?Q?mc=3D?= X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0857;5:dPeWMLxm569YQXrYBmL/6MuuJm2UOSSD+pmL7VLx4NRUW0KYbzGZMRXTnU3qrsXQdz/WKUQHWyw+6bSaPVoTLnFaKtCSVij8aRxkQ5H38kYQWyV6xAHBIccrsIOd/nuVzw9vi9Dgz0w+Vyh4n/aEwQ==;24:GqaOLX75Msxiz/80P2iO1erXfdvpQP+BcxSe5KVPXLvD3rHBNSmjsevri2hfGmvmC8e3NTLVfOAg0kycj2EIpycPUT8Wu5jZ1wRIcKJMmpw=;20:ng90Kot0Xj4XbC7Erb6li5awRgpwjZbmch6iKG2fysNvYYdcaluFw+PvY4psvEJRbddlJ3I72PtAWDrQLSF/9Tb11EFBEmpaue7cPk8V2TLMAdo12DscC89PewGqBcJH1giVT91NGfR+AMg449yeIqzcEiYvvoTRZdZkkbgVRs1aWx+Rn+Xl25qfBLzq3Dl9Ob9IkAiQbPVLFbkQwlqdeSGqjvcu+Ngh8R+TXUP1NUysqyqFpK3QzcS/ljmgBv/M SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Oct 2015 02:55:45.8826 (UTC) X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.222];Helo=[atltwp02.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR12MB0857 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 23, 2015 at 03:27:02PM +0200, Borislav Petkov wrote: > On Tue, Oct 20, 2015 at 10:28:24AM +0800, Huang Rui wrote: > > This patch adds a member in fam15h_power_data which specifies the > > compute unit accumulated power. It adds do_read_registers_on_cu to do > > all the read to all MSRs and run it on one of the online cores on each > > compute unit with smp_call_function_many(). This behavior can decrease > > IPI numbers. > > > > Suggested-by: Borislav Petkov > > Signed-off-by: Huang Rui > > Cc: Guenter Roeck > > Cc: Peter Zijlstra > > Cc: Ingo Molnar > > --- > > drivers/hwmon/fam15h_power.c | 68 +++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 67 insertions(+), 1 deletion(-) > > > > + > > +static int read_registers(struct fam15h_power_data *data) > > +{ > > + int this_cpu, ret; > > + int cu_num, cores_per_cu, cpu, cu; > > + cpumask_var_t mask; > > + > > + cores_per_cu = amd_get_cores_per_cu(); > > + cu_num = boot_cpu_data.x86_max_cores / cores_per_cu; > > + > > + WARN_ON_ONCE(cu_num > MAX_CUS); > > + > > + ret = zalloc_cpumask_var(&mask, GFP_KERNEL); > > + if (!ret) > > + return -ENOMEM; > > + > > + this_cpu = get_cpu(); > > This should be get_online_cpus() and its counterpart below should be > put_online_cpus(). > 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? > > + > > + /* > > + * Choose the first online core of each compute unit, and then > > + * read their MSR value of power and ptsc in one time of IPI, > > in a single IPI. > > > + * because the MSR value of cpu core represent the compute > > s/cpu/CPU/ > > do that in *all* your text. > > > + * unit's. This behavior can decrease IPI numbers between the > > unit's ? > > What does that sentence even mean? > That means "the value(cu_acc_power) of the compute unit", which does not represent the value of one CPU core. > > + * cores. > > + */ > > + cpu = cpumask_first(cpu_online_mask); > > + cu = cpu / cores_per_cu; > > + while (cpu < boot_cpu_data.x86_max_cores) { > > + if (cu <= cpu / cores_per_cu) { > > + cu = cpu / cores_per_cu + 1; > > + cpumask_set_cpu(cpu, mask); > > + } > > + cpu = cpumask_next(cu * cores_per_cu - 1, cpu_online_mask); > > + } > > This is hard to parse - I *think* you're setting a bit in mask for a > core in each CU... > Yes, that's right. My codes' behavior is below: Assumed cores_per_cu is 4 and cu_number is 6, and the online cpumask is: cu5 cu4 cu3 cu2 cu1 cu0 1000_1100_0110_1011_0000_1111 After setting bits of the mask: 1000_0100_0010_0001_0000_0001 on on on on off on > If so, I think you can simplify it by generating a tmp mask which > contains the cores of CU0, i.e. something like that: > > 11_00_00_... > > and then do cpumask_and(res, ...) to find the online cores on that CU > and then do cpumask_set_cpu(cpumask_any(res), mask) to select one CPU on > that CU. > > And then shift to the next CU: > > cpumask_shift_right(dst, src_mask, cores_per_cu); > > I think this should be cleaner and less error prone, without the > conditionals... > 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); } --- Thanks, Rui