From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753536AbcA2ISH (ORCPT ); Fri, 29 Jan 2016 03:18:07 -0500 Received: from mail-by2on0066.outbound.protection.outlook.com ([207.46.100.66]:38720 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753428AbcA2ISE (ORCPT ); Fri, 29 Jan 2016 03:18:04 -0500 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=amd.com; alien8.de; dkim=none (message not signed) header.d=none;alien8.de; dmarc=permerror action=none header.from=amd.com; X-WSS-ID: 0O1PGDV-08-MBE-02 X-M-MSG: Date: Fri, 29 Jan 2016 16:18:33 +0800 From: Huang Rui To: Peter Zijlstra CC: Borislav Petkov , Borislav Petkov , Ingo Molnar , Andy Lutomirski , Thomas Gleixner , Robert Richter , Jacob Shin , John Stultz , =?utf-8?B?RnLvv71k77+9cmlj?= Weisbecker , , , , Guenter Roeck , Andreas Herrmann , Suravee Suthikulpanit , Aravind Gopalakrishnan , Fengguang Wu , Aaron Lu Subject: Re: [PATCH v4] perf/x86/amd/power: Add AMD accumulated power reporting mechanism Message-ID: <20160129081817.GB28282@hr-amur2> References: <1453963131-2013-1-git-send-email-ray.huang@amd.com> <20160128090314.GB14274@pd.tnic> <20160128152848.GT6356@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160128152848.GT6356@twins.programming.kicks-ass.net> 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)(189002)(164054003)(24454002)(199003)(11100500001)(92566002)(33656002)(5008740100001)(3470700001)(101416001)(1076002)(1220700001)(1096002)(586003)(46406003)(23726003)(2906002)(4326007)(50986999)(87936001)(54356999)(110136002)(76176999)(551984002)(97756001)(83506001)(47776003)(4001350100001)(33716001)(50466002)(2950100001)(189998001)(106466001)(86362001)(105586002)(77096005)(107986001);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR12MB0862;H:atltwp02.amd.com;FPR:;SPF:None;MLV:sfv;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0862;2:CYqmQlBQq3VvyeYerXvfLp5Lwki8xrXNtagAkSqwO7CU+J4ItyTc7q828fYi7GHoDvbeqaCwWKb+GQ72e5ncaGM+ODrog18+d/yiXv9wvDEfdnpJcSqTqz4k8gYO5ch5kMYU044qI0UrXeFmTDz70A==;3:mhAN9sxNPKApsP6zuMJeyGn8kOoqfKHOf76kHJM+Jm6P7vwwaemigcLCOBn9cm/V5LHzGNlt9Cnll0RBORGhDEhDJqaDEfAWw5m2LhB4xvcfppt5Oo1wsQpP3XFjdBatKVAQbCJ/ufXGbjP8EqoyV3BdUfTcKwpi6bbnnb3AkhQHA17KuQ68Nh13gAEyOn0cwoAEzIcXfZgRq5223svDFLoYd/6ntZu4tGzWD9fTbbw=;25:yKvbnpjkpJJFG6Tl1OduJGEEJist2zYfVnm3PN025a+YmGjHm+hVOVkfgZtl0yGithj+FTIvxnTQQwbOuWjSrqe8dOnoppDJHZHs2dnEhYVnKgs86lzpg/JP3YeqS+2m1O/zPt+hS+YF8Q/QTmOIq/ru948zN+f+F9E1s1qvzszwA3oJHaW47QMFFyXPYFLXUmFLa27rhyUKICWUto8fRio1u6+apShZGB1e3hWTtvH+Mra2Ic7PLSF6v2ssr7rU X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0862; X-MS-Office365-Filtering-Correlation-Id: e2122d75-01a0-4be9-c1c3-08d32884b469 X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0862;20:RvNWIlRURiNHRdz3wBMn0rpd2pAgPzmdt48l5LVCnU+XXFZDPypXsw3Fj7StCxtLQx1LaSggXFr9DJvZuNm7ATlWDfHx5koppHQr1Gq/GOBj7M5olac6Vul5XAWehy21+IklWth+rctdgCXh/RM2o5xBZFeHi5BhS4s0kXSob+L9z0xFSDIGtSSS43O9KxqaCbdP8ja5t+IFEdOvEpJPPUkELHh7kRJ63PpAhotGWt29EC5RjJdCaW5nGRxVq06bltcWZ+lj4XyM1EMtBzq1p3PVVQ/bLR4LjC9uiz1JENeDKdP07GBjBtXTtp3U2dfJZvuKXf/fq4hpcsqHy/UAVsHaNoJOoYidPY6DZgje19nc9kUt/FzjSaFt0a5tC5mW6J4rMKOXnAT6s8/rGoTsgHTRQ0qNRNdaFIARcBnQofyp+WMju0/vRpg6pdRflg6z2oNkPmcGKrI/dFWqjWMlPXgN6CNto6OzBZS+VGDnBjzxNwg8lw29wxXpXH+T/7v+ X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(13023025)(13015025)(13017025)(13018025)(13024025)(8121501046)(5005006)(3002001)(10201501046);SRVR:SN1PR12MB0862;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0862; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0862;4:iBPUyCxbvQEC8kamEYDeEVfC2riTXyzK7bXZdjRaw1xJeQoNM7kTe+fUF3AH6aTa5Ua0xalcwOBqoFI9exY1YlXTkuPe2bmXaVMwW0wi9jnQtRsWhTi9Bgs5tNmbTCWsciBtO6pEJp2wUlG8YOwuZBsH1bnRkIe6AnAkS0LgPvV1pLOlVPZCoJO3M1+a07aokIEgZCMiQKVmtzGBJV5BVeOVjdNzu8CD51LZQ+bRlbqGw0/rNYgjwjzv0fr7fWZzxeQW8ojewUhMIk/aC4aeRwGg2AbX9/+biRI2oVsg8hMuP9qtWMdLF5Eyh5K/mIL33yBl5EQ+KdCE2a44B1jbJU3tsFHcaAZlG7lc5E7H22JPnxWMxprdxyoDTiw99Cu76OsCl7PFweOr1pNLnWD0BXKgQYIGR+rjtxl97zHQIewQSbDrw3d5BOZLfQ/5QF8jvYLsIDg3P4vhJYdEJsYAkA== X-Forefront-PRVS: 083691450C X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;SN1PR12MB0862;23:0CyLl5R4etjr0Osi/R3R0Mj4w8CVjU2OgMOqRpSdR?= =?us-ascii?Q?U8jzQCNQV7YsLhNP+Z4rz2jJgjdgDRv0YeMk0wIFbtWWGPjt/hVUzp+Wv/Ym?= =?us-ascii?Q?ext4De6+OkGnOEnccXiBL6ng4cjCAozNpvEq7uh91635EXbcbQ8+M0xdV3F1?= =?us-ascii?Q?ZxNovlEqEzzMBVoBXQ5WsuMDl2vP+ob+NFkcEkSn+zNMjAKvpakMMcgQGsz5?= =?us-ascii?Q?ulRZ7VjyGIBZqwKlbtcQfqrXsRlYyT+4HLuMrUhi2Fp2gGCfZ2Xpi5Z0s1Yj?= =?us-ascii?Q?nuW6WmuQwPtvncj2Yy2M6BkBZfSJLln3Dh8n5uGjlG8cyKWbiXyn+x9+2Ed+?= =?us-ascii?Q?Tavg1e9FPUA1D8/DB+LFu6h8EB2LVnFrKaAXOhwQa25h0a4iRRliMs8qH73f?= =?us-ascii?Q?7CT6Pl8NPhpVI6amIPbJAlmGpVy2qKInQJc/FezDPiZid+x0NW5N3nw36JLV?= =?us-ascii?Q?CMSAr5lBIroEUmURMaPjr5QycQ1g//3g16wCEoQlhx4zuSJk0Sa77nsP0uIq?= =?us-ascii?Q?IZ8E/D0Iqah+o4TsZBr1zqqIy0gdY+ubX7CTQz1qG0VoHazfln+fUedZCFoe?= =?us-ascii?Q?osyY40joRm44jIau2jfGO75NsTeNpecslLN99G4FcyyaAvzZMYL+9Es/HxCi?= =?us-ascii?Q?FONC6EKtEGYhForB/MVqDX37lKwNW91a9Sdd1pmiSlbh9oPEMVtoI0y32LIY?= =?us-ascii?Q?rSXBUdY0cLscfUCqe5at9a+QKK7IkNkQhAdw0BYY0NuPIzzV6vQ1BfN3MDsi?= =?us-ascii?Q?R3Ue+I6M0wO2pf8C/dUo8YrZoJI3VAXteqfWlysUqdOo3KBnip9xQDVGGBD3?= =?us-ascii?Q?7N+Pm9f06+E9DjAbajpOsATOz2Q9YdqeY331PjJBItH5kCO0gZGj9z0bz7T3?= =?us-ascii?Q?yWhz53JD3OTfZRq5YG11psXRKw2byI2wFjBWfbs1WwWgJ0Pc+IaVQfzXdZQC?= =?us-ascii?Q?/PF3WZiRMrU3cyhF+IYB9xV8MbUP57xeNXEMPH1kd/KUR45XnGsTfc4muthf?= =?us-ascii?Q?iuCz0OADWlhMJ2DuJZX3Jyqlj0Jq+XZRBDormAUAOc0xw=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0862;5:nKzEciIB0RU+7XwusdliiXWcnwE7DT50VIqA+ujQIPC2p2QGlGuVilbWJa6x+409T13tMqB//w3Zh/liK9pPeg5gELxjk/6DeX7XPrMNV7Xr4DEgU2DJQ3vTv8Yervq/klqGhFDMKVULg0iUUPg4Lw==;24:jk7ascGiZ/XgDODieFZ/yGHFZ26mxFJPRuyixeW11bQiEyip8gTzp552+BCWdln0MT7YlF14AElGWw1efUEhuRs29LzqiCKt70ZJLE9Ba9c=;20:z72bLlTASn53l/i1+LhlvKJT95yAz2WVUavGvV3Yk40k423Bkb3uEuskUnfyJQCU2DHAf+/TbuEICKBvza2Bf/FjVrQQ5yPx7YzU6CEP84FHTnG2zT1pUjkncVcg2Q2zlQovez45MNvP06MvEUMqXe0E7WVJ+jnRney6Sm/LPAFRiB9n2NFbme/Q2glCZw1fmZxhlpkVX+kEaLuRK4J+DRLt8oeu9iFlHMylhXqVoIg1zZNxzTpY0Y3VSOheBAtT SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jan 2016 08:17:59.8870 (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: SN1PR12MB0862 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 28, 2016 at 04:28:48PM +0100, Peter Zijlstra wrote: > On Thu, Jan 28, 2016 at 10:03:15AM +0100, Borislav Petkov wrote: > > > + > > +struct power_pmu { > > + raw_spinlock_t lock; > > Now that the list is gone, what does this thing protect? > Protect the event count value before measure it. > > + struct pmu *pmu; > > This member seems superfluous, there's only the one possible value. > Currently, it's only one. But there will be more power pmu types in future processors. Acc power is one of them. > > + local64_t cpu_sw_pwr_ptsc; > > + > > + /* > > + * These two cpumasks are used for avoiding the allocations on the > > + * CPU_STARTING phase because power_cpu_prepare() will be called with > > + * IRQs disabled. > > + */ > > + cpumask_var_t mask; > > + cpumask_var_t tmp_mask; > > +}; > > + > > +static struct pmu pmu_class; > > + > > +/* > > + * Accumulated power represents the sum of each compute unit's (CU) power > > + * consumption. On any core of each CU we read the total accumulated power from > > + * MSR_F15H_CU_PWR_ACCUMULATOR. cpu_mask represents CPU bit map of all cores > > + * which are picked to measure the power for the CUs they belong to. > > + */ > > +static cpumask_t cpu_mask; > > + > > +static DEFINE_PER_CPU(struct power_pmu *, amd_power_pmu); > > + > > +static u64 event_update(struct perf_event *event, struct power_pmu *pmu) > > +{ > > Is there ever a case where @pmu != __this_cpu_read(power_pmu) ? > It only might be called at pmu:{read, stop}, they ensure __this_cpu_read(amd_power_pmu). Is there any other case I missed? > > + struct hw_perf_event *hwc = &event->hw; > > + u64 prev_raw_count, new_raw_count, prev_ptsc, new_ptsc; > > + u64 delta, tdelta; > > + > > +again: > > + prev_raw_count = local64_read(&hwc->prev_count); > > + prev_ptsc = local64_read(&pmu->cpu_sw_pwr_ptsc); > > + rdmsrl(event->hw.event_base, new_raw_count); > > Is hw.event_base != MSR_F15H_CU_PWR_ACCUMULATOR possible? > Any case that I missed? Could you explain more? > > + rdmsrl(MSR_F15H_PTSC, new_ptsc); > > > Also, I suspect this doesn't do what you expect it to do. > > We measure per-event PWR_ACC deltas, but per CPU PTSC values. These do > not match when there's more than 1 event on the CPU. > OK, I see. My intention of pre-event's count (event->count) should be PWR_ACC values after divided by PTSC. But here we cannot use local64_read(&hwc->prev_count) as previous value of PWR_ACC before divided by PTSC. Thanks to catch it. > I would suggest adding a new struct to the hw_perf_event union with the > two u64 deltas like: > > struct { /* amd_power */ > u64 pwr_acc; > u64 ptsc; > }; > > And track these values per-event. > Thanks to reminder. Thanks, Rui