From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758698AbcFAPls (ORCPT ); Wed, 1 Jun 2016 11:41:48 -0400 Received: from mail-bn1on0081.outbound.protection.outlook.com ([157.56.110.81]:28664 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752465AbcFAPlq convert rfc822-to-8bit (ORCPT ); Wed, 1 Jun 2016 11:41:46 -0400 X-Greylist: delayed 884 seconds by postgrey-1.27 at vger.kernel.org; Wed, 01 Jun 2016 11:41:46 EDT From: "Huang, Ray" To: Guenter Roeck CC: Borislav Petkov , "linux-hwmon@vger.kernel.org" , X86 ML , LKML , "Hurwitz, Sherry" Subject: Re: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers Thread-Topic: [PATCH] hwmon: (fam15h_power) Disable preemption when reading registers Thread-Index: AQHRu+0Pzf+Hx7vl6UuWGyZeyW4SdJ/UmZSAgAAipbg= Date: Wed, 1 Jun 2016 15:26:59 +0000 Message-ID: <5FE1E570-430B-4C0D-9100-E213D5B5ACFA@amd.com> References: <1464775486-23446-1-git-send-email-bp@alien8.de>,<574EE1B3.9010704@roeck-us.net> In-Reply-To: <574EE1B3.9010704@roeck-us.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: roeck-us.net; dkim=none (message not signed) header.d=none;roeck-us.net; dmarc=none action=none header.from=amd.com; x-originating-ip: [117.41.146.210] x-ms-office365-filtering-correlation-id: 7feea371-1b78-4f09-641d-08d38a312ca8 x-microsoft-exchange-diagnostics: 1;DM2PR12MB0016;5:ArRm6kVfTFORrc6KBQdQGOYBZ1lNCcTfts2yACJlR+DtH5JZKkk8uZXFux9F9QvmOIGA3t2S/IMHKypDRrXSwDqyMTF7LbVa1ZqVjG70v0TH8BpItSdKjRw3aZAoUJRVm1w+T++Xa/KFnhFRpSTnVA==;24:F1I7d4N/A/rFPDmWddpWqDtANKMvVK0YGywuyq3eEQ+PUWJIO2sfPXnswJI0n148oixrk5/9+KdvjRc7/F6O/ouVJzjCs3w9d80mDbojvGY=;7:bFueo2pacorTYua5QCbNis9PkROfEIbNWabRHITSwyTTlJ6xGSw3nNNdd5905KvW5qbveI7ElYflvch4EyjDg2bGUcbG88WYaDeKgXdOLGHScKeii4aFED+mcl6YyFnOFMIm1BPD0CBktmSGHfySnVx4/JP4ZENltSRPlW4P0BqeHq6/fF02UQr1FyN/gkaO;20:fgQwMMp7X57dBYPLQouxzz5mRxxBf/3l4bPIX83K/62l3881AvHqhucRli854ih+EHI+pQiMg+RS1PskTfA1EjJ6tskNYfDA3dAM/Jh3PKEE8lmuqen5VrI9fBplNynFVT0Wk5aJN4JQm0Fwu1jzLcQS6klXwvjVuuKcXnXTX3scuf2TL2Bay2pwYCmynq8DDMwPVHaLrD2w5iwlVhdnqpMz4+4tY16o1OWy/ZJHJFRJJ251BG/PMwM/bPBZ/u+O x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR12MB0016; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(209352067349851)(767451399110); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:DM2PR12MB0016;BCL:0;PCL:0;RULEID:;SRVR:DM2PR12MB0016; x-forefront-prvs: 096029FF66 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(24454002)(377454003)(122556002)(8676002)(8936002)(82746002)(10400500002)(5008740100001)(33656002)(36756003)(2950100001)(4326007)(189998001)(99286002)(110136002)(106116001)(5004730100002)(2900100001)(81166006)(6116002)(54356999)(87936001)(102836003)(2906002)(86362001)(586003)(77096005)(76176999)(66066001)(50986999)(5002640100001)(92566002)(19580395003)(19580405001)(83716003)(3660700001)(3846002)(3280700002);DIR:OUT;SFP:1101;SCL:1;SRVR:DM2PR12MB0016;H:DM2PR12MB0016.namprd12.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jun 2016 15:26:59.4291 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR12MB0016 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Jun 1, 2016, at 9:23 PM, Guenter Roeck wrote: > >> On 06/01/2016 03:04 AM, Borislav Petkov wrote: >> From: Borislav Petkov >> >> We need to read a bunch of registers on each compute unit and possibly >> on the current CPU too. Disable preemption around it. > > An explanation would be helpful. Is this a bug fix ? I would like to get > a confirmation from someone at AMD that this is really necessary. > This change looks good for me. But I am in office this week, I will test it on CZ platform next week. :-) Thanks, Rui > Thanks, > Guenter > >> Signed-off-by: Borislav Petkov >> Cc: Rui Huang >> Cc: Sherry Hurwitz >> Cc: Guenter Roeck >> --- >> drivers/hwmon/fam15h_power.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c >> index eb97a9241d17..69bb810f528b 100644 >> --- a/drivers/hwmon/fam15h_power.c >> +++ b/drivers/hwmon/fam15h_power.c >> @@ -172,9 +172,9 @@ static void do_read_registers_on_cu(void *_data) >> */ >> static int read_registers(struct fam15h_power_data *data) >> { >> - int this_cpu, ret, cpu; >> int core, this_core; >> cpumask_var_t mask; >> + int ret, cpu; >> >> ret = zalloc_cpumask_var(&mask, GFP_KERNEL); >> if (!ret) >> @@ -183,7 +183,6 @@ static int read_registers(struct fam15h_power_data *data) >> memset(data->cu_on, 0, sizeof(int) * MAX_CUS); >> >> get_online_cpus(); >> - this_cpu = smp_processor_id(); >> >> /* >> * Choose the first online core of each compute unit, and then >> @@ -205,10 +204,13 @@ static int read_registers(struct fam15h_power_data *data) >> cpumask_set_cpu(cpumask_any(topology_sibling_cpumask(cpu)), mask); >> } >> >> - if (cpumask_test_cpu(this_cpu, mask)) >> + preempt_disable(); >> + smp_call_function_many(mask, do_read_registers_on_cu, data, true); >> + >> + if (cpumask_test_cpu(smp_processor_id(), mask)) >> do_read_registers_on_cu(data); >> >> - smp_call_function_many(mask, do_read_registers_on_cu, data, true); >> + preempt_enable(); >> put_online_cpus(); >> >> free_cpumask_var(mask); >