Re: [tip:perf/core] perf/x86: Export some PMU attributes in caps/ directory

From: Peter Zijlstra
Date: Mon Aug 28 2017 - 06:47:05 EST


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) <peterz@xxxxxxxxxxxxx>
---
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[] = {