Re: [PATCH v4 5/7] powerpc/powernv: add event attribute and group to nest pmu
On Thursday 09 July 2015 04:13 AM, Sukadev Bhattiprolu wrote:
Madhavan Srinivasan [ma...@linux.vnet.ibm.com] wrote:
| Add code to create event/format attributes and attribute groups for
| each nest pmu.
|
| Cc: Michael Ellerman m...@ellerman.id.au
| Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
| Cc: Paul Mackerras pau...@samba.org
| Cc: Anton Blanchard an...@samba.org
| Cc: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
| Cc: Anshuman Khandual khand...@linux.vnet.ibm.com
| Cc: Stephane Eranian eran...@google.com
| Signed-off-by: Madhavan Srinivasan ma...@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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
[PATCH v4 5/7] powerpc/powernv: add event attribute and group to nest pmu
Add code to create event/format attributes and attribute groups for
each nest pmu.
Cc: Michael Ellerman m...@ellerman.id.au
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Cc: Paul Mackerras pau...@samba.org
Cc: Anton Blanchard an...@samba.org
Cc: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
Cc: Anshuman Khandual khand...@linux.vnet.ibm.com
Cc: Stephane Eranian eran...@google.com
Signed-off-by: Madhavan Srinivasan ma...@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[] = {
+ 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;
}
--
1.9.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 5/7] powerpc/powernv: add event attribute and group to nest pmu
Madhavan Srinivasan [ma...@linux.vnet.ibm.com] wrote:
| Add code to create event/format attributes and attribute groups for
| each nest pmu.
|
| Cc: Michael Ellerman m...@ellerman.id.au
| Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
| Cc: Paul Mackerras pau...@samba.org
| Cc: Anton Blanchard an...@samba.org
| Cc: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
| Cc: Anshuman Khandual khand...@linux.vnet.ibm.com
| Cc: Stephane Eranian eran...@google.com
| Signed-off-by: Madhavan Srinivasan ma...@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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
