From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Date: Fri, 28 Aug 2015 14:05:13 +0000 Subject: Re: [lm-sensors] [PATCH 12/15] hwmon, fam15h_power: introduce a cpu accumulated power reporting algo Message-Id: <55E06A99.7070800@roeck-us.net> List-Id: References: <1440662866-28716-1-git-send-email-ray.huang@amd.com> <1440662866-28716-13-git-send-email-ray.huang@amd.com> <20150827173043.GB27452@roeck-us.net> <20150828104525.GD4191@hr-slim.amd.com> In-Reply-To: <20150828104525.GD4191@hr-slim.amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Huang Rui Cc: Borislav Petkov , Jean Delvare , Andy Lutomirski , Andreas Herrmann , Thomas Gleixner , Peter Zijlstra , Ingo Molnar , "Rafael J. Wysocki" , Len Brown , John Stultz , =?UTF-8?B?RnLDqWTDqXJpYyBXZWlzYmVja2Vy?= , lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org, x86@kernel.org, Andreas Herrmann , Aravind Gopalakrishnan , Borislav Petkov , Fengguang Wu , Aaron Lu , Tony Li T24gMDgvMjgvMjAxNSAwMzo0NSBBTSwgSHVhbmcgUnVpIHdyb3RlOgo+IE9uIFRodSwgQXVnIDI3 LCAyMDE1IGF0IDEwOjMwOjQzQU0gLTA3MDAsIEd1ZW50ZXIgUm9lY2sgd3JvdGU6Cj4+IE9uIFRo dSwgQXVnIDI3LCAyMDE1IGF0IDA0OjA3OjQzUE0gKzA4MDAsIEh1YW5nIFJ1aSB3cm90ZToKPj4+ IFRoaXMgcGF0Y2ggaW50cm9kdWNlcyBhbiBhbGdvcml0aG0gdGhhdCBjb21wdXRlcyB0aGUgYXZl cmFnZSBwb3dlciBieQo+Pj4gcmVhZGluZyBhIGRlbHRhIHZhbHVlIG9mIOKAnGNvcmUgcG93ZXIg YWNjdW11bGF0b3LigJ0gcmVnaXN0ZXIgZHVyaW5nCj4+PiBtZWFzdXJlbWVudCBpbnRlcnZhbCwg YW5kIHRoZW4gZGl2aWRpbmcgZGVsdGEgdmFsdWUgYnkgdGhlIGxlbmd0aCBvZgo+Pj4gdGhlIHRp bWUgaW50ZXJ2YWwuCj4+Pgo+Pj4gVXNlciBpcyBhYmxlIHRvIHVzZSBwb3dlcjFfYWNjIGVudHJ5 IHRvIG1lYXN1cmUgdGhlIHByb2Nlc3NvciBwb3dlcgo+Pj4gY29uc3VtcHRpb24gYW5kIHBvd2Vy MV9hY2MganVzdCBuZWVkcyB0byBiZSByZWFkIHR3aWNlIHdpdGggYW4gbmVlZGVkCj4+PiBpbnRl cnZhbCBpbi1iZXR3ZWVuLgo+Pj4KPj4+IEEgc2ltcGxlIGV4YW1wbGU6Cj4+Pgo+Pj4gJCBjYXQg L3N5cy9idXMvcGNpL2RldmljZXMvMDAwMFw6MDBcOjE4LjQvaHdtb24vaHdtb24wL3Bvd2VyMV9h Y2MKPj4+ICQgc2xlZXAgMTAwMDBzCj4+PiAkIGNhdCAvc3lzL2J1cy9wY2kvZGV2aWNlcy8wMDAw XDowMFw6MTguNC9od21vbi9od21vbjAvcG93ZXIxX2FjYwo+Pj4KPj4+IFRoZSByZXN1bHQgaXMg Y3VycmVudCBhdmVyYWdlIHByb2Nlc3NvciBwb3dlciBjb25zdW1wdGlvbiBpbiAxMDAwMAo+Pj4g c2Vjb25kcy4gVGhlIHVuaXQgb2YgdGhlIHJlc3VsdCBpcyB1V2F0dC4KPj4+Cj4+PiBTaWduZWQt b2ZmLWJ5OiBIdWFuZyBSdWkgPHJheS5odWFuZ0BhbWQuY29tPgo+Pj4gLS0tCj4+PiAgIGRyaXZl cnMvaHdtb24vZmFtMTVoX3Bvd2VyLmMgfCA2MiArKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrKwo+Pj4gICAxIGZpbGUgY2hhbmdlZCwgNjIgaW5zZXJ0aW9ucygrKQo+ Pj4KPj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2h3bW9uL2ZhbTE1aF9wb3dlci5jIGIvZHJpdmVy cy9od21vbi9mYW0xNWhfcG93ZXIuYwo+Pj4gaW5kZXggZDUyOWU0Yi4uM2JhYjc5NyAxMDA2NDQK Pj4+IC0tLSBhL2RyaXZlcnMvaHdtb24vZmFtMTVoX3Bvd2VyLmMKPj4+ICsrKyBiL2RyaXZlcnMv aHdtb24vZmFtMTVoX3Bvd2VyLmMKPj4+IEBAIC02MCw2ICs2MCw3IEBAIHN0cnVjdCBmYW0xNWhf cG93ZXJfZGF0YSB7Cj4+PiAgIAl1NjQgY3VfYWNjX3Bvd2VyW01BWF9DVVNdOwo+Pj4gICAJLyog cGVyZm9ybWFuY2UgdGltZXN0YW1wIGNvdW50ZXIgKi8KPj4+ICAgCXU2NCBjcHVfc3dfcHdyX3B0 c2NbTUFYX0NVU107Cj4+PiArCXN0cnVjdCBtdXRleCBhY2NfcHdyX211dGV4Owo+Pj4gICB9Owo+ Pj4KPj4+ICAgc3RhdGljIHNzaXplX3Qgc2hvd19wb3dlcihzdHJ1Y3QgZGV2aWNlICpkZXYsCj4+ PiBAQCAtMTIxLDE3ICsxMjIsNzQgQEAgc3RhdGljIERFVklDRV9BVFRSKHBvd2VyMV9jcml0LCBT X0lSVUdPLCBzaG93X3Bvd2VyX2NyaXQsIE5VTEwpOwo+Pj4gICBzdGF0aWMgc3RydWN0IGF0dHJp YnV0ZV9ncm91cCBmYW0xNWhfcG93ZXJfZ3JvdXA7Cj4+PiAgIF9fQVRUUklCVVRFX0dST1VQUyhm YW0xNWhfcG93ZXIpOwo+Pj4KPj4+ICtzdGF0aWMgc3NpemVfdCBzaG93X3Bvd2VyX2FjYyhzdHJ1 Y3QgZGV2aWNlICpkZXYsCj4+PiArCQkJICAgICAgc3RydWN0IGRldmljZV9hdHRyaWJ1dGUgKmF0 dHIsIGNoYXIgKmJ1ZikKPj4+ICt7Cj4+PiArCWludCBjcHUsIGN1LCBjdV9udW0sIGNvcmVzX3Bl cl9jdTsKPj4+ICsJdTY0IGN1cnJfY3VfYWNjX3Bvd2VyW01BWF9DVVNdLAo+Pj4gKwkgICAgY3Vy cl9wdHNjW01BWF9DVVNdLCBqZGVsdGFbTUFYX0NVU107Cj4+PiArCXU2NCB0ZGVsdGEsIGF2Z19h Y2M7Cj4+PiArCXN0cnVjdCBmYW0xNWhfcG93ZXJfZGF0YSAqZGF0YSA9IGRldl9nZXRfZHJ2ZGF0 YShkZXYpOwo+Pj4gKwo+Pj4gKwljb3Jlc19wZXJfY3UgPSBhbWRfZ2V0X2NvcmVzX3Blcl9jdSgp Owo+Pj4gKwljdV9udW0gPSBib290X2NwdV9kYXRhLng4Nl9tYXhfY29yZXMgLyBjb3Jlc19wZXJf Y3U7Cj4+PiArCj4+PiArCWZvciAoY3B1ID0gMCwgYXZnX2FjYyA9IDA7IGNwdSA8IGN1X251bSAq IGNvcmVzX3Blcl9jdTsgY3B1ICs9IGNvcmVzX3Blcl9jdSkgewo+Pj4gKwkJY3UgPSBjcHUgLyBj b3Jlc19wZXJfY3U7Cj4+PiArCQlpZiAocmRtc3JsX3NhZmVfb25fY3B1KGNwdSwgTVNSX0YxNUhf UFRTQywgJmN1cnJfcHRzY1tjdV0pKSB7Cj4+PiArCQkJcHJfZXJyKCJGYWlsZWQgdG8gcmVhZCBQ VFNDIGNvdW50ZXIgTVNSIG9uIGNvcmUlZFxuIiwKPj4+ICsJCQkgICAgICAgY3B1KTsKPj4+ICsJ CQlyZXR1cm4gMDsKPj4+ICsJCX0KPj4+ICsKPj4+ICsJCWlmIChyZG1zcmxfc2FmZV9vbl9jcHUo Y3B1LCBNU1JfRjE1SF9DVV9QV1JfQUNDVU1VTEFUT1IsCj4+PiArCQkJCSAgICAgICAmY3Vycl9j dV9hY2NfcG93ZXJbY3VdKSkgewo+Pj4gKwkJCXByX2VycigiRmFpbGVkIHRvIHJlYWQgY29tcHV0 ZSB1bml0IHBvd2VyIGFjY3VtdWxhdG9yIE1TUiBvbiBjb3JlJWRcbiIsCj4+PiArCQkJICAgICAg IGNwdSk7Cj4+PiArCQkJcmV0dXJuIDA7Cj4+PiArCQl9Cj4+PiArCj4+PiArCQlpZiAoY3Vycl9j dV9hY2NfcG93ZXJbY3VdIDwgZGF0YS0+Y3VfYWNjX3Bvd2VyW2N1XSkgewo+Pj4gKwkJCWpkZWx0 YVtjdV0gPSBkYXRhLT5tYXhfY3VfYWNjX3Bvd2VyICsgY3Vycl9jdV9hY2NfcG93ZXJbY3VdOwo+ Pj4gKwkJCWpkZWx0YVtjdV0gLT0gZGF0YS0+Y3VfYWNjX3Bvd2VyW2N1XTsKPj4+ICsJCX0gZWxz ZSB7Cj4+PiArCQkJamRlbHRhW2N1XSA9IGN1cnJfY3VfYWNjX3Bvd2VyW2N1XSAtIGRhdGEtPmN1 X2FjY19wb3dlcltjdV07Cj4+PiArCQl9Cj4+PiArCQl0ZGVsdGEgPSBjdXJyX3B0c2NbY3VdIC0g ZGF0YS0+Y3B1X3N3X3B3cl9wdHNjW2N1XTsKPj4+ICsJCWpkZWx0YVtjdV0gKj0gZGF0YS0+Y3B1 X3B3cl9zYW1wbGVfcmF0aW8gKiAxMDAwOwo+Pj4gKwkJZG9fZGl2KGpkZWx0YVtjdV0sIHRkZWx0 YSk7Cj4+PiArCj4+PiArCQltdXRleF9sb2NrKCZkYXRhLT5hY2NfcHdyX211dGV4KTsKPj4+ICsJ CWRhdGEtPmN1X2FjY19wb3dlcltjdV0gPSBjdXJyX2N1X2FjY19wb3dlcltjdV07Cj4+PiArCQlk YXRhLT5jcHVfc3dfcHdyX3B0c2NbY3VdID0gY3Vycl9wdHNjW2N1XTsKPj4+ICsJCW11dGV4X3Vu bG9jaygmZGF0YS0+YWNjX3B3cl9tdXRleCk7Cj4+PiArCj4+PiArCQkvKiB0aGUgdW5pdCBpcyBt aWNyb1dhdHQgKi8KPj4+ICsJCWF2Z19hY2MgKz0gamRlbHRhW2N1XTsKPj4+ICsJfQo+Pj4gKwo+ Pj4gKwlyZXR1cm4gc3ByaW50ZihidWYsICIldVxuIiwgKHVuc2lnbmVkIGludCkgYXZnX2FjYyk7 Cj4+PiArfQo+Pj4gK3N0YXRpYyBERVZJQ0VfQVRUUihwb3dlcjFfYWNjLCBTX0lSVUdPLCBzaG93 X3Bvd2VyX2FjYywgTlVMTCk7Cj4+Cj4+IEkgYW0gbm90IHJlYWxseSBhIGZyaWVuZCBvZiBpbnRy b2R1Y2luZyBhIG5vbi1zdGFuZGFyZCBhdHRyaWJ1dGUuCj4+IERvZXMgdGhlIGVuZXJneSBhdHRy aWJ1dGUgbm90IHdvcmsgaGVyZSA/Cj4+Cj4KPiBZb3UncmUgcmlnaHQuIE5vbi1zdGFuZGFyZCBh dHRyaWJ1dGUgbWlnaHQgbm90IGJlIGdvb2QuIENvdWxkIHlvdQo+IHBsZWFzZSBnaXZlIG1lIHNv bWUgaGludHMgaWYgSSB1c2UgImVuZXJneSIgaW5zdGVhZD8KPgoxIEpvdWxlID0gMSBXYXR0LXNl Y29uZC4KClNvbWV0aGluZyBlbHNlLCB0aG91Z2ggLSBkaWQgeW91IG1ha2Ugc3VyZSB0aGF0IHlv dXIgY29kZSBkb2Vzbid0IG92ZXJmbG93ID8KRXZlbiB0aG91Z2ggeW91IGNhbGN1bGF0ZSB0aGUg YXZlcmFnZSBpbiBhbiB1NjQsIHlvdSBkaXNwbGF5IGl0IGFzIHVuc2lnbmVkLgoKMTAwdyAqIDEw LDAwMHMgPSAxLDAwMCwwMDB3cyA9IDEsMDAwLDAwMCwwMDAsMDAwIG1pY3JvLXdhdHQtc2Vjb25k cywgd2hpY2ggaXMKYSBiaXQgbGFyZ2UgZm9yIGFuIHVuc2lnbmVkLgoKQWxzbywgdGhlIHZhbHVl cyBzaG91bGQgbm90IGJlIHJlc2V0IGFmdGVyIHJlYWRpbmcsIGJ1dCBhY2N1bXVsYXRlLgoKQWxz bywgSSB0aGluayB5b3VyIGNvZGUgbWF5IGJlIHZ1bG5lcmFibGUgdG8gb3ZlcmZsb3dzIG9uIHRo ZSBDUFUgcmVnaXN0ZXIgc2lkZS4KSG93IGxvbmcgZG9lcyBpdCB0YWtlIGJlZm9yZSB0aGUgQ1BV IGNvdW50ZXJzIG92ZXJmbG93ID8KClRoYW5rcywKR3VlbnRlcgoKCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0Cmxt LXNlbnNvcnNAbG0tc2Vuc29ycy5vcmcKaHR0cDovL2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxt YW4vbGlzdGluZm8vbG0tc2Vuc29ycw= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752636AbbH1OFV (ORCPT ); Fri, 28 Aug 2015 10:05:21 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:45151 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752106AbbH1OFT (ORCPT ); Fri, 28 Aug 2015 10:05:19 -0400 Message-ID: <55E06A99.7070800@roeck-us.net> Date: Fri, 28 Aug 2015 07:05:13 -0700 From: Guenter Roeck User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Huang Rui CC: Borislav Petkov , Jean Delvare , Andy Lutomirski , Andreas Herrmann , Thomas Gleixner , Peter Zijlstra , Ingo Molnar , "Rafael J. Wysocki" , Len Brown , John Stultz , =?UTF-8?B?RnLDqWTDqXJpYyBXZWlzYmVja2Vy?= , lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org, x86@kernel.org, Andreas Herrmann , Aravind Gopalakrishnan , Borislav Petkov , Fengguang Wu , Aaron Lu , Tony Li Subject: Re: [PATCH 12/15] hwmon, fam15h_power: introduce a cpu accumulated power reporting algorithm References: <1440662866-28716-1-git-send-email-ray.huang@amd.com> <1440662866-28716-13-git-send-email-ray.huang@amd.com> <20150827173043.GB27452@roeck-us.net> <20150828104525.GD4191@hr-slim.amd.com> In-Reply-To: <20150828104525.GD4191@hr-slim.amd.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=0.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/28/2015 03:45 AM, Huang Rui wrote: > On Thu, Aug 27, 2015 at 10:30:43AM -0700, Guenter Roeck wrote: >> On Thu, Aug 27, 2015 at 04:07:43PM +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_acc entry to measure the processor power >>> consumption and power1_acc just needs to be read twice with an needed >>> interval in-between. >>> >>> A simple example: >>> >>> $ cat /sys/bus/pci/devices/0000\:00\:18.4/hwmon/hwmon0/power1_acc >>> $ sleep 10000s >>> $ cat /sys/bus/pci/devices/0000\:00\:18.4/hwmon/hwmon0/power1_acc >>> >>> The result is current average processor power consumption in 10000 >>> seconds. The unit of the result is uWatt. >>> >>> Signed-off-by: Huang Rui >>> --- >>> drivers/hwmon/fam15h_power.c | 62 ++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 62 insertions(+) >>> >>> diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c >>> index d529e4b..3bab797 100644 >>> --- a/drivers/hwmon/fam15h_power.c >>> +++ b/drivers/hwmon/fam15h_power.c >>> @@ -60,6 +60,7 @@ struct fam15h_power_data { >>> u64 cu_acc_power[MAX_CUS]; >>> /* performance timestamp counter */ >>> u64 cpu_sw_pwr_ptsc[MAX_CUS]; >>> + struct mutex acc_pwr_mutex; >>> }; >>> >>> static ssize_t show_power(struct device *dev, >>> @@ -121,17 +122,74 @@ static DEVICE_ATTR(power1_crit, S_IRUGO, show_power_crit, NULL); >>> static struct attribute_group fam15h_power_group; >>> __ATTRIBUTE_GROUPS(fam15h_power); >>> >>> +static ssize_t show_power_acc(struct device *dev, >>> + struct device_attribute *attr, char *buf) >>> +{ >>> + int cpu, cu, cu_num, cores_per_cu; >>> + u64 curr_cu_acc_power[MAX_CUS], >>> + curr_ptsc[MAX_CUS], jdelta[MAX_CUS]; >>> + u64 tdelta, avg_acc; >>> + struct fam15h_power_data *data = dev_get_drvdata(dev); >>> + >>> + cores_per_cu = amd_get_cores_per_cu(); >>> + cu_num = boot_cpu_data.x86_max_cores / cores_per_cu; >>> + >>> + for (cpu = 0, avg_acc = 0; cpu < cu_num * cores_per_cu; cpu += cores_per_cu) { >>> + cu = cpu / cores_per_cu; >>> + if (rdmsrl_safe_on_cpu(cpu, MSR_F15H_PTSC, &curr_ptsc[cu])) { >>> + pr_err("Failed to read PTSC counter MSR on core%d\n", >>> + cpu); >>> + return 0; >>> + } >>> + >>> + if (rdmsrl_safe_on_cpu(cpu, MSR_F15H_CU_PWR_ACCUMULATOR, >>> + &curr_cu_acc_power[cu])) { >>> + pr_err("Failed to read compute unit power accumulator MSR on core%d\n", >>> + cpu); >>> + return 0; >>> + } >>> + >>> + if (curr_cu_acc_power[cu] < data->cu_acc_power[cu]) { >>> + jdelta[cu] = data->max_cu_acc_power + curr_cu_acc_power[cu]; >>> + jdelta[cu] -= data->cu_acc_power[cu]; >>> + } else { >>> + jdelta[cu] = curr_cu_acc_power[cu] - data->cu_acc_power[cu]; >>> + } >>> + tdelta = curr_ptsc[cu] - data->cpu_sw_pwr_ptsc[cu]; >>> + jdelta[cu] *= data->cpu_pwr_sample_ratio * 1000; >>> + do_div(jdelta[cu], tdelta); >>> + >>> + mutex_lock(&data->acc_pwr_mutex); >>> + data->cu_acc_power[cu] = curr_cu_acc_power[cu]; >>> + data->cpu_sw_pwr_ptsc[cu] = curr_ptsc[cu]; >>> + mutex_unlock(&data->acc_pwr_mutex); >>> + >>> + /* the unit is microWatt */ >>> + avg_acc += jdelta[cu]; >>> + } >>> + >>> + return sprintf(buf, "%u\n", (unsigned int) avg_acc); >>> +} >>> +static DEVICE_ATTR(power1_acc, S_IRUGO, show_power_acc, NULL); >> >> I am not really a friend of introducing a non-standard attribute. >> Does the energy attribute not work here ? >> > > You're right. Non-standard attribute might not be good. Could you > please give me some hints if I use "energy" instead? > 1 Joule = 1 Watt-second. Something else, though - did you make sure that your code doesn't overflow ? Even though you calculate the average in an u64, you display it as unsigned. 100w * 10,000s = 1,000,000ws = 1,000,000,000,000 micro-watt-seconds, which is a bit large for an unsigned. Also, the values should not be reset after reading, but accumulate. Also, I think your code may be vulnerable to overflows on the CPU register side. How long does it take before the CPU counters overflow ? Thanks, Guenter