* [PATCH v2 1/2] perf jevents: Tidy error handling
2020-10-22 11:02 [PATCH v2 0/2] perf PMU events test: Add scenario for arch std events John Garry
@ 2020-10-22 11:02 ` John Garry
2020-10-23 7:49 ` kajoljain
2020-10-22 11:02 ` [PATCH v2 2/2] perf jevents: Add test for arch std events John Garry
2020-10-27 16:34 ` [PATCH v2 0/2] perf PMU events test: Add scenario " Arnaldo Carvalho de Melo
2 siblings, 1 reply; 6+ messages in thread
From: John Garry @ 2020-10-22 11:02 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, kjain, irogers, yao.jin, yeyunfeng
Cc: linux-kernel, linuxarm
There is much duplication in the error handling for directory transvering
for prcessing JSONs.
Factor out the common code to tidy a bit.
Signed-off-by: John Garry <john.garry@huawei.com>
---
tools/perf/pmu-events/jevents.c | 83 ++++++++++++++-------------------
1 file changed, 35 insertions(+), 48 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index e47644cab3fa..7326c14c4623 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -1100,12 +1100,13 @@ static int process_one_file(const char *fpath, const struct stat *sb,
*/
int main(int argc, char *argv[])
{
- int rc, ret = 0;
+ int rc, ret = 0, empty_map = 0;
int maxfds;
char ldirname[PATH_MAX];
const char *arch;
const char *output_file;
const char *start_dirname;
+ char *err_string_ext = "";
struct stat stbuf;
prog = basename(argv[0]);
@@ -1133,7 +1134,8 @@ int main(int argc, char *argv[])
/* If architecture does not have any event lists, bail out */
if (stat(ldirname, &stbuf) < 0) {
pr_info("%s: Arch %s has no PMU event lists\n", prog, arch);
- goto empty_map;
+ empty_map = 1;
+ goto err_close_eventsfp;
}
/* Include pmu-events.h first */
@@ -1150,75 +1152,60 @@ int main(int argc, char *argv[])
*/
maxfds = get_maxfds();
- mapfile = NULL;
rc = nftw(ldirname, preprocess_arch_std_files, maxfds, 0);
- if (rc && verbose) {
- pr_info("%s: Error preprocessing arch standard files %s\n",
- prog, ldirname);
- goto empty_map;
- } else if (rc < 0) {
- /* Make build fail */
- fclose(eventsfp);
- free_arch_std_events();
- return 1;
- } else if (rc) {
- goto empty_map;
- }
+ if (rc)
+ goto err_processing_std_arch_event_dir;
rc = nftw(ldirname, process_one_file, maxfds, 0);
- if (rc && verbose) {
- pr_info("%s: Error walking file tree %s\n", prog, ldirname);
- goto empty_map;
- } else if (rc < 0) {
- /* Make build fail */
- fclose(eventsfp);
- free_arch_std_events();
- ret = 1;
- goto out_free_mapfile;
- } else if (rc) {
- goto empty_map;
- }
+ if (rc)
+ goto err_processing_dir;
sprintf(ldirname, "%s/test", start_dirname);
rc = nftw(ldirname, process_one_file, maxfds, 0);
- if (rc && verbose) {
- pr_info("%s: Error walking file tree %s rc=%d for test\n",
- prog, ldirname, rc);
- goto empty_map;
- } else if (rc < 0) {
- /* Make build fail */
- free_arch_std_events();
- ret = 1;
- goto out_free_mapfile;
- } else if (rc) {
- goto empty_map;
- }
+ if (rc)
+ goto err_processing_dir;
if (close_table)
print_events_table_suffix(eventsfp);
if (!mapfile) {
pr_info("%s: No CPU->JSON mapping?\n", prog);
- goto empty_map;
+ empty_map = 1;
+ goto err_close_eventsfp;
}
- if (process_mapfile(eventsfp, mapfile)) {
+ rc = process_mapfile(eventsfp, mapfile);
+ fclose(eventsfp);
+ if (rc) {
pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
/* Make build fail */
- fclose(eventsfp);
- free_arch_std_events();
ret = 1;
+ goto err_out;
}
+ free_arch_std_events();
+ free(mapfile);
+ return 0;
- goto out_free_mapfile;
-
-empty_map:
+err_processing_std_arch_event_dir:
+ err_string_ext = " for std arch event";
+err_processing_dir:
+ if (verbose) {
+ pr_info("%s: Error walking file tree %s%s\n", prog, ldirname,
+ err_string_ext);
+ empty_map = 1;
+ } else if (rc < 0) {
+ ret = 1;
+ } else {
+ empty_map = 1;
+ }
+err_close_eventsfp:
fclose(eventsfp);
- create_empty_mapping(output_file);
+ if (empty_map)
+ create_empty_mapping(output_file);
+err_out:
free_arch_std_events();
-out_free_mapfile:
free(mapfile);
return ret;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] perf jevents: Tidy error handling
2020-10-22 11:02 ` [PATCH v2 1/2] perf jevents: Tidy error handling John Garry
@ 2020-10-23 7:49 ` kajoljain
0 siblings, 0 replies; 6+ messages in thread
From: kajoljain @ 2020-10-23 7:49 UTC (permalink / raw)
To: John Garry, peterz, mingo, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, yao.jin, yeyunfeng
Cc: linux-kernel, linuxarm
On 10/22/20 4:32 PM, John Garry wrote:
> There is much duplication in the error handling for directory transvering
> for prcessing JSONs.
>
> Factor out the common code to tidy a bit.
>
Patch looks good to me.
Reviewed-By: Kajol Jain<kjain@linux.ibm.com>
Thanks,
Kajol Jain
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
> tools/perf/pmu-events/jevents.c | 83 ++++++++++++++-------------------
> 1 file changed, 35 insertions(+), 48 deletions(-)
>
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index e47644cab3fa..7326c14c4623 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -1100,12 +1100,13 @@ static int process_one_file(const char *fpath, const struct stat *sb,
> */
> int main(int argc, char *argv[])
> {
> - int rc, ret = 0;
> + int rc, ret = 0, empty_map = 0;
> int maxfds;
> char ldirname[PATH_MAX];
> const char *arch;
> const char *output_file;
> const char *start_dirname;
> + char *err_string_ext = "";
> struct stat stbuf;
>
> prog = basename(argv[0]);
> @@ -1133,7 +1134,8 @@ int main(int argc, char *argv[])
> /* If architecture does not have any event lists, bail out */
> if (stat(ldirname, &stbuf) < 0) {
> pr_info("%s: Arch %s has no PMU event lists\n", prog, arch);
> - goto empty_map;
> + empty_map = 1;
> + goto err_close_eventsfp;
> }
>
> /* Include pmu-events.h first */
> @@ -1150,75 +1152,60 @@ int main(int argc, char *argv[])
> */
>
> maxfds = get_maxfds();
> - mapfile = NULL;
> rc = nftw(ldirname, preprocess_arch_std_files, maxfds, 0);
> - if (rc && verbose) {
> - pr_info("%s: Error preprocessing arch standard files %s\n",
> - prog, ldirname);
> - goto empty_map;
> - } else if (rc < 0) {
> - /* Make build fail */
> - fclose(eventsfp);
> - free_arch_std_events();
> - return 1;
> - } else if (rc) {
> - goto empty_map;
> - }
> + if (rc)
> + goto err_processing_std_arch_event_dir;
>
> rc = nftw(ldirname, process_one_file, maxfds, 0);
> - if (rc && verbose) {
> - pr_info("%s: Error walking file tree %s\n", prog, ldirname);
> - goto empty_map;
> - } else if (rc < 0) {
> - /* Make build fail */
> - fclose(eventsfp);
> - free_arch_std_events();
> - ret = 1;
> - goto out_free_mapfile;
> - } else if (rc) {
> - goto empty_map;
> - }
> + if (rc)
> + goto err_processing_dir;
>
> sprintf(ldirname, "%s/test", start_dirname);
>
> rc = nftw(ldirname, process_one_file, maxfds, 0);
> - if (rc && verbose) {
> - pr_info("%s: Error walking file tree %s rc=%d for test\n",
> - prog, ldirname, rc);
> - goto empty_map;
> - } else if (rc < 0) {
> - /* Make build fail */
> - free_arch_std_events();
> - ret = 1;
> - goto out_free_mapfile;
> - } else if (rc) {
> - goto empty_map;
> - }
> + if (rc)
> + goto err_processing_dir;
>
> if (close_table)
> print_events_table_suffix(eventsfp);
>
> if (!mapfile) {
> pr_info("%s: No CPU->JSON mapping?\n", prog);
> - goto empty_map;
> + empty_map = 1;
> + goto err_close_eventsfp;
> }
>
> - if (process_mapfile(eventsfp, mapfile)) {
> + rc = process_mapfile(eventsfp, mapfile);
> + fclose(eventsfp);
> + if (rc) {
> pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
> /* Make build fail */
> - fclose(eventsfp);
> - free_arch_std_events();
> ret = 1;
> + goto err_out;
> }
>
> + free_arch_std_events();
> + free(mapfile);
> + return 0;
>
> - goto out_free_mapfile;
> -
> -empty_map:
> +err_processing_std_arch_event_dir:
> + err_string_ext = " for std arch event";
> +err_processing_dir:
> + if (verbose) {
> + pr_info("%s: Error walking file tree %s%s\n", prog, ldirname,
> + err_string_ext);
> + empty_map = 1;
> + } else if (rc < 0) {
> + ret = 1;
> + } else {
> + empty_map = 1;
> + }
> +err_close_eventsfp:
> fclose(eventsfp);
> - create_empty_mapping(output_file);
> + if (empty_map)
> + create_empty_mapping(output_file);
> +err_out:
> free_arch_std_events();
> -out_free_mapfile:
> free(mapfile);
> return ret;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] perf jevents: Add test for arch std events
2020-10-22 11:02 [PATCH v2 0/2] perf PMU events test: Add scenario for arch std events John Garry
2020-10-22 11:02 ` [PATCH v2 1/2] perf jevents: Tidy error handling John Garry
@ 2020-10-22 11:02 ` John Garry
2020-10-23 7:57 ` kajoljain
2020-10-27 16:34 ` [PATCH v2 0/2] perf PMU events test: Add scenario " Arnaldo Carvalho de Melo
2 siblings, 1 reply; 6+ messages in thread
From: John Garry @ 2020-10-22 11:02 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, kjain, irogers, yao.jin, yeyunfeng
Cc: linux-kernel, linuxarm
Recently there was an undetected breakage for std arch event support.
Add support in "PMU events" testcase to detect such breakages.
For this, the "test" arch needs has support added to process std arch
events. And a test event is added for the test, ifself.
Also add a few code comments to help understand the code a bit better.
Signed-off-by: John Garry <john.garry@huawei.com>
---
.../perf/pmu-events/arch/test/arch-std-events.json | 8 ++++++++
.../perf/pmu-events/arch/test/test_cpu/cache.json | 5 +++++
tools/perf/pmu-events/jevents.c | 4 ++++
tools/perf/tests/pmu-events.c | 14 ++++++++++++++
4 files changed, 31 insertions(+)
create mode 100644 tools/perf/pmu-events/arch/test/arch-std-events.json
create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/cache.json
diff --git a/tools/perf/pmu-events/arch/test/arch-std-events.json b/tools/perf/pmu-events/arch/test/arch-std-events.json
new file mode 100644
index 000000000000..43f6f729d6ae
--- /dev/null
+++ b/tools/perf/pmu-events/arch/test/arch-std-events.json
@@ -0,0 +1,8 @@
+[
+ {
+ "PublicDescription": "Attributable Level 3 cache access, read",
+ "EventCode": "0x40",
+ "EventName": "L3_CACHE_RD",
+ "BriefDescription": "L3 cache access, read"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/test/test_cpu/cache.json b/tools/perf/pmu-events/arch/test/test_cpu/cache.json
new file mode 100644
index 000000000000..036d0efdb2bb
--- /dev/null
+++ b/tools/perf/pmu-events/arch/test/test_cpu/cache.json
@@ -0,0 +1,5 @@
+[
+ {
+ "ArchStdEvent": "L3_CACHE_RD"
+ }
+]
\ No newline at end of file
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 7326c14c4623..72cfa3b5046d 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -1162,6 +1162,10 @@ int main(int argc, char *argv[])
sprintf(ldirname, "%s/test", start_dirname);
+ rc = nftw(ldirname, preprocess_arch_std_files, maxfds, 0);
+ if (rc)
+ goto err_processing_std_arch_event_dir;
+
rc = nftw(ldirname, process_one_file, maxfds, 0);
if (rc)
goto err_processing_dir;
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index d3517a74d95e..ad2b21591275 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -14,8 +14,10 @@
#include "util/parse-events.h"
struct perf_pmu_test_event {
+ /* used for matching against events from generated pmu-events.c */
struct pmu_event event;
+ /* used for matching against event aliases */
/* extra events for aliases */
const char *alias_str;
@@ -78,6 +80,17 @@ static struct perf_pmu_test_event test_cpu_events[] = {
.alias_str = "umask=0,(null)=0x30d40,event=0x3a",
.alias_long_desc = "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions",
},
+ {
+ .event = {
+ .name = "l3_cache_rd",
+ .event = "event=0x40",
+ .desc = "L3 cache access, read",
+ .long_desc = "Attributable Level 3 cache access, read",
+ .topic = "cache",
+ },
+ .alias_str = "event=0x40",
+ .alias_long_desc = "Attributable Level 3 cache access, read",
+ },
{ /* sentinel */
.event = {
.name = NULL,
@@ -357,6 +370,7 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
}
+/* Test that aliases generated are as expected */
static int test_aliases(void)
{
struct perf_pmu *pmu = NULL;
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] perf jevents: Add test for arch std events
2020-10-22 11:02 ` [PATCH v2 2/2] perf jevents: Add test for arch std events John Garry
@ 2020-10-23 7:57 ` kajoljain
0 siblings, 0 replies; 6+ messages in thread
From: kajoljain @ 2020-10-23 7:57 UTC (permalink / raw)
To: John Garry, peterz, mingo, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, yao.jin, yeyunfeng
Cc: linux-kernel, linuxarm
On 10/22/20 4:32 PM, John Garry wrote:
> Recently there was an undetected breakage for std arch event support.
>
> Add support in "PMU events" testcase to detect such breakages.
>
> For this, the "test" arch needs has support added to process std arch
> events. And a test event is added for the test, ifself.
>
> Also add a few code comments to help understand the code a bit better.
Patch looks good to me.
Reviewed-By: Kajol Jain<kjain@linux.ibm.com>
Thanks,
Kajol Jain
>
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
> .../perf/pmu-events/arch/test/arch-std-events.json | 8 ++++++++
> .../perf/pmu-events/arch/test/test_cpu/cache.json | 5 +++++
> tools/perf/pmu-events/jevents.c | 4 ++++
> tools/perf/tests/pmu-events.c | 14 ++++++++++++++
> 4 files changed, 31 insertions(+)
> create mode 100644 tools/perf/pmu-events/arch/test/arch-std-events.json
> create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/cache.json
>
> diff --git a/tools/perf/pmu-events/arch/test/arch-std-events.json b/tools/perf/pmu-events/arch/test/arch-std-events.json
> new file mode 100644
> index 000000000000..43f6f729d6ae
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/test/arch-std-events.json
> @@ -0,0 +1,8 @@
> +[
> + {
> + "PublicDescription": "Attributable Level 3 cache access, read",
> + "EventCode": "0x40",
> + "EventName": "L3_CACHE_RD",
> + "BriefDescription": "L3 cache access, read"
> + }
> +]
> diff --git a/tools/perf/pmu-events/arch/test/test_cpu/cache.json b/tools/perf/pmu-events/arch/test/test_cpu/cache.json
> new file mode 100644
> index 000000000000..036d0efdb2bb
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/test/test_cpu/cache.json
> @@ -0,0 +1,5 @@
> +[
> + {
> + "ArchStdEvent": "L3_CACHE_RD"
> + }
> +]
> \ No newline at end of file
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 7326c14c4623..72cfa3b5046d 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -1162,6 +1162,10 @@ int main(int argc, char *argv[])
>
> sprintf(ldirname, "%s/test", start_dirname);
>
> + rc = nftw(ldirname, preprocess_arch_std_files, maxfds, 0);
> + if (rc)
> + goto err_processing_std_arch_event_dir;
> +
> rc = nftw(ldirname, process_one_file, maxfds, 0);
> if (rc)
> goto err_processing_dir;
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index d3517a74d95e..ad2b21591275 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -14,8 +14,10 @@
> #include "util/parse-events.h"
>
> struct perf_pmu_test_event {
> + /* used for matching against events from generated pmu-events.c */
> struct pmu_event event;
>
> + /* used for matching against event aliases */
> /* extra events for aliases */
> const char *alias_str;
>
> @@ -78,6 +80,17 @@ static struct perf_pmu_test_event test_cpu_events[] = {
> .alias_str = "umask=0,(null)=0x30d40,event=0x3a",
> .alias_long_desc = "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions",
> },
> + {
> + .event = {
> + .name = "l3_cache_rd",
> + .event = "event=0x40",
> + .desc = "L3 cache access, read",
> + .long_desc = "Attributable Level 3 cache access, read",
> + .topic = "cache",
> + },
> + .alias_str = "event=0x40",
> + .alias_long_desc = "Attributable Level 3 cache access, read",
> + },
> { /* sentinel */
> .event = {
> .name = NULL,
> @@ -357,6 +370,7 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
> }
>
>
> +/* Test that aliases generated are as expected */
> static int test_aliases(void)
> {
> struct perf_pmu *pmu = NULL;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] perf PMU events test: Add scenario for arch std events
2020-10-22 11:02 [PATCH v2 0/2] perf PMU events test: Add scenario for arch std events John Garry
2020-10-22 11:02 ` [PATCH v2 1/2] perf jevents: Tidy error handling John Garry
2020-10-22 11:02 ` [PATCH v2 2/2] perf jevents: Add test for arch std events John Garry
@ 2020-10-27 16:34 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-27 16:34 UTC (permalink / raw)
To: John Garry
Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
kjain, irogers, yao.jin, yeyunfeng, linux-kernel, linuxarm
Em Thu, Oct 22, 2020 at 07:02:25PM +0800, John Garry escreveu:
> The small series covers the following:
> - Tidy error handling in jevents a bit
> - Expands on PMU events test to cover jevents arch std events support
>
> Differences to v1:
> - Revert to original logic in jevents.c error path
Thanks, applied both together with Kajol's Reviewed-by tags.
- Arnaldo
> John Garry (2):
> perf jevents: Tidy error handling
> perf jevents: Add test for arch std events
>
> .../pmu-events/arch/test/arch-std-events.json | 8 ++
> .../pmu-events/arch/test/test_cpu/cache.json | 5 ++
> tools/perf/pmu-events/jevents.c | 87 +++++++++----------
> tools/perf/tests/pmu-events.c | 14 +++
> 4 files changed, 66 insertions(+), 48 deletions(-)
> create mode 100644 tools/perf/pmu-events/arch/test/arch-std-events.json
> create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/cache.json
>
> --
> 2.26.2
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread