[PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[Athira Rajeev]

Testcase "Parsing of all PMU events from sysfs" parse events for
all PMUs, and not just cpu. In case of powerpc, the PowerVM
environment supports events from hv_24x7 and hv_gpci PMU which
is of example format like below:

- hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
- hv_gpci/event,partition_id=?/

The value for "?" needs to be filled in depending on system
configuration. It is better to skip these parametrized events
in this test as it is done in:
'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
parametrized events")' which handled a simialr instance with
"all PMU test".

Fix parse-events test to skip parametrized events since
it needs proper setup of the parameters.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
Changelog:
v1 -> v2:
 Addressed review comments from Ian. Updated size of
 pmu event name variable and changed bool name which is
 used to skip the test.

 tools/perf/tests/parse-events.c | 38 +++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 658fb9599d95..1ecaeceb69f8 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -2514,9 +2514,14 @@ static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest
 	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
 		struct stat st;
 		char path[PATH_MAX];
+		char pmu_event[PATH_MAX];
+		char *buf = NULL;
+		FILE *file;
 		struct dirent *ent;
+		size_t len = 0;
 		DIR *dir;
 		int err;
+		int n;
 
 		snprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s/events/",
 			sysfs__mountpoint(), pmu->name);
@@ -2538,11 +2543,44 @@ static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest
 			struct evlist_test e = { .name = NULL, };
 			char name[2 * NAME_MAX + 1 + 12 + 3];
 			int test_ret;
+			bool is_event_parameterized = 0;
 
 			/* Names containing . are special and cannot be used directly */
 			if (strchr(ent->d_name, '.'))
 				continue;
 
+			/* exclude parametrized ones (name contains '?') */
+			n = snprintf(pmu_event, sizeof(pmu_event), "%s%s", path, ent->d_name);
+			if (n >= PATH_MAX) {
+				pr_err("pmu event name crossed PATH_MAX(%d) size\n", PATH_MAX);
+				continue;
+			}
+
+			file = fopen(pmu_event, "r");
+			if (!file) {
+				pr_debug("can't open pmu event file for '%s'\n", ent->d_name);
+				ret = combine_test_results(ret, TEST_FAIL);
+				continue;
+			}
+
+			if (getline(&buf, &len, file) < 0) {
+				pr_debug(" pmu event: %s is a null event\n", ent->d_name);
+				ret = combine_test_results(ret, TEST_FAIL);
+				continue;
+			}
+
+			if (strchr(buf, '?'))
+				is_event_parameterized = 1;
+
+			free(buf);
+			buf = NULL;
+			fclose(file);
+
+			if (is_event_parameterized == 1) {
+				pr_debug("skipping parametrized PMU event: %s which contains ?\n", pmu_event);
+				continue;
+			}
+
 			snprintf(name, sizeof(name), "%s/event=%s/u", pmu->name, ent->d_name);
 
 			e.name  = name;
-- 
2.31.1

Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[Ian Rogers]

On Thu, Sep 7, 2023 at 9:59 AM Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:
>
> Testcase "Parsing of all PMU events from sysfs" parse events for
> all PMUs, and not just cpu. In case of powerpc, the PowerVM
> environment supports events from hv_24x7 and hv_gpci PMU which
> is of example format like below:
>
> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> - hv_gpci/event,partition_id=?/
>
> The value for "?" needs to be filled in depending on system
> configuration. It is better to skip these parametrized events
> in this test as it is done in:
> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
> parametrized events")' which handled a simialr instance with
> "all PMU test".
>
> Fix parse-events test to skip parametrized events since
> it needs proper setup of the parameters.
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

