Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
> how about recognizing attribute based on the '.' prefix being > existing file rather than the suffix like in the attached patch Fine too. My patch is simpler and works well enough though, and also handles other cases. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Tue, Aug 11, 2015 at 06:40:27AM -0700, Andi Kleen wrote: > On Tue, Aug 11, 2015 at 03:24:27PM +0200, Jiri Olsa wrote: > > On Tue, Aug 11, 2015 at 06:14:57AM -0700, Andi Kleen wrote: > > > > Which attribute parsing is failing for you? > > > > > > The new .agg-per-core attribute I added later in the series. > > > I think it will happen to any not-yet-known attribute. > > > > alias can contain only terms defined in formats directory, > > and the *.XXX attributes parsing does not return error code > > > > can't see the failure, please get some example > > Apply the kernel patch that adds several .agg-per-core attributes > Then try to use any cpu/.../ event > > % perf stat -e cpu/event=0x3c/ true > invalid or unsupported event: 'cpu/event=0x3c/' > > because the PMU parsing bailed out. ugh right, the new attribute wont be recognized.. how about recognizing attribute based on the '.' prefix being existing file rather than the suffix like in the attached patch jirka --- diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index d4b0e6454bc6..937ecc35a60e 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -258,21 +258,23 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI return __perf_pmu__new_alias(list, dir, name, NULL, buf); } -static inline bool pmu_alias_info_file(char *name) +static inline bool pmu_alias_attr_file(char *dir, char *name) { - size_t len; - - len = strlen(name); - if (len > 5 && !strcmp(name + len - 5, ".unit")) - return true; - if (len > 6 && !strcmp(name + len - 6, ".scale")) - return true; - if (len > 8 && !strcmp(name + len - 8, ".per-pkg")) - return true; - if (len > 9 && !strcmp(name + len - 9, ".snapshot")) - return true; + bool ret = false; + struct stat st; + char *path, *s; - return false; + if (asprintf(, "%s/%s", dir, name) == -1) + return false; + + s = strrchr(path, '.'); + if (s) { + *s = 0; + ret = !stat(path, ); + } + + free(path); + return ret; } /* @@ -300,7 +302,7 @@ static int pmu_aliases_parse(char *dir, struct list_head *head) /* * skip info files parsed in perf_pmu__new_alias() */ - if (pmu_alias_info_file(name)) + if (pmu_alias_attr_file(dir, name)) continue; snprintf(path, PATH_MAX, "%s/%s", dir, name); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Tue, Aug 11, 2015 at 03:24:27PM +0200, Jiri Olsa wrote: > On Tue, Aug 11, 2015 at 06:14:57AM -0700, Andi Kleen wrote: > > > Which attribute parsing is failing for you? > > > > The new .agg-per-core attribute I added later in the series. > > I think it will happen to any not-yet-known attribute. > > alias can contain only terms defined in formats directory, > and the *.XXX attributes parsing does not return error code > > can't see the failure, please get some example Apply the kernel patch that adds several .agg-per-core attributes Then try to use any cpu/.../ event % perf stat -e cpu/event=0x3c/ true invalid or unsupported event: 'cpu/event=0x3c/' because the PMU parsing bailed out. With patched perf (either this patch or the patch that adds the .agg-per-core parsing) it works. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Tue, Aug 11, 2015 at 06:14:57AM -0700, Andi Kleen wrote: > > Which attribute parsing is failing for you? > > The new .agg-per-core attribute I added later in the series. > I think it will happen to any not-yet-known attribute. alias can contain only terms defined in formats directory, and the *.XXX attributes parsing does not return error code can't see the failure, please get some example jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
> Which attribute parsing is failing for you? The new .agg-per-core attribute I added later in the series. I think it will happen to any not-yet-known attribute. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Fri, Aug 07, 2015 at 06:06:17PM -0700, Andi Kleen wrote: > From: Andi Kleen > > When an error happens during alias parsing currently the complete > parsing of all attributes of the PMU is stopped. This is breaks > old perf on a newer kernel that may have not-yet-know > alias attributes (such as .scale or .per-pkg). hum, both .scale and .per-pgk are skip from term parsing via: /* * skip info files parsed in perf_pmu__new_alias() */ if (pmu_alias_info_file(name)) continue; and loaded without any error report: static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name, char *desc __maybe_unused, char *val) SNIP if (dir) { /* * load unit name and scale if available */ perf_pmu__parse_unit(alias, dir, name); perf_pmu__parse_scale(alias, dir, name); perf_pmu__parse_per_pkg(alias, dir, name); perf_pmu__parse_snapshot(alias, dir, name); } list_add_tail(>list, list); return 0; } Which attribute parsing is failing for you? thanks, jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Fri, Aug 07, 2015 at 06:06:17PM -0700, Andi Kleen wrote: From: Andi Kleen a...@linux.intel.com When an error happens during alias parsing currently the complete parsing of all attributes of the PMU is stopped. This is breaks old perf on a newer kernel that may have not-yet-know alias attributes (such as .scale or .per-pkg). hum, both .scale and .per-pgk are skip from term parsing via: /* * skip info files parsed in perf_pmu__new_alias() */ if (pmu_alias_info_file(name)) continue; and loaded without any error report: static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name, char *desc __maybe_unused, char *val) SNIP if (dir) { /* * load unit name and scale if available */ perf_pmu__parse_unit(alias, dir, name); perf_pmu__parse_scale(alias, dir, name); perf_pmu__parse_per_pkg(alias, dir, name); perf_pmu__parse_snapshot(alias, dir, name); } list_add_tail(alias-list, list); return 0; } Which attribute parsing is failing for you? thanks, jirka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
Which attribute parsing is failing for you? The new .agg-per-core attribute I added later in the series. I think it will happen to any not-yet-known attribute. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Tue, Aug 11, 2015 at 03:24:27PM +0200, Jiri Olsa wrote: On Tue, Aug 11, 2015 at 06:14:57AM -0700, Andi Kleen wrote: Which attribute parsing is failing for you? The new .agg-per-core attribute I added later in the series. I think it will happen to any not-yet-known attribute. alias can contain only terms defined in formats directory, and the *.XXX attributes parsing does not return error code can't see the failure, please get some example Apply the kernel patch that adds several .agg-per-core attributes Then try to use any cpu/.../ event % perf stat -e cpu/event=0x3c/ true invalid or unsupported event: 'cpu/event=0x3c/' because the PMU parsing bailed out. With patched perf (either this patch or the patch that adds the .agg-per-core parsing) it works. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Tue, Aug 11, 2015 at 06:40:27AM -0700, Andi Kleen wrote: On Tue, Aug 11, 2015 at 03:24:27PM +0200, Jiri Olsa wrote: On Tue, Aug 11, 2015 at 06:14:57AM -0700, Andi Kleen wrote: Which attribute parsing is failing for you? The new .agg-per-core attribute I added later in the series. I think it will happen to any not-yet-known attribute. alias can contain only terms defined in formats directory, and the *.XXX attributes parsing does not return error code can't see the failure, please get some example Apply the kernel patch that adds several .agg-per-core attributes Then try to use any cpu/.../ event % perf stat -e cpu/event=0x3c/ true invalid or unsupported event: 'cpu/event=0x3c/' because the PMU parsing bailed out. ugh right, the new attribute wont be recognized.. how about recognizing attribute based on the '.' prefix being existing file rather than the suffix like in the attached patch jirka --- diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index d4b0e6454bc6..937ecc35a60e 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -258,21 +258,23 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI return __perf_pmu__new_alias(list, dir, name, NULL, buf); } -static inline bool pmu_alias_info_file(char *name) +static inline bool pmu_alias_attr_file(char *dir, char *name) { - size_t len; - - len = strlen(name); - if (len 5 !strcmp(name + len - 5, .unit)) - return true; - if (len 6 !strcmp(name + len - 6, .scale)) - return true; - if (len 8 !strcmp(name + len - 8, .per-pkg)) - return true; - if (len 9 !strcmp(name + len - 9, .snapshot)) - return true; + bool ret = false; + struct stat st; + char *path, *s; - return false; + if (asprintf(path, %s/%s, dir, name) == -1) + return false; + + s = strrchr(path, '.'); + if (s) { + *s = 0; + ret = !stat(path, st); + } + + free(path); + return ret; } /* @@ -300,7 +302,7 @@ static int pmu_aliases_parse(char *dir, struct list_head *head) /* * skip info files parsed in perf_pmu__new_alias() */ - if (pmu_alias_info_file(name)) + if (pmu_alias_attr_file(dir, name)) continue; snprintf(path, PATH_MAX, %s/%s, dir, name); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
On Tue, Aug 11, 2015 at 06:14:57AM -0700, Andi Kleen wrote: Which attribute parsing is failing for you? The new .agg-per-core attribute I added later in the series. I think it will happen to any not-yet-known attribute. alias can contain only terms defined in formats directory, and the *.XXX attributes parsing does not return error code can't see the failure, please get some example jirka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
how about recognizing attribute based on the '.' prefix being existing file rather than the suffix like in the attached patch Fine too. My patch is simpler and works well enough though, and also handles other cases. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
From: Andi Kleen When an error happens during alias parsing currently the complete parsing of all attributes of the PMU is stopped. This is breaks old perf on a newer kernel that may have not-yet-know alias attributes (such as .scale or .per-pkg). Continue when some attribute is unparseable. This is IMHO a stable candidate and should be backported to older versions to avoid problems with newer kernels. Signed-off-by: Andi Kleen --- tools/perf/util/pmu.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index d4b0e64..ce56354 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -283,13 +283,12 @@ static int pmu_aliases_parse(char *dir, struct list_head *head) { struct dirent *evt_ent; DIR *event_dir; - int ret = 0; event_dir = opendir(dir); if (!event_dir) return -EINVAL; - while (!ret && (evt_ent = readdir(event_dir))) { + while ((evt_ent = readdir(event_dir))) { char path[PATH_MAX]; char *name = evt_ent->d_name; FILE *file; @@ -305,17 +304,16 @@ static int pmu_aliases_parse(char *dir, struct list_head *head) snprintf(path, PATH_MAX, "%s/%s", dir, name); - ret = -EINVAL; file = fopen(path, "r"); if (!file) - break; + continue; - ret = perf_pmu__new_alias(head, dir, name, file); + perf_pmu__new_alias(head, dir, name, file); fclose(file); } closedir(event_dir); - return ret; + return 0; } /* -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error
From: Andi Kleen a...@linux.intel.com When an error happens during alias parsing currently the complete parsing of all attributes of the PMU is stopped. This is breaks old perf on a newer kernel that may have not-yet-know alias attributes (such as .scale or .per-pkg). Continue when some attribute is unparseable. This is IMHO a stable candidate and should be backported to older versions to avoid problems with newer kernels. Signed-off-by: Andi Kleen a...@linux.intel.com --- tools/perf/util/pmu.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index d4b0e64..ce56354 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -283,13 +283,12 @@ static int pmu_aliases_parse(char *dir, struct list_head *head) { struct dirent *evt_ent; DIR *event_dir; - int ret = 0; event_dir = opendir(dir); if (!event_dir) return -EINVAL; - while (!ret (evt_ent = readdir(event_dir))) { + while ((evt_ent = readdir(event_dir))) { char path[PATH_MAX]; char *name = evt_ent-d_name; FILE *file; @@ -305,17 +304,16 @@ static int pmu_aliases_parse(char *dir, struct list_head *head) snprintf(path, PATH_MAX, %s/%s, dir, name); - ret = -EINVAL; file = fopen(path, r); if (!file) - break; + continue; - ret = perf_pmu__new_alias(head, dir, name, file); + perf_pmu__new_alias(head, dir, name, file); fclose(file); } closedir(event_dir); - return ret; + return 0; } /* -- 2.4.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/