From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933558AbcATEsu (ORCPT ); Tue, 19 Jan 2016 23:48:50 -0500 Received: from mail-bn1on0089.outbound.protection.outlook.com ([157.56.110.89]:20960 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933176AbcATEsj (ORCPT ); Tue, 19 Jan 2016 23:48:39 -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: 0O18IOV-08-KNL-02 X-M-MSG: Date: Wed, 20 Jan 2016 12:48:24 +0800 From: Huang Rui To: Peter Zijlstra CC: 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 , Borislav Petkov , "Fengguang Wu" , Aaron Lu Subject: Re: [PATCH v2 5/5] perf/x86/amd/power: Add AMD accumulated power reporting mechanism Message-ID: <20160120044823.GA13477@hr-amur2> References: <1452739808-11871-1-git-send-email-ray.huang@amd.com> <1452739808-11871-6-git-send-email-ray.huang@amd.com> <20160119121250.GA6344@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160119121250.GA6344@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)(24454002)(199003)(164054003)(189002)(5008740100001)(2906002)(1096002)(97756001)(23726003)(86362001)(11100500001)(76176999)(1220700001)(105586002)(92566002)(4326007)(87936001)(77096005)(586003)(47776003)(33716001)(4001350100001)(189998001)(83506001)(33656002)(2950100001)(106466001)(54356999)(110136002)(50466002)(101416001)(97736004)(50986999)(46406003)(1076002)(107986001);DIR:OUT;SFP:1101;SCL:1;SRVR:BY2PR12MB0709;H:atltwp02.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0709;2:rijGKaC44+63qVJ6NZ9W2ck18FjfvINBAQeAM2bIKZjqWhuTUcjqjc9IdoCYJtJnpkFA6RL+5jOsGT/ETKJ+jVkBqDMHdiBjOXxIj4Gq8Q5bAJbFKMjw3Q07C3FtVvZSTdix4cVGt0H656Vigm6oXg==;3:JDrInKfQT4bTy/UKn8Pr4GQ/Y7XBWC0zCJtB+mU2tjgJoTzT4n6UV+fTIwPi6q3gvykGwLVZqs+4OtkxuqgCOuIUeSWkohGWYfi+6ThUPZQCX2OzZihW0y6S6MGmH8NQnCRxUdcHTywtjVHH3EXgGzE+i3ncNoWha0dPDGYBuYWdTQUd3ub1gvWZ8F5WirsIfqLnAiIK1w9u6RksBCDv5qyfZ2O2RQr8YW6ncDGbzB8=;25:k7hRSepCE9AeFIMWm9kzvZfacw4b2BXriJMzUYSfib9dXSv2qXDaSNQuq2SJl+CWcJGPUFs8WrzBwN6PXZrxjOkC+u06oqMJezYfdTjajtoTgF0vCgJI5Tq0SfJBVIzYulJPnZ+DUtschAUX9UWrjdAZIQnTWyprwR04b2CKrQCkTq017a2kWgCRm8fvuQ6Qb2MTopHuO8cuiF9hZLXqR0XDlaHnc7OfvTzV/MUhsf3HFCxTXaO5bmjlGD0u7OON X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY2PR12MB0709; X-MS-Office365-Filtering-Correlation-Id: 907ba397-be57-4b02-8cfc-08d32154f471 X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0709;20:jcmLAds2dFrayNW8NLU67e+qh39xXkFJXUczkIL56SJ457hqXRYYFM0b5QHAfjtgnZhltM39Wsx+UvTzGSnSRePC0dM/ITwy7lkjn9h6zKIcMnv8pT+vJdbMAVLKQ0T0NiH3NVoD/ssEwqRscW5jUibcGXFGxZ8G13iGLaE+nLRxxQCvbp9neyXH72iX7//13VfP4IOL/iUor4X7XQAdVzUWb3lKNopHxPq/99cSuLWpCxTlJdioqq3BDzcdZX94lZ9OVnuIPIYvPgcYY1VuHdUeOLj0uGlx0DHyNRK013JcxgqLnwyHeYnOeF2w0dEMQRKXB596IWdx2GxmEF6mtixAmo7c90eBEITDQfovgTtZJ2s/odcJSEyhs67TsKJsJHYRhpJZm8FE0Moug24pM/jtVPveM79eeEBup/KdGJuVprTOTC7tEe1QZF2/WzMygHr67PJ/hWJkWmQyum3aMRM/4s0ioVaikp4LxeH0tzJOyJXRpgRzpaXJ9UE/1ng1 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(13023025)(520078)(13024025)(8121501046)(13015025)(5005006)(13017025)(13018025)(3002001)(10201501046);SRVR:BY2PR12MB0709;BCL:0;PCL:0;RULEID:;SRVR:BY2PR12MB0709; X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0709;4:lj7Zygln+g4JyEjZ6q9vNJDc/YZoJmexx0YjwdiFbwEZzenlusdXSsGrLKSaUMvhE3TGGQh1MVi3QADsxdixFtWsaa2hjfkq7rv0EplunUBjiLhsqJSutoUEvnArNLWKtaasBc0VNKcc3T/3ioaquLEz8DKD4mvreRcXDlDUojx/ONt59W8zRZwxp01ms7BVypnVaVkcId5jxrAvl3hLm+1oAuIR04gsa0MoOpcTfpKA27AooqXYb68NAYeAbTyo8NO0j5+nyjbBbaFQc6zC2uKiW1oczkXLtrbn/0Tat08PatJT0I/dm87mXiXPul3Gbah3Z8leGbal78V6wdYmlCQCa0acVXawrAWMCXbkosAT4U/VDpPdbVQSdUg3lhWYBoWpLIKV0iEwbV9EZ3cYyYPLR3wKqDvnLSadMuI9DCr9O/bmz4jHY46OQiNvgbhhVIVSKHORlSqCfQVC3qWsd0z/teo6ujv1W6Ifa44rSbE= X-Forefront-PRVS: 0827D7ACB9 X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BY2PR12MB0709;23:+3iAsJr5o2qjS7b1sdjEtJedrPZeBcYOgUlZfISRD?= =?us-ascii?Q?fZSioc5Se9SDf5mSCzBYoQKXyUDFommE7I8InYgErby8pXIdNQKaaf4IOpaa?= =?us-ascii?Q?a+udKSaNQ6SNqt1XXDudZHDjFC/I+4Nkv4RGy5gozkCLZ++dZKLEKmIbi2x9?= =?us-ascii?Q?17bWhBuHdyXLwpHh+3z2dpO+55ZpZLyNNVc1PplXFlL1tGWPBJtaL5DxDTQc?= =?us-ascii?Q?7dcrVsNfEUzkT0rNgLefzIXmLQ5aMpjmRXAmqP1+0x9K0Bf7RIVLjj8Na9bV?= =?us-ascii?Q?0jZguhAJDw5xOEKlKfjTA940FwrHoueUxFaZMV0MzCudMCWVNu6MegHooCAA?= =?us-ascii?Q?tkCog9P9lE8ZGmZM9BISwVuJMGPoGAKjSBFkDlTI5Hld3ybH3lyBx5GOTuKS?= =?us-ascii?Q?nH9NkdkggyRTucyIpk9qG3h3QVclTA7PxgLiZ53Sht61PCGWwS7PLHxLMSz8?= =?us-ascii?Q?M2/uvJjPfYbLmPeqqjloCdK/XvSviwmWIfFqu3FNYFWdCTKxGqX4yiXlkU3a?= =?us-ascii?Q?MTd87Kig4+ANKxT48BaU5GbSnDnA19tpBOAJJmPvTswKbsFpas6yKGAjQ0i8?= =?us-ascii?Q?HSXi+MOVRrpbmGIyHsZQI45/gX+WACR72Z5sb43ms7Tf7/p0dnhD9lLrFp43?= =?us-ascii?Q?H9+xta9MsqxckandTUwH4321WBXBZV/d7lBfg2TGae1KPhoGRlUw+VgQeeTP?= =?us-ascii?Q?Fzsq46HCydXPlXPQ1aU5brWSYbnVaC5/PUihmrG8xlijcTC9wqaTTmJUnPHP?= =?us-ascii?Q?SPUOsdYmSSSHClG4AuY3IHJxdfrreUTREqqdtUEwru+KCX970mmiLbcm3HIj?= =?us-ascii?Q?dbEs+JXufZsQBUbfAK8DS6DIGnviDTPjntkR0Kxm/b5fV1PlbFmpCaor3Q7P?= =?us-ascii?Q?4jRqkEy3RJod9aNnvLC9C/2QBxScGywX13no2Sdo/2lldRpq8Qzwjhq0z5zC?= =?us-ascii?Q?zR1YSkEi1JvoV7Sciu8RD/4V/obeN7/C3vdWZXlfEVr0jOLpcp1MqA2STTJ4?= =?us-ascii?Q?uNsbz8N7W8EvJAgjPWFSD6h?= X-Microsoft-Exchange-Diagnostics: 1;BY2PR12MB0709;5:fI1iC/qpWT+trIiV/c5isgyBUw5ScRmykBHnEB3q00Rhx/A4kDS8tOYD56UNbleQwYc5Ctxzyl1H2KSWbPtjmdxHQl5ou7MnY2nJOtawePIMyXO6QGHCpyq+oV/Kbdr8cerGz+jpteVeYZh11dz/Rg==;24:7y54j22/Jc3Ipszvbe8KExsws7KY7xC3o2GfMyYltc/hsHKdVcJOtUaPCYUsdik4GAVC/RnpVt8q8jbZa4ApYn+zkSF3hpBsb9Kk+hsm0UQ=;20:rXxcUyNWRg3L9p0vSk2WOfzmTQtdnpSQ64NbHYssCwEnPG67IdxMY5r9spkJN1aLUdML3LUVqIJthkrhzsGDZF/AhOLeuX9J/31LZDBBEHm4zeTwtvLO+KseL3W1OxJrj/mRzLt7WEf+ZhXZORPX2vFKEfNZpVZr3QmwyU/nYNV6SuiWu7UMNsvHZfMvA2MUb2+ZQCwb0tm+rZcvoirxHrVfc/ximfQzmCaCuwSELUL4BJxBzW7oUK8oOPZ3hkYl SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jan 2016 04:48:33.7985 (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: BY2PR12MB0709 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, Thanks so much to your comments. On Tue, Jan 19, 2016 at 01:12:50PM +0100, Peter Zijlstra wrote: > On Thu, Jan 14, 2016 at 10:50:08AM +0800, Huang Rui wrote: > > +struct power_pmu { > > + spinlock_t lock; > > This should be a raw_spinlock_t, as it'll be nested under other > raw_spinlock_t's. > Do you mean the following spinlock operations are in hardware interrupts disabled case, so I need use raw_spinlock_t instead, right? Use raw_spin_lock_irqsave/raw_spin_unlock_irqrestore? > > + struct list_head active_list; > > + struct pmu *pmu; /* pointer to power_pmu_class */ > > + local64_t cpu_sw_pwr_ptsc; > > +}; > > > +static void pmu_event_start(struct perf_event *event, int mode) > > +{ > > + struct power_pmu *pmu = __this_cpu_read(amd_power_pmu); > > + unsigned long flags; > > + > > + spin_lock_irqsave(&pmu->lock, flags); > > IRQs will be disabled here. > > > + __pmu_event_start(pmu, event); > > + spin_unlock_irqrestore(&pmu->lock, flags); > > +} > > + > > +static void pmu_event_stop(struct perf_event *event, int mode) > > +{ > > + struct power_pmu *pmu = __this_cpu_read(amd_power_pmu); > > + struct hw_perf_event *hwc = &event->hw; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&pmu->lock, flags); > > idem > > > + > > + /* mark event as deactivated and stopped */ > > + if (!(hwc->state & PERF_HES_STOPPED)) { > > + list_del(&event->active_entry); > > + hwc->state |= PERF_HES_STOPPED; > > + } > > + > > + /* check if update of sw counter is necessary */ > > + if ((mode & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) { > > + /* > > + * Drain the remaining delta count out of a event > > + * that we are disabling: > > + */ > > + event_update(event, pmu); > > + hwc->state |= PERF_HES_UPTODATE; > > + } > > + > > + spin_unlock_irqrestore(&pmu->lock, flags); > > +} > > + > > +static int pmu_event_add(struct perf_event *event, int mode) > > +{ > > + struct power_pmu *pmu = __this_cpu_read(amd_power_pmu); > > + struct hw_perf_event *hwc = &event->hw; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&pmu->lock, flags); > > + > > idem > > > + hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED; > > + > > + if (mode & PERF_EF_START) > > + __pmu_event_start(pmu, event); > > + > > + spin_unlock_irqrestore(&pmu->lock, flags); > > + > > + return 0; > > +} > > > > +static int power_cpu_init(int cpu) > > +{ > > + int i, cu, ret = 0; > > + cpumask_var_t mask, dummy_mask; > > + > > + cu = cpu / cores_per_cu; > > + > > + if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) > > + return -ENOMEM; > > + > > + if (!zalloc_cpumask_var(&dummy_mask, GFP_KERNEL)) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + > > + for (i = 0; i < cores_per_cu; i++) > > + cpumask_set_cpu(i, mask); > > + > > + cpumask_shift_left(mask, mask, cu * cores_per_cu); > > + > > + if (!cpumask_and(dummy_mask, mask, &cpu_mask)) > > + cpumask_set_cpu(cpu, &cpu_mask); > > + > > + free_cpumask_var(dummy_mask); > > +out: > > + free_cpumask_var(mask); > > + > > + return ret; > > +} > > > +static int power_cpu_notifier(struct notifier_block *self, > > + unsigned long action, void *hcpu) > > +{ > > + unsigned int cpu = (long)hcpu; > > + > > + switch (action & ~CPU_TASKS_FROZEN) { > > + case CPU_UP_PREPARE: > > + if (power_cpu_prepare(cpu)) > > + return NOTIFY_BAD; > > + break; > > + case CPU_STARTING: > > + if (power_cpu_init(cpu)) > > + return NOTIFY_BAD; > > this is called with IRQs disabled, which makes those GFP_KERNEL allocs > above a pretty bad idea. > Right, so should I use GFP_ATOMIC to allocate cpumask here? > Also, note that -rt cannot actually do _any_ allocations/frees from > STARTING. > > Please move the allocs/frees to PREPARE/ONLINE. > How about add two cpumask_var_t at power_pmu structure? Then allocate the two cpumask_var_t (pmu->mask, pmu->dummy_mask), and they can be also used on power_cpu_init. Thanks, Rui