Tested-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
> Changelog:
> v1 -> v2:
>  Addressed review comments from Ian. Updated size of
>  pmu event name variable and changed bool name which is
>  used to skip the test.
>
>  tools/perf/tests/parse-events.c | 38 +++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 658fb9599d95..1ecaeceb69f8 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -2514,9 +2514,14 @@ static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest
>         while ((pmu = perf_pmus__scan(pmu)) != NULL) {
>                 struct stat st;
>                 char path[PATH_MAX];
> +               char pmu_event[PATH_MAX];
> +               char *buf = NULL;
> +               FILE *file;
>                 struct dirent *ent;
> +               size_t len = 0;
>                 DIR *dir;
>                 int err;
> +               int n;
>
>                 snprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s/events/",
>                         sysfs__mountpoint(), pmu->name);
> @@ -2538,11 +2543,44 @@ static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest
>                         struct evlist_test e = { .name = NULL, };
>                         char name[2 * NAME_MAX + 1 + 12 + 3];
>                         int test_ret;
> +                       bool is_event_parameterized = 0;
>
>                         /* Names containing . are special and cannot be used directly */
>                         if (strchr(ent->d_name, '.'))
>                                 continue;
>
> +                       /* exclude parametrized ones (name contains '?') */
> +                       n = snprintf(pmu_event, sizeof(pmu_event), "%s%s", path, ent->d_name);
> +                       if (n >= PATH_MAX) {
> +                               pr_err("pmu event name crossed PATH_MAX(%d) size\n", PATH_MAX);
> +                               continue;
> +                       }
> +
> +                       file = fopen(pmu_event, "r");
> +                       if (!file) {
> +                               pr_debug("can't open pmu event file for '%s'\n", ent->d_name);
> +                               ret = combine_test_results(ret, TEST_FAIL);
> +                               continue;
> +                       }
> +
> +                       if (getline(&buf, &len, file) < 0) {
> +                               pr_debug(" pmu event: %s is a null event\n", ent->d_name);
> +                               ret = combine_test_results(ret, TEST_FAIL);
> +                               continue;
> +                       }
> +
> +                       if (strchr(buf, '?'))
> +                               is_event_parameterized = 1;
> +
> +                       free(buf);
> +                       buf = NULL;
> +                       fclose(file);
> +
> +                       if (is_event_parameterized == 1) {
> +                               pr_debug("skipping parametrized PMU event: %s which contains ?\n", pmu_event);
> +                               continue;
> +                       }
> +
>                         snprintf(name, sizeof(name), "%s/event=%s/u", pmu->name, ent->d_name);
>
>                         e.name  = name;
> --
> 2.31.1
>

Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[Sachin Sant]

> On 07-Sep-2023, at 10:29 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
> 
> Testcase "Parsing of all PMU events from sysfs" parse events for
> all PMUs, and not just cpu. In case of powerpc, the PowerVM
> environment supports events from hv_24x7 and hv_gpci PMU which
> is of example format like below:
> 
> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> - hv_gpci/event,partition_id=?/
> 
> The value for "?" needs to be filled in depending on system
> configuration. It is better to skip these parametrized events
> in this test as it is done in:
> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
> parametrized events")' which handled a simialr instance with
> "all PMU test".
> 
> Fix parse-events test to skip parametrized events since
> it needs proper setup of the parameters.
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changelog:
> v1 -> v2:
> Addressed review comments from Ian. Updated size of
> pmu event name variable and changed bool name which is
> used to skip the test.
> 

The patch fixes the reported issue.

6.2: Parsing of all PMU events from sysfs                          : Ok
6.3: Parsing of given PMU events from sysfs                        : Ok

Tested-by: Sachin Sant <sachinp@linux.ibm.com>

- Sachin

Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[Athira Rajeev]

> On 08-Sep-2023, at 11:04 AM, Sachin Sant <sachinp@linux.ibm.com> wrote:
> 
> 
> 
>> On 07-Sep-2023, at 10:29 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>> 
>> Testcase "Parsing of all PMU events from sysfs" parse events for
>> all PMUs, and not just cpu. In case of powerpc, the PowerVM
>> environment supports events from hv_24x7 and hv_gpci PMU which
>> is of example format like below:
>> 
>> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
>> - hv_gpci/event,partition_id=?/
>> 
>> The value for "?" needs to be filled in depending on system
>> configuration. It is better to skip these parametrized events
>> in this test as it is done in:
>> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
>> parametrized events")' which handled a simialr instance with
>> "all PMU test".
>> 
>> Fix parse-events test to skip parametrized events since
>> it needs proper setup of the parameters.
>> 
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
>> Changelog:
>> v1 -> v2:
>> Addressed review comments from Ian. Updated size of
>> pmu event name variable and changed bool name which is
>> used to skip the test.
>> 
> 
> The patch fixes the reported issue.
> 
> 6.2: Parsing of all PMU events from sysfs                          : Ok
> 6.3: Parsing of given PMU events from sysfs                        : Ok
> 
> Tested-by: Sachin Sant <sachinp@linux.ibm.com>
> 
> - Sachin

Hi Sachin, Ian

Thanks for testing the patch

Athira

Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[Athira Rajeev]

