[PATCH v5 5/7] powerpc/powernv: add event attribute and group to nest pmu

[Daniel Axtens]

On Thu, 2015-07-16 at 16:43 +0530, Madhavan Srinivasan wrote:
> Add code to create event/format attributes and attribute groups for
> each nest pmu.
> 
> Cc: Michael Ellerman <mpe at ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> Cc: Paul Mackerras <paulus at samba.org>
> Cc: Anton Blanchard <anton at samba.org>
> Cc: Sukadev Bhattiprolu <sukadev at linux.vnet.ibm.com>
> Cc: Anshuman Khandual <khandual at linux.vnet.ibm.com>
> Cc: Stephane Eranian <eranian at google.com>
> Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
> ---
>  arch/powerpc/perf/nest-pmu.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/arch/powerpc/perf/nest-pmu.c b/arch/powerpc/perf/nest-pmu.c
> index c4c08e4dee55..f3418bdec1cd 100644
> --- a/arch/powerpc/perf/nest-pmu.c
> +++ b/arch/powerpc/perf/nest-pmu.c
> @@ -13,6 +13,17 @@
>  static struct perchip_nest_info p8_nest_perchip_info[P8_NEST_MAX_CHIPS];
>  static struct nest_pmu *per_nest_pmu_arr[P8_NEST_MAX_PMUS];
>  
> +PMU_FORMAT_ATTR(event, "config:0-20");
> +static struct attribute *p8_nest_format_attrs[] = {
> +	&format_attr_event.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group p8_nest_format_group = {
> +	.name = "format",
> +	.attrs = p8_nest_format_attrs,
> +};
> +
>  static int nest_event_info(struct property *pp, char *name,
>  			struct nest_ima_events *p8_events, int string, u32 val)
>  {
> @@ -46,6 +57,56 @@ static int nest_event_info(struct property *pp, char *name,
>  	return 0;
>  }
>  
> +/*
> + * Populate event name and string in attribute
> + */
> +static struct attribute *dev_str_attr(const char *name, const char *str)
> +{
> +	struct perf_pmu_events_attr *attr;
> +
> +	attr = kzalloc(sizeof(*attr), GFP_KERNEL);
> +
> +	sysfs_attr_init(&attr->attr.attr);
> +
> +	attr->event_str = str;
> +	attr->attr.attr.name = name;
> +	attr->attr.attr.mode = 0444;
> +	attr->attr.show = perf_event_sysfs_show;
> +
> +	return &attr->attr.attr;
So I asked you about this before, and you pointed me to
perf_event_sysfs_show. Looking at that in kernel/events/core.c, it looks
like that uses container_of to pull out the perf_pmu_events_attr. So I
guess that is at least mostly correct.

I'm hoping something else uses container_of to pull out attr->attr, so
that they can actually grab the attr->attr.show function pointer, so
that perf_event_sysfs_show actually gets called. Where would that be?

> +}
> +
> +static int update_events_in_group(
> +	struct nest_ima_events *p8_events, int idx, struct nest_pmu *pmu)
> +{
> +	struct attribute_group *attr_group;
> +	struct attribute **attrs;
> +	int i;
> +
> +	/*
> +	 * Allocate memory for both event attribute group and for
> +	 * event attributes array.
> +	 */
> +	attr_group = kzalloc(((sizeof(struct attribute *) * (idx + 1)) +
> +				sizeof(*attr_group)), GFP_KERNEL);
> +	if (!attr_group)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Assign memory for event attribute array
> +	 */
> +	attrs = (struct attribute **)(attr_group + 1);
> +	attr_group->name = "events";
> +	attr_group->attrs = attrs;
I am super uncomfortable with this block, especially the assignment to
attrs. I *think* you're trying to allocate an attribute group and a set
of attributes, but you've combined the allocation into one big
contiguous chunk, and then you're trying to tease them apart. Is that
necessary? Could it be two allocs, one for the attribute_group and one
for the attribute?

-- 
Regards,
Daniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 860 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20150722/c73777ee/attachment-0001.sig>