From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751274AbdH1KrB (ORCPT ); Mon, 28 Aug 2017 06:47:01 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:54726 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750838AbdH1Kq7 (ORCPT ); Mon, 28 Aug 2017 06:46:59 -0400 Date: Mon, 28 Aug 2017 12:46:50 +0200 From: Peter Zijlstra To: mingo@kernel.org, linux-kernel@vger.kernel.org, hpa@zytor.com, tglx@linutronix.de, ak@linux.intel.com, torvalds@linux-foundation.org Cc: linux-tip-commits@vger.kernel.org, Borislav Petkov Subject: Re: [tip:perf/core] perf/x86: Export some PMU attributes in caps/ directory Message-ID: <20170828104650.2u3rsim4jafyjzv2@hirez.programming.kicks-ass.net> References: <20170822185201.9261-3-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 25, 2017 at 04:55:03AM -0700, tip-bot for Andi Kleen wrote: > @@ -1798,6 +1808,7 @@ static int __init init_hw_perf_events(void) > 0, x86_pmu.num_counters, 0, 0); > > x86_pmu_format_group.attrs = x86_pmu.format_attrs; > + x86_pmu_caps_group.attrs = x86_pmu.caps_attrs; > > if (x86_pmu.event_attrs) > x86_pmu_events_group.attrs = x86_pmu.event_attrs; > @@ -2217,6 +2228,7 @@ static const struct attribute_group *x86_pmu_attr_groups[] = { > &x86_pmu_attr_group, > &x86_pmu_format_group, > &x86_pmu_events_group, > + &x86_pmu_caps_group, > NULL, > }; This generates: [ 1.421821] ------------[ cut here ]------------ [ 1.423424] WARNING: CPU: 1 PID: 1 at fs/sysfs/group.c:120 internal_create_group+0x277/0x2c0 [ 1.426453] Modules linked in: [ 1.427777] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc7+ #2 [ 1.429538] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014 [ 1.432218] task: ffff88007c138000 task.stack: ffffc90000008000 [ 1.433760] RIP: 0010:internal_create_group+0x277/0x2c0 [ 1.435187] RSP: 0018:ffffc9000000bd78 EFLAGS: 00010296 [ 1.436757] RAX: 000000000000003b RBX: 0000000000000003 RCX: 0000000000000000 [ 1.438461] RDX: 0000000000000001 RSI: ffffffff8109ff63 RDI: ffffffff8109ff63 [ 1.447453] RBP: ffffc9000000bdb0 R08: 0000000000000000 R09: 0000000000000102 [ 1.449227] R10: ffff88007c0e10d0 R11: 0000000081e22b01 R12: ffffffff81c10fc0 [ 1.450924] R13: 0000000000000000 R14: ffff88007b018aa0 R15: ffff88007b018810 [ 1.452718] FS: 0000000000000000(0000) GS:ffff88007ec40000(0000) knlGS:0000000000000000 [ 1.455053] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1.456630] CR2: ffffc900001fc000 CR3: 0000000001c09000 CR4: 00000000000406e0 [ 1.458334] Call Trace: [ 1.459341] sysfs_create_groups+0x41/0x80 [ 1.460675] device_add+0x5ae/0x600 [ 1.461842] ? set_debug_rodata+0x17/0x17 [ 1.463042] pmu_dev_alloc+0x9a/0xf0 [ 1.464297] perf_event_sysfs_init+0x54/0x8d [ 1.465570] ? trace_event_define_fields_xdp_exception+0x87/0x87 [ 1.486645] do_one_initcall+0x52/0x190 [ 1.487872] ? set_debug_rodata+0x17/0x17 [ 1.489188] kernel_init_freeable+0x11e/0x1a1 [ 1.490800] ? rest_init+0xd0/0xd0 [ 1.492628] kernel_init+0xe/0x100 [ 1.494344] ret_from_fork+0x27/0x40 [ 1.496130] Code: 48 83 7a 20 00 0f 85 f5 fd ff ff 48 8b 02 48 8b 37 48 c7 c2 16 d4 9e 81 48 c7 c7 c8 08 9f 81 48 85 c0 48 0f 45 d0 e8 3a 17 ea ff <0f> ff b8 ea ff ff ff e9 ab fe ff ff 48 83 7f 30 00 0f 85 98 fd [ 1.501991] ---[ end trace aa30ea041c8942a2 ]--- When ran on !intel systems and: > +static ssize_t max_precise_show(struct device *cdev, > + struct device_attribute *attr, > + char *buf) > +{ > + return snprintf(buf, PAGE_SIZE, "%d\n", x86_pmu_max_precise()); > +} > + > +static DEVICE_ATTR_RO(max_precise); is not intel specific at all.. Not very nice. Boris, could you give this a spin? --- Subject: perf/x86: Fix caps/ for !Intel Move the 'max_precise' capability into generic x86 code where it belongs. This fixes a sysfs splat on !Intel systems where we fail to set x86_pmu_caps_group.atts. Fixes: 22688d1c20f5 ("x86/perf: Export some PMU attributes in caps/ directory") Signed-off-by: Peter Zijlstra (Intel) --- arch/x86/events/core.c | 33 ++++++++++++++++++++++++++++----- arch/x86/events/intel/core.c | 14 ++------------ 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index d5f98095a155..73a6311c8baa 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -1757,10 +1757,7 @@ ssize_t x86_event_sysfs_show(char *page, u64 config, u64 event) } static struct attribute_group x86_pmu_attr_group; - -static struct attribute_group x86_pmu_caps_group = { - .name = "caps", -}; +static struct attribute_group x86_pmu_caps_group; static int __init init_hw_perf_events(void) { @@ -1808,7 +1805,14 @@ static int __init init_hw_perf_events(void) 0, x86_pmu.num_counters, 0, 0); x86_pmu_format_group.attrs = x86_pmu.format_attrs; - x86_pmu_caps_group.attrs = x86_pmu.caps_attrs; + + if (x86_pmu.caps_attrs) { + struct attribute **tmp; + + tmp = merge_attr(x86_pmu_caps_group.attrs, x86_pmu.caps_attrs); + if (!WARN_ON(!tmp)) + x86_pmu_caps_group.attrs = tmp; + } if (x86_pmu.event_attrs) x86_pmu_events_group.attrs = x86_pmu.event_attrs; @@ -2224,6 +2228,25 @@ static struct attribute_group x86_pmu_attr_group = { .attrs = x86_pmu_attrs, }; +static ssize_t max_precise_show(struct device *cdev, + struct device_attribute *attr, + char *buf) +{ + return snprintf(buf, PAGE_SIZE, "%d\n", x86_pmu_max_precise()); +} + +static DEVICE_ATTR_RO(max_precise); + +static struct attribute *x86_pmu_caps_attrs[] = { + &dev_attr_max_precise.attr, + NULL +}; + +static struct attribute_group x86_pmu_caps_group = { + .name = "caps", + .attrs = x86_pmu_caps_attrs, +}; + static const struct attribute_group *x86_pmu_attr_groups[] = { &x86_pmu_attr_group, &x86_pmu_format_group, diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c index 8fa2abd9c8b6..829e89cfcee2 100644 --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -3820,19 +3820,9 @@ static ssize_t pmu_name_show(struct device *cdev, static DEVICE_ATTR_RO(pmu_name); -static ssize_t max_precise_show(struct device *cdev, - struct device_attribute *attr, - char *buf) -{ - return snprintf(buf, PAGE_SIZE, "%d\n", x86_pmu_max_precise()); -} - -static DEVICE_ATTR_RO(max_precise); - static struct attribute *intel_pmu_caps_attrs[] = { - &dev_attr_pmu_name.attr, - &dev_attr_max_precise.attr, - NULL + &dev_attr_pmu_name.attr, + NULL }; static struct attribute *intel_pmu_attrs[] = {