> On 08-Sep-2023, at 7:48 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
> 
> 
> 
>> On 08-Sep-2023, at 11:04 AM, Sachin Sant <sachinp@linux.ibm.com> wrote:
>> 
>> 
>> 
>>> On 07-Sep-2023, at 10:29 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>>> 
>>> Testcase "Parsing of all PMU events from sysfs" parse events for
>>> all PMUs, and not just cpu. In case of powerpc, the PowerVM
>>> environment supports events from hv_24x7 and hv_gpci PMU which
>>> is of example format like below:
>>> 
>>> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
>>> - hv_gpci/event,partition_id=?/
>>> 
>>> The value for "?" needs to be filled in depending on system
>>> configuration. It is better to skip these parametrized events
>>> in this test as it is done in:
>>> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
>>> parametrized events")' which handled a simialr instance with
>>> "all PMU test".
>>> 
>>> Fix parse-events test to skip parametrized events since
>>> it needs proper setup of the parameters.
>>> 
>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>> ---
>>> Changelog:
>>> v1 -> v2:
>>> Addressed review comments from Ian. Updated size of
>>> pmu event name variable and changed bool name which is
>>> used to skip the test.
>>> 
>> 
>> The patch fixes the reported issue.
>> 
>> 6.2: Parsing of all PMU events from sysfs                          : Ok
>> 6.3: Parsing of given PMU events from sysfs                        : Ok
>> 
>> Tested-by: Sachin Sant <sachinp@linux.ibm.com>
>> 
>> - Sachin
> 
> Hi Sachin, Ian
> 
> Thanks for testing the patch

Hi Arnaldo

Can you please check and pull this if it looks good to go .

Thanks
Athira
> 
> Athira
> 
> 

Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[kajoljain]

On 9/7/23 22:29, Athira Rajeev wrote:
> Testcase "Parsing of all PMU events from sysfs" parse events for
> all PMUs, and not just cpu. In case of powerpc, the PowerVM
> environment supports events from hv_24x7 and hv_gpci PMU which
> is of example format like below:
> 
> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
> - hv_gpci/event,partition_id=?/
> 
> The value for "?" needs to be filled in depending on system
> configuration. It is better to skip these parametrized events
> in this test as it is done in:
> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
> parametrized events")' which handled a simialr instance with
> "all PMU test".
> 
> Fix parse-events test to skip parametrized events since
> it needs proper setup of the parameters.

Patch looks good to me.

Reviewed-by: Kajol Jain <kjain@linux.ibm.com>

Thanks,
Kajol Jain

> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changelog:
> v1 -> v2:
>  Addressed review comments from Ian. Updated size of
>  pmu event name variable and changed bool name which is
>  used to skip the test.
> 
>  tools/perf/tests/parse-events.c | 38 +++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 658fb9599d95..1ecaeceb69f8 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -2514,9 +2514,14 @@ static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest
>  	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
>  		struct stat st;
>  		char path[PATH_MAX];
> +		char pmu_event[PATH_MAX];
> +		char *buf = NULL;
> +		FILE *file;
>  		struct dirent *ent;
> +		size_t len = 0;
>  		DIR *dir;
>  		int err;
> +		int n;
>  
>  		snprintf(path, PATH_MAX, "%s/bus/event_source/devices/%s/events/",
>  			sysfs__mountpoint(), pmu->name);
> @@ -2538,11 +2543,44 @@ static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest
>  			struct evlist_test e = { .name = NULL, };
>  			char name[2 * NAME_MAX + 1 + 12 + 3];
>  			int test_ret;
> +			bool is_event_parameterized = 0;
>  
>  			/* Names containing . are special and cannot be used directly */
>  			if (strchr(ent->d_name, '.'))
>  				continue;
>  
> +			/* exclude parametrized ones (name contains '?') */
> +			n = snprintf(pmu_event, sizeof(pmu_event), "%s%s", path, ent->d_name);
> +			if (n >= PATH_MAX) {
> +				pr_err("pmu event name crossed PATH_MAX(%d) size\n", PATH_MAX);
> +				continue;
> +			}
> +
> +			file = fopen(pmu_event, "r");
> +			if (!file) {
> +				pr_debug("can't open pmu event file for '%s'\n", ent->d_name);
> +				ret = combine_test_results(ret, TEST_FAIL);
> +				continue;
> +			}
> +
> +			if (getline(&buf, &len, file) < 0) {
> +				pr_debug(" pmu event: %s is a null event\n", ent->d_name);
> +				ret = combine_test_results(ret, TEST_FAIL);
> +				continue;
> +			}
> +
> +			if (strchr(buf, '?'))
> +				is_event_parameterized = 1;
> +
> +			free(buf);
> +			buf = NULL;
> +			fclose(file);
> +
> +			if (is_event_parameterized == 1) {
> +				pr_debug("skipping parametrized PMU event: %s which contains ?\n", pmu_event);
> +				continue;
> +			}
> +
>  			snprintf(name, sizeof(name), "%s/event=%s/u", pmu->name, ent->d_name);
>  
>  			e.name  = name;

Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[Namhyung Kim]

Hello,

