From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752867AbbJZDrU (ORCPT ); Sun, 25 Oct 2015 23:47:20 -0400 Received: from mail-bn1on0098.outbound.protection.outlook.com ([157.56.110.98]:21991 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752505AbbJZDrS convert rfc822-to-8bit (ORCPT ); Sun, 25 Oct 2015 23:47:18 -0400 Authentication-Results: spf=none (sender IP is 165.204.84.221) 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: 0NWT6IN-07-5TN-02 X-M-MSG: Date: Mon, 26 Oct 2015 11:46:16 +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 07/10] hwmon: (fam15h_power) Introduce a cpu accumulated power reporting algorithm Message-ID: <20151026034615.GE8036@hr-amur2> References: <1445308109-17970-1-git-send-email-ray.huang@amd.com> <1445308109-17970-8-git-send-email-ray.huang@amd.com> <20151023132802.GD2844@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset="gb2312" Content-Disposition: inline In-Reply-To: <20151023132802.GD2844@pd.tnic> User-Agent: Mutt/1.5.21 (2010-09-15) Content-Transfer-Encoding: 8BIT X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.221;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(428002)(24454002)(43544003)(164054003)(199003)(189002)(87936001)(50986999)(47136003)(76176999)(54356999)(19580405001)(19580395003)(50466002)(83506001)(23696002)(92566002)(101416001)(2950100001)(5008740100001)(110136002)(5001920100001)(97736004)(4001350100001)(5007970100001)(33656002)(48336001)(86362001)(33716001)(11100500001)(106466001)(77096005)(189998001)(105586002)(47776003)(107986001);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR12MB0718;H:atltwp01.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0718;2:zZtPf1ESdCNijP0J51gsOmhECGlJvl/h7ckWIU05Z1b3mbfltIQMQtDygCXo7hzsUKiHeiVrh046gpAQEX5/OHrzx3gg4CF0JjzKH+bhN70QXGaDct+dan9lmdNPgwl6B40JTa42E91dhFVqjDeSSz8SD4Od9nrUPyI8j3Sgmgc=;3:pM9ewxmZDUTjXXlGuKChyYEEx2fmbJ3RPXj98x5VHi+zSmFoWZt4FsEXwZSQ7YBxjilDriwusLGAHnET0rKH0EoGwBP8XSL6nw43EmdtDpCnv6jnseeAykZye3Bz85emnwTrlMES/7LoWhZ0QlRaE8TZrRslxjvf3/M0K3cCKhslOJLswRZV9iI1w2s8CZpByg3ctCewoDCC9JbLLWSDo1ThYRVGgJKtFFFQzt0VkO1rCPm/z6zv/ncZCzZ7smHO;25:DdpdOWNjr07fDHlW6kZa1rlI81RIS41acyJGqtQMRcmgphPJLV7wuMgC9i9Pi3jr9lZdqL8Qo48758BdEugxvyvpObZ99kHkVuJ2B8FMqnYQkxSJeAwOz9u599E7Fy7V1TGsOnRgE2fTcbkPPvhfkZZA/E9tmNmT+XO34OxlQU2BnLIdJ86xCPcnd0nHyoqZRXohAAAiS7xrKH/hkoVBgE0zxEJKgDx6gzYKzSqFa5M9uOfrZBFb40bLXU3nO2hh8m0E0WVVJ29Bj48KHvtAgw== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0718; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0718;20:S6W9+YJVgCAiMJ+UB63C6EQKboyWXohCFxBQXPGoac/rBL/V5OTM1kLu5ZtWPxBQPl/cE6HJk509EG/V32jbnlJWjQxC5x5CPDCU5Tu7nsuYiR40lUCXNt4jcMvXM+O4Sb3C34H3XDRzKY8hCM20nFjcdLYQJ0DCBK2L6uYkTU8dFRmEnc0UwzVEPp5PqMCAUNtGNyXCeOXMvvoVcKzGKfC6uX97E0fC7KYPTXWL4Xmdckqc7a+xRFBGPITXejgAlFHuQvdhep/HZOtibmY1lnxxOYcVJTfmKEGZR3YaDcXdnAOCNfcNuH88CfOcgxSZh4kDxCgUXwOPG29GjqlY2Yfx/4uUAi4mcIuFJwG+ohe8ImUL//hNGflTpOk2ChqeyR/NnInneJJmhPOR3xW6/jKUdAmNq408+a4RivHPxNX4xy+k0JRCya1zoVCRzrhFF0nJvNvVe5XkaPCDWsyaPnJGa5ABjmv0Z4ajANcFffyuVJxdsrv77DkV3qLid26z;4:G3YaicI3+o8XKoVUnB/5Re446UVJWub2xeiS5s5/YmqyIBFc3qZBRueNH0JdHiVjOyTCSd5kCw7l70pb6qAJPOftEYEx9KjPx/BMG6Pj7DBFd6BnvUyR1Uq1LEQ44wxCsazRUUsqu5UZSPJS2LEnAk4kvu193E9T5JFjCGaVcIfm63vZlw7Xnl8viec2v/WouGbW/NHZ0gKivAcFPe9deJbtDROJ7Yfuo8ofWRYdGe8YxJCzjw7ZaEFTlOf5LVzT3+Eatgs7UpOU2JHcTP9ZgiXkMZvbA0xmemxll5yTzcFlIrlO8f/zEungZasVLGiUi90LMnWpPZ/fYafVxLOUYIBfzIvIUF2WJu4cmJ/8i75eK08n/iq56Aue3yfBgUuf X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(5005006)(8121501046)(3002001)(102215026);SRVR:SN1PR12MB0718;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0718; X-Forefront-PRVS: 0741C77572 X-Microsoft-Exchange-Diagnostics: =?gb2312?B?MTtTTjFQUjEyTUIwNzE4OzIzOmh1d0F3V1kvYXJLSmZTWFg0UXNZbUhxUDVE?= =?gb2312?B?Wm1xNDN2NmVzV0xQMnBMTHdjSzB5VUNZTWk5N0svVVJaVTRRYkF5UFpKeWFt?= =?gb2312?B?UURoVzRRdFlBV012Ky9UV1IvMGZVZEE3Rnp4NUVxRlNRNHRPQW5KLzRCQ3RZ?= =?gb2312?B?Q2FRbjY2eWs5R2kwb3JySE5QM1NXTGRra093UEF1WVVaanR6ZVZ4ZUpaNkdQ?= =?gb2312?B?VzhoNWZyVXpnamtNaEw4ZmI5eHFtWUc3b3pYQ0RGZ1UybXFqRjRBNHZoMUZs?= =?gb2312?B?NXBLUkUzdlhmeEg4NURyZm00bmdKTUhGZExkYlQyeE5PTDB1ZENYZGJkeTdT?= =?gb2312?B?RXBibGNwcTRScmNYWHVDcEdPaUdFUXFBN0Y5QWxLVVBjcnNPVFdrV1BVWEE0?= =?gb2312?B?RmlMMk1QaCt0QUpCbFFEUTM0RTVXTkxnU2p1bFBCZExXZDduaWVrd1RsY0Qz?= =?gb2312?B?S0dJbWxiUDM1YWlRR21seVVDUUZzK2ZLOVpuSmplQkh1Y0RoZTJGbTdOaFZk?= =?gb2312?B?RDVQSXhIOTlUbWdkVnJRNWthMFBDdDVyMm4rMTFXM3VNTXdScDAxaTFOUUFB?= =?gb2312?B?eXdBang5S2t6dTVGdFhKblRDY3FtTGphU3VYOTk1bWFnM3F0Q0ZrWjRyK2p6?= =?gb2312?B?YUtWVzFNajJFUEdXR3Fmam01V3htNUtUcHVVbXNPQTBaMi9oT0JOTm5CNlg5?= =?gb2312?B?OXNGYkcwUEtXVy9selM3a2N2SVlWUTd5YjBTWFV2c0Y0cWEwQTJCSEdDQWs2?= =?gb2312?B?VnVSNGtNWmRYYUF6WnJyVHRGcUtuc2dXZGpGOHQ4Tk9TY3E2TitGSHhjOUlH?= =?gb2312?B?S09EVmFEWGxXQ01OSExxZ3hNUnVxRk5FQjk0TkJFY0xOY3dkbnNibkpnZ1Mw?= =?gb2312?B?ZnI1Y1JKYXZ0UllwVzFWWk5nemRPcklKclA0UkF2TW1lRUJYem9IQnhmTi9s?= =?gb2312?B?TXhNL1RBdCt5dXJBMXFhYkhoYnE2VVFLR1hCQlB6c0E4Zmo0WVRIcmpEblUy?= =?gb2312?B?K0lNYXVpbE1IOTMrbTk4a0ZmTlQvdEM4NlJwU1pwU1BvK3k1OHk2b2Jhcjdh?= =?gb2312?B?M2g5T29rUnpWYThLdDZXOExnZFJsejlzdUxTSDRrZkl3cE1BYSt4YTVLZVA2?= =?gb2312?B?UW9wSUl4WkFQWHhsMmRydytMa3lKb3RteFpJOXRJZU1FVG5tQ1RteFdnd0hs?= =?gb2312?B?NkxqeEhZNWhpM094TEFwZGZoZUM2WFdnbmFraUcwRFZTWjgxd2Q5YWJ6ZXZR?= =?gb2312?B?RDFkb2JRcit0aWh6amh6dkRuOVhycDgrc0hmS1hLdnpabFVqTjE1aTV4OEps?= =?gb2312?B?bEpoUnk3bzZRVVVjdzRMa3JUbTB1ZGk3Y0w2WGNodWk5eUxHcFd0UDlTNkVE?= =?gb2312?B?RjBXck9Sd0U1UUcwUjlPR2VEZ2Qrb0t0djI5Ky9BPT0=?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0718;5:GaL7d+7DQbQAwD4NF34P/l49ZcSg/0uJVD/cx4Wna4QK3Tn/bRRMFPzwtXxQ65dKtpSsZpE/IQ6HLi/XLCJxAQrPCVeM2eUlrXZw8MV2OJX+ji/kk4OH2zAbxUfD9gcSNo+j+sUbKyeBq34co3dHKg==;24:qK2DgcnhBftLbXEpS72Zx4tndUqdStX0JqUd7yBuPxuG5XZTXgldZJJm1bCleZaVnHzL/MIYTI4hWmmOwjBK9XbzS0gEHOezHwYMbx2OvhU=;20:jhc4STRW96jdhtQ+CZmuitS33Fzvv9rfqGE5muBgwX04OtDJiuc+tk2RN6fZj/hJfeNNQLHsKPrV8ZT0/BbTsGmaCOqRcMUd2kH96x7F19Elxt7+KTA6jLcTKdB+juKTbHTXgnlhrDKkBElYfOlkI45qQ7lI82a/ZXMvfvBEfbbhWZLQ26I2DY2mmnErEKYsh25T7+D6+3uTBqUmV6KKhBjm7XyOYZJILFFT/M5B7TLACvGS1mh1Upf3TqCNYjgi SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2015 03:47:12.3525 (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.221];Helo=[atltwp01.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB0718 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 23, 2015 at 03:28:02PM +0200, Borislav Petkov wrote: > On Tue, Oct 20, 2015 at 10:28:26AM +0800, Huang Rui wrote: > > This patch introduces an algorithm that computes the average power by > > reading a delta value of ¡°core power accumulator¡± register during > > measurement interval, and then dividing delta value by the length of > > the time interval. > > > > User is able to use power1_average entry to measure the processor power > > consumption and power1_average_interval entry to set the interval. > > > > A simple example: > > > > ray@hr-ub:~/tip$ sensors > > fam15h_power-pci-00c4 > > Adapter: PCI adapter > > power1: 23.73 mW (avg = 634.63 mW, interval = 0.01 s) > > (crit = 15.00 W) > > > > ... > > I need to play with this more after I get back from KS. Just a partial > review for now. > Sounds good. > > > > The result is current average processor power consumption in 10 > > millisecond. The unit of the result is uWatt. > > > > Suggested-by: Guenter Roeck > > Signed-off-by: Huang Rui > > Cc: Borislav Petkov > > Cc: Peter Zijlstra > > Cc: Ingo Molnar > > --- > > drivers/hwmon/fam15h_power.c | 120 +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 120 insertions(+) > > > > diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c > > index 6321f73..a5a539e 100644 > > --- a/drivers/hwmon/fam15h_power.c > > +++ b/drivers/hwmon/fam15h_power.c > > @@ -26,6 +26,9 @@ > > #include > > #include > > #include > > +#include > > +#include > > +#include > > #include > > #include > > > > @@ -64,6 +67,10 @@ struct fam15h_power_data { > > u64 cu_acc_power[MAX_CUS]; > > /* performance timestamp counter */ > > u64 cpu_sw_pwr_ptsc[MAX_CUS]; > > + /* online/offline status of current compute unit */ > > + int cu_on[MAX_CUS]; > > + unsigned long power_period; > > + struct mutex acc_pwr_mutex; > > }; > > > > static ssize_t show_power(struct device *dev, > > @@ -132,11 +139,15 @@ static void do_read_registers_on_cu(void *_data) > > cores_per_cu = amd_get_cores_per_cu(); > > cu = cpu / cores_per_cu; > > > > + mutex_lock(&data->acc_pwr_mutex); > > WARN_ON(rdmsrl_safe(MSR_F15H_CU_PWR_ACCUMULATOR, > > &data->cu_acc_power[cu])); > > > > WARN_ON(rdmsrl_safe(MSR_F15H_PTSC, > > &data->cpu_sw_pwr_ptsc[cu])); > > + > > + data->cu_on[cu] = 1; > > + mutex_unlock(&data->acc_pwr_mutex); > > } > > > > static int read_registers(struct fam15h_power_data *data) > > @@ -148,6 +159,10 @@ static int read_registers(struct fam15h_power_data *data) > > cores_per_cu = amd_get_cores_per_cu(); > > cu_num = boot_cpu_data.x86_max_cores / cores_per_cu; > > > > + mutex_lock(&data->acc_pwr_mutex); > > + memset(data->cu_on, 0, sizeof(int) * MAX_CUS); > > + mutex_unlock(&data->acc_pwr_mutex); > > + > > WARN_ON_ONCE(cu_num > MAX_CUS); > > > > ret = zalloc_cpumask_var(&mask, GFP_KERNEL); > > @@ -184,18 +199,113 @@ static int read_registers(struct fam15h_power_data *data) > > return 0; > > } > > > > +static ssize_t acc_show_power(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct fam15h_power_data *data = dev_get_drvdata(dev); > > + u64 prev_cu_acc_power[MAX_CUS], prev_ptsc[MAX_CUS], > > + jdelta[MAX_CUS]; > > + u64 tdelta, avg_acc; > > + int cu, cu_num, cores_per_cu, ret; > > + signed long leftover; > > + > > + cores_per_cu = amd_get_cores_per_cu(); > > + cu_num = boot_cpu_data.x86_max_cores / cores_per_cu; > > + > > + ret = read_registers(data); > > + if (ret) > > + return 0; > > + > > + cu = 0; > > + while(cu++ < cu_num) { > > + prev_cu_acc_power[cu] = data->cu_acc_power[cu]; > > + prev_ptsc[cu] = data->cpu_sw_pwr_ptsc[cu]; > > + } > > Please integrate checkpatch into your workflow of creating patches - it > can be correct sometimes: > > ERROR: space required before the open parenthesis '(' > #130: FILE: drivers/hwmon/fam15h_power.c:221: > + while(cu++ < cu_num) { > Oh, I will take care next time. > > > + > > + leftover = schedule_timeout_interruptible( > > + msecs_to_jiffies(data->power_period) > > + ); > > This way of writing a function call is reaaally ugly. What's wrong with: > > leftover = schedule_timeout_interruptible(msecs_to_jiffies(data->power_period)); > > ? > > And don't tell me 80 columns - that rule is not a hard one. > Actually, yes. I found it too long, so... :) OK, I will fix it on V3. Thanks, Rui