* [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events
@ 2020-12-17 11:32 Kajol Jain
2020-12-17 11:40 ` Madhavan Srinivasan
2020-12-18 0:56 ` Michael Ellerman
0 siblings, 2 replies; 5+ messages in thread
From: Kajol Jain @ 2020-12-17 11:32 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: kjain, suka, maddy, atrajeev
hv_24x7 performance monitoring unit creates list of supported events
from the event catalog obtained via HCALL. hv_24x7 catalog could also
contain invalid or dummy events (with names like FREE_ or CPM_FREE_ so
on). These events does not have any hardware counters backing them.
So patch adds a check to string compare the event names to filter
out them.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 6e7e820508df..c3252d8a7818 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -894,6 +894,11 @@ static int create_events_from_catalog(struct attribute ***events_,
name = event_name(event, &nl);
+ if (strstr(name, "FREE_")) {
+ pr_info("invalid event %zu (%.*s)\n", event_idx, nl, name);
+ junk_events++;
+ continue;
+ }
if (event->event_group_record_len == 0) {
pr_devel("invalid event %zu (%.*s): group_record_len == 0, skipping\n",
event_idx, nl, name);
@@ -955,6 +960,9 @@ static int create_events_from_catalog(struct attribute ***events_,
continue;
name = event_name(event, &nl);
+ if (strstr(name, "FREE_"))
+ continue;
+
nonce = event_uniq_add(&ev_uniq, name, nl, event->domain);
ct = event_data_to_attrs(event_idx, events + event_attr_ct,
event, nonce);
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events
2020-12-17 11:32 [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events Kajol Jain
@ 2020-12-17 11:40 ` Madhavan Srinivasan
2020-12-17 11:51 ` kajoljain
2020-12-18 0:56 ` Michael Ellerman
1 sibling, 1 reply; 5+ messages in thread
From: Madhavan Srinivasan @ 2020-12-17 11:40 UTC (permalink / raw)
To: Kajol Jain, mpe, linuxppc-dev; +Cc: suka, maddy, atrajeev
On 12/17/20 5:02 PM, Kajol Jain wrote:
> hv_24x7 performance monitoring unit creates list of supported events
> from the event catalog obtained via HCALL. hv_24x7 catalog could also
> contain invalid or dummy events (with names like FREE_ or CPM_FREE_ so
Can you also include " RESERVED_NEST*" as part of the check.
# ls /sys/devices/hv_24x7/events | grep RESERVED
RESERVED_NEST1
RESERVED_NEST10
RESERVED_NEST11
RESERVED_NEST12
...
Maddy
> on). These events does not have any hardware counters backing them.
> So patch adds a check to string compare the event names to filter
> out them.
>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> arch/powerpc/perf/hv-24x7.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 6e7e820508df..c3252d8a7818 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -894,6 +894,11 @@ static int create_events_from_catalog(struct attribute ***events_,
>
> name = event_name(event, &nl);
>
> + if (strstr(name, "FREE_")) {
> + pr_info("invalid event %zu (%.*s)\n", event_idx, nl, name);
> + junk_events++;
> + continue;
> + }
> if (event->event_group_record_len == 0) {
> pr_devel("invalid event %zu (%.*s): group_record_len == 0, skipping\n",
> event_idx, nl, name);
> @@ -955,6 +960,9 @@ static int create_events_from_catalog(struct attribute ***events_,
> continue;
>
> name = event_name(event, &nl);
> + if (strstr(name, "FREE_"))
> + continue;
> +
> nonce = event_uniq_add(&ev_uniq, name, nl, event->domain);
> ct = event_data_to_attrs(event_idx, events + event_attr_ct,
> event, nonce);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events
2020-12-17 11:40 ` Madhavan Srinivasan
@ 2020-12-17 11:51 ` kajoljain
0 siblings, 0 replies; 5+ messages in thread
From: kajoljain @ 2020-12-17 11:51 UTC (permalink / raw)
To: Madhavan Srinivasan, mpe, linuxppc-dev; +Cc: suka, maddy, atrajeev
On 12/17/20 5:10 PM, Madhavan Srinivasan wrote:
>
> On 12/17/20 5:02 PM, Kajol Jain wrote:
>> hv_24x7 performance monitoring unit creates list of supported events
>> from the event catalog obtained via HCALL. hv_24x7 catalog could also
>> contain invalid or dummy events (with names like FREE_ or CPM_FREE_ so
>
>
> Can you also include " RESERVED_NEST*" as part of the check.
Hi Maddy,
Sure, I will add this check.
Thanks,
Kajol Jain
>
> # ls /sys/devices/hv_24x7/events | grep RESERVED
> RESERVED_NEST1
> RESERVED_NEST10
> RESERVED_NEST11
> RESERVED_NEST12
> ...
>
>
> Maddy
>
>
>> on). These events does not have any hardware counters backing them.
>> So patch adds a check to string compare the event names to filter
>> out them.
>>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> ---
>> arch/powerpc/perf/hv-24x7.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
>> index 6e7e820508df..c3252d8a7818 100644
>> --- a/arch/powerpc/perf/hv-24x7.c
>> +++ b/arch/powerpc/perf/hv-24x7.c
>> @@ -894,6 +894,11 @@ static int create_events_from_catalog(struct attribute ***events_,
>>
>> name = event_name(event, &nl);
>>
>> + if (strstr(name, "FREE_")) {
>> + pr_info("invalid event %zu (%.*s)\n", event_idx, nl, name);
>> + junk_events++;
>> + continue;
>> + }
>> if (event->event_group_record_len == 0) {
>> pr_devel("invalid event %zu (%.*s): group_record_len == 0, skipping\n",
>> event_idx, nl, name);
>> @@ -955,6 +960,9 @@ static int create_events_from_catalog(struct attribute ***events_,
>> continue;
>>
>> name = event_name(event, &nl);
>> + if (strstr(name, "FREE_"))
>> + continue;
>> +
>> nonce = event_uniq_add(&ev_uniq, name, nl, event->domain);
>> ct = event_data_to_attrs(event_idx, events + event_attr_ct,
>> event, nonce);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events
2020-12-17 11:32 [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events Kajol Jain
2020-12-17 11:40 ` Madhavan Srinivasan
@ 2020-12-18 0:56 ` Michael Ellerman
2020-12-18 7:51 ` kajoljain
1 sibling, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2020-12-18 0:56 UTC (permalink / raw)
To: Kajol Jain, linuxppc-dev; +Cc: kjain, suka, maddy, atrajeev
Kajol Jain <kjain@linux.ibm.com> writes:
> hv_24x7 performance monitoring unit creates list of supported events
> from the event catalog obtained via HCALL. hv_24x7 catalog could also
> contain invalid or dummy events (with names like FREE_ or CPM_FREE_ so
> on). These events does not have any hardware counters backing them.
> So patch adds a check to string compare the event names to filter
> out them.
>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> arch/powerpc/perf/hv-24x7.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 6e7e820508df..c3252d8a7818 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -894,6 +894,11 @@ static int create_events_from_catalog(struct attribute ***events_,
>
> name = event_name(event, &nl);
>
> + if (strstr(name, "FREE_")) {
> + pr_info("invalid event %zu (%.*s)\n", event_idx, nl, name);
> + junk_events++;
> + continue;
I don't think we want a print for each event, just one at the end saying
"Dropped %d invalid events" would be preferable I think.
> + }
> if (event->event_group_record_len == 0) {
> pr_devel("invalid event %zu (%.*s): group_record_len == 0, skipping\n",
> event_idx, nl, name);
> @@ -955,6 +960,9 @@ static int create_events_from_catalog(struct attribute ***events_,
> continue;
>
> name = event_name(event, &nl);
> + if (strstr(name, "FREE_"))
> + continue;
Would be nice if the string comparison was in a single place, ie. in a
helper function.
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events
2020-12-18 0:56 ` Michael Ellerman
@ 2020-12-18 7:51 ` kajoljain
0 siblings, 0 replies; 5+ messages in thread
From: kajoljain @ 2020-12-18 7:51 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: suka, maddy, atrajeev
On 12/18/20 6:26 AM, Michael Ellerman wrote:
> Kajol Jain <kjain@linux.ibm.com> writes:
>> hv_24x7 performance monitoring unit creates list of supported events
>> from the event catalog obtained via HCALL. hv_24x7 catalog could also
>> contain invalid or dummy events (with names like FREE_ or CPM_FREE_ so
>> on). These events does not have any hardware counters backing them.
>> So patch adds a check to string compare the event names to filter
>> out them.
>>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> ---
>> arch/powerpc/perf/hv-24x7.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
>> index 6e7e820508df..c3252d8a7818 100644
>> --- a/arch/powerpc/perf/hv-24x7.c
>> +++ b/arch/powerpc/perf/hv-24x7.c
>> @@ -894,6 +894,11 @@ static int create_events_from_catalog(struct attribute ***events_,
>>
>> name = event_name(event, &nl);
>>
>> + if (strstr(name, "FREE_")) {
>> + pr_info("invalid event %zu (%.*s)\n", event_idx, nl, name);
>> + junk_events++;
>> + continue;
>
> I don't think we want a print for each event, just one at the end saying
> "Dropped %d invalid events" would be preferable I think.
Hi Michael,
Sure I will remove prints for each event. Having one print for number of dropped
events may not be useful. So I will drop that too.
>
>
>> + }
>> if (event->event_group_record_len == 0) {
>> pr_devel("invalid event %zu (%.*s): group_record_len == 0, skipping\n",
>> event_idx, nl, name);
>> @@ -955,6 +960,9 @@ static int create_events_from_catalog(struct attribute ***events_,
>> continue;
>>
>> name = event_name(event, &nl);
>> + if (strstr(name, "FREE_"))
>> + continue;
>
> Would be nice if the string comparison was in a single place, ie. in a
> helper function.
Sure I will make that change.
Thanks,
Kajol Jain
>
> cheers
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-18 7:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-17 11:32 [PATCH] powerpc/perf/hv-24x7: Dont create sysfs event files for dummy events Kajol Jain
2020-12-17 11:40 ` Madhavan Srinivasan
2020-12-17 11:51 ` kajoljain
2020-12-18 0:56 ` Michael Ellerman
2020-12-18 7:51 ` kajoljain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).