Re: [PATCH 1/9] perf, tools: Dont stop PMU parsing on alias parse error

2015-08-11 Thread Andi Kleen
> 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

2015-08-11 Thread Jiri Olsa
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

2015-08-11 Thread Andi Kleen
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

2015-08-11 Thread Jiri Olsa
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

2015-08-11 Thread Andi Kleen
> 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

2015-08-11 Thread Jiri Olsa
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

2015-08-11 Thread Jiri Olsa
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

2015-08-11 Thread Andi Kleen
 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

2015-08-11 Thread Andi Kleen
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

2015-08-11 Thread Jiri Olsa
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

2015-08-11 Thread Jiri Olsa
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

2015-08-11 Thread Andi Kleen
 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

2015-08-07 Thread Andi Kleen
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

2015-08-07 Thread Andi Kleen
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/