Re: [PATCH] arm64: perf: Simplify the ARMv8 PMUv3 event attributes

[Mark Rutland <mark.rutland@arm.com>]

On Fri, Nov 01, 2019 at 10:36:17AM +0000, Will Deacon wrote:
> On Fri, Nov 01, 2019 at 08:53:19AM +0000, Mark Rutland wrote:
> > On Thu, Oct 31, 2019 at 04:08:04PM +0000, Will Deacon wrote:
> > > On Wed, Oct 30, 2019 at 11:46:17AM +0800, Shaokun Zhang wrote:
> > > > For each PMU event, there is a ARMV8_EVENT_ATTR(xx, XX) and
> > > > &armv8_event_attr_xx.attr.attr. Let's redefine the ARMV8_EVENT_ATTR
> > > > to simplify the armv8_pmuv3_event_attrs.
> > > > 
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> > > > ---
> > > >  arch/arm64/kernel/perf_event.c | 189 ++++++++++++++---------------------------
> > > >  1 file changed, 65 insertions(+), 124 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > > > index a0b4f1bca491..d0f084939bcf 100644
> > > > --- a/arch/arm64/kernel/perf_event.c
> > > > +++ b/arch/arm64/kernel/perf_event.c
> > > > @@ -159,132 +159,73 @@ armv8pmu_events_sysfs_show(struct device *dev,
> > > >  }
> > > 
> > > [...]
> > > 
> > > > +	(&((struct perf_pmu_events_attr[]) { \
> > > > +		{ .attr = __ATTR(name, 0444, armv8pmu_events_sysfs_show, NULL), \
> > > > +		  .id = config, } \
> > > > +	})[0].attr.attr)
> > > 
> > > I don't get the need for the array here. Why can't you do:
> > > 
> > > 	(&((struct perf_pmu_events_attr) {
> > > 		.attr = ...,
> > > 		.id = ...,
> > > 	}).attr.attr)
> > 
> > You need want &(obj.attr.attr) rather than &(obj).attr.attr, i.e.
> > 
> > #define ARMV8_EVENT_ATTR(name, config) \
> > 	(&((struct perf_pmu_events_attr) { \
> > 		.attr = __ATTR(name, 0444, armv8pmu_events_sysfs_show, NULL), \
> > 		.id = config, \
> > 	}.attr.attr))
> >  
> > ... which compiles for me.
> 
> Weird, the following compiles fine for me with both GCC and clang:
> 
> #define ARMV8_EVENT_ATTR(name, config)						\
> 	(&((struct perf_pmu_events_attr) {					\
> 		.attr = __ATTR(name, 0444, armv8pmu_events_sysfs_show, NULL),	\
> 		.id = config,							\
> 	}).attr.attr)

Works for me, too (I'm using the kernel.org crosstool GCC 8.1.0).

I must've messed up locally such that I had (&obj).attr.attr.

> > It'd be worth checking that yields a working data structure at runtime.
> 
> ...and perf list works as expected.

Perfect.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel