| Message ID | 1436360506-18805-6-git-send-email-maddy@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Madhavan Srinivasan [maddy@linux.vnet.ibm.com] wrote:
| Add code to create event/format attributes and attribute groups for
| each nest pmu.
|
| Cc: Michael Ellerman <mpe@ellerman.id.au>
| Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
| Cc: Paul Mackerras <paulus@samba.org>
| Cc: Anton Blanchard <anton@samba.org>
| Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
| Cc: Stephane Eranian <eranian@google.com>
| Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
| ---
| arch/powerpc/perf/nest-pmu.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
| 1 file changed, 57 insertions(+)
|
| diff --git a/arch/powerpc/perf/nest-pmu.c b/arch/powerpc/perf/nest-pmu.c
| index 6116ff3..20ed9f8 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");
| +struct attribute *p8_nest_format_attrs[] = {
name collision unlikely, but could this be static struct?
| + &format_attr_event.attr,
| + NULL,
| +};
| +
| +struct attribute_group p8_nest_format_group = {
static struct?
| + .name = "format",
| + .attrs = p8_nest_format_attrs,
| +};
| +
| static int nest_event_info(struct property *pp, char *start,
| struct nest_ima_events *p8_events, int flg, u32 val)
| {
| @@ -45,6 +56,48 @@ static int nest_event_info(struct property *pp, char *start,
| return 0;
| }
|
| +/*
| + * Populate event name and string in attribute
| + */
| +struct attribute *dev_str_attr(const char *name, const char *str)
static function?
| +{
| + struct perf_pmu_events_attr *attr;
| +
| + attr = kzalloc(sizeof(*attr), GFP_KERNEL);
| +
We recently needed following in 24x7 counters to keep lockdep happy:
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;
| +}
| +
| +int update_events_in_group(
static function?
nit: do we need a new line before the first parameter? some functions
in the file don't add the new line.
| + struct nest_ima_events *p8_events, int nevents, struct nest_pmu *pmu)
s/idx/nevents/?
| +{
| + struct attribute_group *attr_group;
| + struct attribute **attrs;
| + int i;
| +
| + /* Allocate memory for event attribute group */
| + attr_group = kzalloc(((sizeof(struct attribute *) * (idx + 1)) +
| + sizeof(*attr_group)), GFP_KERNEL);
| + if (!attr_group)
| + return -ENOMEM;
| +
| + attrs = (struct attribute **)(attr_group + 1);
Can you add a comment on the +1?
| + attr_group->name = "events";
| + attr_group->attrs = attrs;
| +
| + for (i = 0; i < idx; i++, p8_events++)
| + attrs[i] = dev_str_attr((char *)p8_events->ev_name,
| + (char *)p8_events->ev_value);
| +
| + pmu->attr_groups[0] = attr_group;
The ->attr_groups[0] is initialized here, after the ->attr_groups[1] and
attr_groups[2] are initialized in caller. Since, ->attr_groups[1] and
->attr_groups[2] are set to global (loop-invariant) values, can we
initialize all the attribute-groups here? May need to rename function.
| + return 0;
| +}
| +
| static int nest_pmu_create(struct device_node *dev, int pmu_index)
| {
| struct nest_ima_events **p8_events_arr, *p8_events;
| @@ -91,6 +144,7 @@ static int nest_pmu_create(struct device_node *dev, int pmu_index)
| /* Save the name to register it later */
| sprintf(buf, "Nest_%s", (char *)pp->value);
| pmu_ptr->pmu.name = (char *)buf;
| + pmu_ptr->attr_groups[1] = &p8_nest_format_group;
| continue;
| }
|
| @@ -122,6 +176,9 @@ static int nest_pmu_create(struct device_node *dev, int pmu_index)
| idx++;
| }
|
| + update_events_in_group(
nit: need newline before first param?
| + (struct nest_ima_events *)p8_events_arr, idx, pmu_ptr);
| +
| return 0;
| }
|
| --
| 1.9.1
On Thursday 09 July 2015 04:13 AM, Sukadev Bhattiprolu wrote: > Madhavan Srinivasan [maddy@linux.vnet.ibm.com] wrote: > | Add code to create event/format attributes and attribute groups for > | each nest pmu. > | > | Cc: Michael Ellerman <mpe@ellerman.id.au> > | Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > | Cc: Paul Mackerras <paulus@samba.org> > | Cc: Anton Blanchard <anton@samba.org> > | Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> > | Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com> > | Cc: Stephane Eranian <eranian@google.com> > | Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > | --- > | arch/powerpc/perf/nest-pmu.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ > | 1 file changed, 57 insertions(+) > | > | diff --git a/arch/powerpc/perf/nest-pmu.c b/arch/powerpc/perf/nest-pmu.c > | index 6116ff3..20ed9f8 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"); > | +struct attribute *p8_nest_format_attrs[] = { > > name collision unlikely, but could this be static struct? > > | + &format_attr_event.attr, > | + NULL, > | +}; > | + > | +struct attribute_group p8_nest_format_group = { > > static struct? Sure will check it. > > | + .name = "format", > | + .attrs = p8_nest_format_attrs, > | +}; > | + > | static int nest_event_info(struct property *pp, char *start, > | struct nest_ima_events *p8_events, int flg, u32 val) > | { > | @@ -45,6 +56,48 @@ static int nest_event_info(struct property *pp, char *start, > | return 0; > | } > | > | +/* > | + * Populate event name and string in attribute > | + */ > | +struct attribute *dev_str_attr(const char *name, const char *str) > > static function? Sure. Will check it. > > | +{ > | + struct perf_pmu_events_attr *attr; > | + > | + attr = kzalloc(sizeof(*attr), GFP_KERNEL); > | + > > We recently needed following in 24x7 counters to keep lockdep happy: Yeah, i saw your patch and wanted to add it, but missed it. My bad Will add it in the next version. > > 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; > | +} > | + > | +int update_events_in_group( > > static function? > > nit: do we need a new line before the first parameter? some functions > in the file don't add the new line. Checkpatch complained abt 80 character limit, hence did a split. > > | + struct nest_ima_events *p8_events, int nevents, struct nest_pmu *pmu) > > s/idx/nevents/? Weird, I dont see "nevents" in the patch I sent? it is "idx". Dont know where it came from :) > > | +{ > | + struct attribute_group *attr_group; > | + struct attribute **attrs; > | + int i; > | + > | + /* Allocate memory for event attribute group */ > | + attr_group = kzalloc(((sizeof(struct attribute *) * (idx + 1)) + > | + sizeof(*attr_group)), GFP_KERNEL); > | + if (!attr_group) > | + return -ENOMEM; > | + > | + attrs = (struct attribute **)(attr_group + 1); > > Can you add a comment on the +1? Sure, will do. > > | + attr_group->name = "events"; > | + attr_group->attrs = attrs; > | + > | + for (i = 0; i < idx; i++, p8_events++) > | + attrs[i] = dev_str_attr((char *)p8_events->ev_name, > | + (char *)p8_events->ev_value); > | + > | + pmu->attr_groups[0] = attr_group; > > The ->attr_groups[0] is initialized here, after the ->attr_groups[1] and > attr_groups[2] are initialized in caller. Since, ->attr_groups[1] and > ->attr_groups[2] are set to global (loop-invariant) values, can we > initialize all the attribute-groups here? May need to rename function. I dont get it, reinitialize all the three here in this function? > | + return 0; > | +} > | + > | static int nest_pmu_create(struct device_node *dev, int pmu_index) > | { > | struct nest_ima_events **p8_events_arr, *p8_events; > | @@ -91,6 +144,7 @@ static int nest_pmu_create(struct device_node *dev, int pmu_index) > | /* Save the name to register it later */ > | sprintf(buf, "Nest_%s", (char *)pp->value); > | pmu_ptr->pmu.name = (char *)buf; > | + pmu_ptr->attr_groups[1] = &p8_nest_format_group; > | continue; > | } > | > | @@ -122,6 +176,9 @@ static int nest_pmu_create(struct device_node *dev, int pmu_index) > | idx++; > | } > | > | + update_events_in_group( > > nit: need newline before first param? once again, checkpatch warning of 80 character. > > | + (struct nest_ima_events *)p8_events_arr, idx, pmu_ptr); > | + > | return 0; > | } > | > | -- > | 1.9.1
diff --git a/arch/powerpc/perf/nest-pmu.c b/arch/powerpc/perf/nest-pmu.c index 6116ff3..20ed9f8 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"); +struct attribute *p8_nest_format_attrs[] = { + &format_attr_event.attr, + NULL, +}; + +struct attribute_group p8_nest_format_group = { + .name = "format", + .attrs = p8_nest_format_attrs, +}; + static int nest_event_info(struct property *pp, char *start, struct nest_ima_events *p8_events, int flg, u32 val) { @@ -45,6 +56,48 @@ static int nest_event_info(struct property *pp, char *start, return 0; } +/* + * Populate event name and string in attribute + */ +struct attribute *dev_str_attr(const char *name, const char *str) +{ + struct perf_pmu_events_attr *attr; + + attr = kzalloc(sizeof(*attr), GFP_KERNEL); + + 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; +} + +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 event attribute group */ + attr_group = kzalloc(((sizeof(struct attribute *) * (idx + 1)) + + sizeof(*attr_group)), GFP_KERNEL); + if (!attr_group) + return -ENOMEM; + + attrs = (struct attribute **)(attr_group + 1); + attr_group->name = "events"; + attr_group->attrs = attrs; + + for (i = 0; i < idx; i++, p8_events++) + attrs[i] = dev_str_attr((char *)p8_events->ev_name, + (char *)p8_events->ev_value); + + pmu->attr_groups[0] = attr_group; + return 0; +} + static int nest_pmu_create(struct device_node *dev, int pmu_index) { struct nest_ima_events **p8_events_arr, *p8_events; @@ -91,6 +144,7 @@ static int nest_pmu_create(struct device_node *dev, int pmu_index) /* Save the name to register it later */ sprintf(buf, "Nest_%s", (char *)pp->value); pmu_ptr->pmu.name = (char *)buf; + pmu_ptr->attr_groups[1] = &p8_nest_format_group; continue; } @@ -122,6 +176,9 @@ static int nest_pmu_create(struct device_node *dev, int pmu_index) idx++; } + update_events_in_group( + (struct nest_ima_events *)p8_events_arr, idx, pmu_ptr); + return 0; }
Add code to create event/format attributes and attribute groups for each nest pmu. Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Anton Blanchard <anton@samba.org> Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com> Cc: Stephane Eranian <eranian@google.com> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> --- arch/powerpc/perf/nest-pmu.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)