On Mon, Sep 25, 2023 at 10:37 AM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
>
>
> On Wed, Sep 13, 2023, 7:40 AM Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>>
>>
>>
>> > On 08-Sep-2023, at 7:48 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>> >
>> >
>> >
>> >> On 08-Sep-2023, at 11:04 AM, Sachin Sant <sachinp@linux.ibm.com> wrote:
>> >>
>> >>
>> >>
>> >>> On 07-Sep-2023, at 10:29 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>> >>>
>> >>> Testcase "Parsing of all PMU events from sysfs" parse events for
>> >>> all PMUs, and not just cpu. In case of powerpc, the PowerVM
>> >>> environment supports events from hv_24x7 and hv_gpci PMU which
>> >>> is of example format like below:
>> >>>
>> >>> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
>> >>> - hv_gpci/event,partition_id=?/
>> >>>
>> >>> The value for "?" needs to be filled in depending on system
>> >>> configuration. It is better to skip these parametrized events
>> >>> in this test as it is done in:
>> >>> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
>> >>> parametrized events")' which handled a simialr instance with
>> >>> "all PMU test".
>> >>>
>> >>> Fix parse-events test to skip parametrized events since
>> >>> it needs proper setup of the parameters.
>> >>>
>> >>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> >>> ---
>> >>> Changelog:
>> >>> v1 -> v2:
>> >>> Addressed review comments from Ian. Updated size of
>> >>> pmu event name variable and changed bool name which is
>> >>> used to skip the test.
>> >>>
>> >>
>> >> The patch fixes the reported issue.
>> >>
>> >> 6.2: Parsing of all PMU events from sysfs                          : Ok
>> >> 6.3: Parsing of given PMU events from sysfs                        : Ok
>> >>
>> >> Tested-by: Sachin Sant <sachinp@linux.ibm.com>
>> >>
>> >> - Sachin
>> >
>> > Hi Sachin, Ian
>> >
>> > Thanks for testing the patch
>>
>> Hi Arnaldo
>>
>> Can you please check and pull this if it looks good to go .
>
>
> Namhyung, can you please take a look?

Yep sure.  I think it needs to close the file when getline() fails.

Athira, can you please send v3 with that?

Thanks,
Namhyung

Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events

[Athira Rajeev]

> On 27-Sep-2023, at 4:07 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> 
> Hello,
> 
> On Mon, Sep 25, 2023 at 10:37 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
>> 
>> 
>> 
>> On Wed, Sep 13, 2023, 7:40 AM Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>>> 
>>> 
>>> 
>>>> On 08-Sep-2023, at 7:48 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On 08-Sep-2023, at 11:04 AM, Sachin Sant <sachinp@linux.ibm.com> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>>> On 07-Sep-2023, at 10:29 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>>>>>> 
>>>>>> Testcase "Parsing of all PMU events from sysfs" parse events for
>>>>>> all PMUs, and not just cpu. In case of powerpc, the PowerVM
>>>>>> environment supports events from hv_24x7 and hv_gpci PMU which
>>>>>> is of example format like below:
>>>>>> 
>>>>>> - hv_24x7/CPM_ADJUNCT_INST,domain=?,core=?/
>>>>>> - hv_gpci/event,partition_id=?/
>>>>>> 
>>>>>> The value for "?" needs to be filled in depending on system
>>>>>> configuration. It is better to skip these parametrized events
>>>>>> in this test as it is done in:
>>>>>> 'commit b50d691e50e6 ("perf test: Fix "all PMU test" to skip
>>>>>> parametrized events")' which handled a simialr instance with
>>>>>> "all PMU test".
>>>>>> 
>>>>>> Fix parse-events test to skip parametrized events since
>>>>>> it needs proper setup of the parameters.
>>>>>> 
>>>>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> Changelog:
>>>>>> v1 -> v2:
>>>>>> Addressed review comments from Ian. Updated size of
>>>>>> pmu event name variable and changed bool name which is
>>>>>> used to skip the test.
>>>>>> 
>>>>> 
>>>>> The patch fixes the reported issue.
>>>>> 
>>>>> 6.2: Parsing of all PMU events from sysfs                          : Ok
>>>>> 6.3: Parsing of given PMU events from sysfs                        : Ok
>>>>> 
>>>>> Tested-by: Sachin Sant <sachinp@linux.ibm.com>
>>>>> 
>>>>> - Sachin
>>>> 
>>>> Hi Sachin, Ian
>>>> 
>>>> Thanks for testing the patch
>>> 
>>> Hi Arnaldo
>>> 
>>> Can you please check and pull this if it looks good to go .
>> 
>> 
>> Namhyung, can you please take a look?
> 
> Yep sure.  I think it needs to close the file when getline() fails.
> 
> Athira, can you please send v3 with that?

Sure, I will post V3 with this change

Athira
> 
> Thanks,
> Namhyung