Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events
> On 27-Sep-2023, at 4:07 AM, Namhyung Kim wrote:
>
> Hello,
>
> On Mon, Sep 25, 2023 at 10:37 AM Arnaldo Carvalho de Melo
> wrote:
>>
>>
>>
>> On Wed, Sep 13, 2023, 7:40 AM Athira Rajeev
>> wrote:
>>>
>>>
>>>
On 08-Sep-2023, at 7:48 PM, Athira Rajeev
wrote:
> On 08-Sep-2023, at 11:04 AM, Sachin Sant wrote:
>
>
>
>> On 07-Sep-2023, at 10:29 PM, 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.
>>
>> Signed-off-by: Athira Rajeev
>> ---
>> 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
>
> - 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
Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events
Hello,
On Mon, Sep 25, 2023 at 10:37 AM Arnaldo Carvalho de Melo
wrote:
>
>
>
> On Wed, Sep 13, 2023, 7:40 AM Athira Rajeev
> wrote:
>>
>>
>>
>> > On 08-Sep-2023, at 7:48 PM, Athira Rajeev
>> > wrote:
>> >
>> >
>> >
>> >> On 08-Sep-2023, at 11:04 AM, Sachin Sant wrote:
>> >>
>> >>
>> >>
>> >>> On 07-Sep-2023, at 10:29 PM, 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.
>> >>>
>> >>> Signed-off-by: Athira Rajeev
>> >>> ---
>> >>> 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
>> >>
>> >> - 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
On Wed, Sep 13, 2023, 7:40 AM Athira Rajeev
wrote:
>
>
> > On 08-Sep-2023, at 7:48 PM, Athira Rajeev
> wrote:
> >
> >
> >
> >> On 08-Sep-2023, at 11:04 AM, Sachin Sant wrote:
> >>
> >>
> >>
> >>> On 07-Sep-2023, at 10:29 PM, Athira Rajeev <
> atraj...@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
> >>> ---
> >>> 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
> >>
> >> - 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?
I'll be back home next week, now at Kernel Recipes in Paris.
- Arnaldo
>
> Thanks
> Athira
> >
> > Athira
> >
> >
>
>
Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events
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
Thanks,
Kajol Jain
>
> Signed-off-by: Athira Rajeev
> ---
> 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(, , 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
> On 08-Sep-2023, at 7:48 PM, Athira Rajeev wrote:
>
>
>
>> On 08-Sep-2023, at 11:04 AM, Sachin Sant wrote:
>>
>>
>>
>>> On 07-Sep-2023, at 10:29 PM, 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.
>>>
>>> Signed-off-by: Athira Rajeev
>>> ---
>>> 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
>>
>> - 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
> On 08-Sep-2023, at 11:04 AM, Sachin Sant wrote:
>
>
>
>> On 07-Sep-2023, at 10:29 PM, 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.
>>
>> Signed-off-by: Athira Rajeev
>> ---
>> 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
>
> - Sachin
Hi Sachin, Ian
Thanks for testing the patch
Athira
Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events
> On 07-Sep-2023, at 10:29 PM, 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.
>
> Signed-off-by: Athira Rajeev
> ---
> 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
- Sachin
Re: [PATCH V2] perf test: Fix parse-events tests to skip parametrized events
On Thu, Sep 7, 2023 at 9:59 AM 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.
>
> Signed-off-by: Athira Rajeev
Tested-by: Ian Rogers
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(, , 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
>
[PATCH V2] perf test: Fix parse-events tests to skip parametrized events
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
---
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(, , 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
