Re: [PATCH 02/14] perf util: Use evsel->name to get tracepoint_paths

2013-04-24 Thread Namhyung Kim 
Hi Jiri,

On Wed, 24 Apr 2013 14:42:57 +0200, Jiri Olsa wrote:
> On Tue, Apr 23, 2013 at 05:31:00PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim 
>> 
>> Most tracepoint events already have their system and event name in
>> ->name field so that searching whole event tracing directory for each
>> evsel to match given id is suboptimal.
>> 
>> Cc: Jiri Olsa 
>> Cc: Frederic Weisbecker 
>> Signed-off-by: Namhyung Kim 
>> ---
>>  tools/perf/util/trace-event-info.c | 19 +++
>>  1 file changed, 19 insertions(+)
>> 
>> diff --git a/tools/perf/util/trace-event-info.c 
>> b/tools/perf/util/trace-event-info.c
>> index ab18bf12d54a..dcfc1869c9af 100644
>> --- a/tools/perf/util/trace-event-info.c
>> +++ b/tools/perf/util/trace-event-info.c
>> @@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
>>  if (pos->attr.type != PERF_TYPE_TRACEPOINT)
>>  continue;
>>  ++nr_tracepoints;
>> +
>> +if (pos->name && strchr(pos->name, ':')) {
>> +char *str = strchr(pos->name, ':');
>> +
>> +ppath->next = zalloc(sizeof(path));
>> +if (!ppath->next)
>> +goto error;
>> +
>> +ppath->next->system = strndup(pos->name, str - 
>> pos->name);
>> +ppath->next->name = strdup(str+1);
>> +
>> +if (!ppath->next->system || !ppath->next->name)
>> +goto error;
>> +
>> +goto next;
>> +}
>
> good idea, could you please move this ^^^ into function ?
> something like 'tracepoint_name_to_path'

Will do!

Thanks,
Namhyung
--
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 02/14] perf util: Use evsel->name to get tracepoint_paths

2013-04-24 Thread Jiri Olsa 
On Tue, Apr 23, 2013 at 05:31:00PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim 
> 
> Most tracepoint events already have their system and event name in
> ->name field so that searching whole event tracing directory for each
> evsel to match given id is suboptimal.
> 
> Cc: Jiri Olsa 
> Cc: Frederic Weisbecker 
> Signed-off-by: Namhyung Kim 
> ---
>  tools/perf/util/trace-event-info.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/tools/perf/util/trace-event-info.c 
> b/tools/perf/util/trace-event-info.c
> index ab18bf12d54a..dcfc1869c9af 100644
> --- a/tools/perf/util/trace-event-info.c
> +++ b/tools/perf/util/trace-event-info.c
> @@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
>   if (pos->attr.type != PERF_TYPE_TRACEPOINT)
>   continue;
>   ++nr_tracepoints;
> +
> + if (pos->name && strchr(pos->name, ':')) {
> + char *str = strchr(pos->name, ':');
> +
> + ppath->next = zalloc(sizeof(path));
> + if (!ppath->next)
> + goto error;
> +
> + ppath->next->system = strndup(pos->name, str - 
> pos->name);
> + ppath->next->name = strdup(str+1);
> +
> + if (!ppath->next->system || !ppath->next->name)
> + goto error;
> +
> + goto next;
> + }

good idea, could you please move this ^^^ into function ?
something like 'tracepoint_name_to_path'

jirka

> +
>   ppath->next = tracepoint_id_to_path(pos->attr.config);
>   if (!ppath->next) {
> +error:
>   pr_debug("No memory to alloc tracepoints list\n");
>   put_tracepoints_path();
>   return NULL;
>   }
> +next:
>   ppath = ppath->next;
>   }
>  
> -- 
> 1.7.11.7
> 
--
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 02/14] perf util: Use evsel->name to get tracepoint_paths

2013-04-24 Thread Namhyung Kim 
On Tue, 23 Apr 2013 09:07:20 -0400, Steven Rostedt wrote:
>> +
>> +if (pos->name && strchr(pos->name, ':')) {
>> +char *str = strchr(pos->name, ':');
>
> Why not make the above into:
>
>   if (pos->name && (str = strchr(pos->name, ':'))) {
>
> ?

I wanted not to have an assignment in an if condition, just for my
preference.  I will change it.

Thanks,
Namhyung
--
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 02/14] perf util: Use evsel-name to get tracepoint_paths

2013-04-24 Thread Namhyung Kim 
On Tue, 23 Apr 2013 09:07:20 -0400, Steven Rostedt wrote:
 +
 +if (pos-name  strchr(pos-name, ':')) {
 +char *str = strchr(pos-name, ':');

 Why not make the above into:

   if (pos-name  (str = strchr(pos-name, ':'))) {

 ?

I wanted not to have an assignment in an if condition, just for my
preference.  I will change it.

Thanks,
Namhyung
--
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 02/14] perf util: Use evsel-name to get tracepoint_paths

2013-04-24 Thread Jiri Olsa 
On Tue, Apr 23, 2013 at 05:31:00PM +0900, Namhyung Kim wrote:
 From: Namhyung Kim namhyung@lge.com
 
 Most tracepoint events already have their system and event name in
 -name field so that searching whole event tracing directory for each
 evsel to match given id is suboptimal.
 
 Cc: Jiri Olsa jo...@redhat.com
 Cc: Frederic Weisbecker fweis...@gmail.com
 Signed-off-by: Namhyung Kim namhy...@kernel.org
 ---
  tools/perf/util/trace-event-info.c | 19 +++
  1 file changed, 19 insertions(+)
 
 diff --git a/tools/perf/util/trace-event-info.c 
 b/tools/perf/util/trace-event-info.c
 index ab18bf12d54a..dcfc1869c9af 100644
 --- a/tools/perf/util/trace-event-info.c
 +++ b/tools/perf/util/trace-event-info.c
 @@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
   if (pos-attr.type != PERF_TYPE_TRACEPOINT)
   continue;
   ++nr_tracepoints;
 +
 + if (pos-name  strchr(pos-name, ':')) {
 + char *str = strchr(pos-name, ':');
 +
 + ppath-next = zalloc(sizeof(path));
 + if (!ppath-next)
 + goto error;
 +
 + ppath-next-system = strndup(pos-name, str - 
 pos-name);
 + ppath-next-name = strdup(str+1);
 +
 + if (!ppath-next-system || !ppath-next-name)
 + goto error;
 +
 + goto next;
 + }

good idea, could you please move this ^^^ into function ?
something like 'tracepoint_name_to_path'

jirka

 +
   ppath-next = tracepoint_id_to_path(pos-attr.config);
   if (!ppath-next) {
 +error:
   pr_debug(No memory to alloc tracepoints list\n);
   put_tracepoints_path(path);
   return NULL;
   }
 +next:
   ppath = ppath-next;
   }
  
 -- 
 1.7.11.7
 
--
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 02/14] perf util: Use evsel-name to get tracepoint_paths

2013-04-24 Thread Namhyung Kim 
Hi Jiri,

On Wed, 24 Apr 2013 14:42:57 +0200, Jiri Olsa wrote:
 On Tue, Apr 23, 2013 at 05:31:00PM +0900, Namhyung Kim wrote:
 From: Namhyung Kim namhyung@lge.com
 
 Most tracepoint events already have their system and event name in
 -name field so that searching whole event tracing directory for each
 evsel to match given id is suboptimal.
 
 Cc: Jiri Olsa jo...@redhat.com
 Cc: Frederic Weisbecker fweis...@gmail.com
 Signed-off-by: Namhyung Kim namhy...@kernel.org
 ---
  tools/perf/util/trace-event-info.c | 19 +++
  1 file changed, 19 insertions(+)
 
 diff --git a/tools/perf/util/trace-event-info.c 
 b/tools/perf/util/trace-event-info.c
 index ab18bf12d54a..dcfc1869c9af 100644
 --- a/tools/perf/util/trace-event-info.c
 +++ b/tools/perf/util/trace-event-info.c
 @@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
  if (pos-attr.type != PERF_TYPE_TRACEPOINT)
  continue;
  ++nr_tracepoints;
 +
 +if (pos-name  strchr(pos-name, ':')) {
 +char *str = strchr(pos-name, ':');
 +
 +ppath-next = zalloc(sizeof(path));
 +if (!ppath-next)
 +goto error;
 +
 +ppath-next-system = strndup(pos-name, str - 
 pos-name);
 +ppath-next-name = strdup(str+1);
 +
 +if (!ppath-next-system || !ppath-next-name)
 +goto error;
 +
 +goto next;
 +}

 good idea, could you please move this ^^^ into function ?
 something like 'tracepoint_name_to_path'

Will do!

Thanks,
Namhyung
--
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 02/14] perf util: Use evsel->name to get tracepoint_paths

2013-04-23 Thread Steven Rostedt 
On Tue, 2013-04-23 at 17:31 +0900, Namhyung Kim wrote:
> From: Namhyung Kim 
> 
> Most tracepoint events already have their system and event name in
> ->name field so that searching whole event tracing directory for each
> evsel to match given id is suboptimal.
> 
> Cc: Jiri Olsa 
> Cc: Frederic Weisbecker 
> Signed-off-by: Namhyung Kim 
> ---
>  tools/perf/util/trace-event-info.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/tools/perf/util/trace-event-info.c 
> b/tools/perf/util/trace-event-info.c
> index ab18bf12d54a..dcfc1869c9af 100644
> --- a/tools/perf/util/trace-event-info.c
> +++ b/tools/perf/util/trace-event-info.c
> @@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
>   if (pos->attr.type != PERF_TYPE_TRACEPOINT)
>   continue;
>   ++nr_tracepoints;
> +
> + if (pos->name && strchr(pos->name, ':')) {
> + char *str = strchr(pos->name, ':');

Why not make the above into:

if (pos->name && (str = strchr(pos->name, ':'))) {

?

-- Steve



--
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 02/14] perf util: Use evsel->name to get tracepoint_paths

2013-04-23 Thread Namhyung Kim 
From: Namhyung Kim 

Most tracepoint events already have their system and event name in
->name field so that searching whole event tracing directory for each
evsel to match given id is suboptimal.

Cc: Jiri Olsa 
Cc: Frederic Weisbecker 
Signed-off-by: Namhyung Kim 
---
 tools/perf/util/trace-event-info.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/tools/perf/util/trace-event-info.c 
b/tools/perf/util/trace-event-info.c
index ab18bf12d54a..dcfc1869c9af 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
if (pos->attr.type != PERF_TYPE_TRACEPOINT)
continue;
++nr_tracepoints;
+
+   if (pos->name && strchr(pos->name, ':')) {
+   char *str = strchr(pos->name, ':');
+
+   ppath->next = zalloc(sizeof(path));
+   if (!ppath->next)
+   goto error;
+
+   ppath->next->system = strndup(pos->name, str - 
pos->name);
+   ppath->next->name = strdup(str+1);
+
+   if (!ppath->next->system || !ppath->next->name)
+   goto error;
+
+   goto next;
+   }
+
ppath->next = tracepoint_id_to_path(pos->attr.config);
if (!ppath->next) {
+error:
pr_debug("No memory to alloc tracepoints list\n");
put_tracepoints_path();
return NULL;
}
+next:
ppath = ppath->next;
}
 
-- 
1.7.11.7

--
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 02/14] perf util: Use evsel-name to get tracepoint_paths

2013-04-23 Thread Namhyung Kim 
From: Namhyung Kim namhyung@lge.com

Most tracepoint events already have their system and event name in
-name field so that searching whole event tracing directory for each
evsel to match given id is suboptimal.

Cc: Jiri Olsa jo...@redhat.com
Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
 tools/perf/util/trace-event-info.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/tools/perf/util/trace-event-info.c 
b/tools/perf/util/trace-event-info.c
index ab18bf12d54a..dcfc1869c9af 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
if (pos-attr.type != PERF_TYPE_TRACEPOINT)
continue;
++nr_tracepoints;
+
+   if (pos-name  strchr(pos-name, ':')) {
+   char *str = strchr(pos-name, ':');
+
+   ppath-next = zalloc(sizeof(path));
+   if (!ppath-next)
+   goto error;
+
+   ppath-next-system = strndup(pos-name, str - 
pos-name);
+   ppath-next-name = strdup(str+1);
+
+   if (!ppath-next-system || !ppath-next-name)
+   goto error;
+
+   goto next;
+   }
+
ppath-next = tracepoint_id_to_path(pos-attr.config);
if (!ppath-next) {
+error:
pr_debug(No memory to alloc tracepoints list\n);
put_tracepoints_path(path);
return NULL;
}
+next:
ppath = ppath-next;
}
 
-- 
1.7.11.7

--
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 02/14] perf util: Use evsel-name to get tracepoint_paths

2013-04-23 Thread Steven Rostedt 
On Tue, 2013-04-23 at 17:31 +0900, Namhyung Kim wrote:
 From: Namhyung Kim namhyung@lge.com
 
 Most tracepoint events already have their system and event name in
 -name field so that searching whole event tracing directory for each
 evsel to match given id is suboptimal.
 
 Cc: Jiri Olsa jo...@redhat.com
 Cc: Frederic Weisbecker fweis...@gmail.com
 Signed-off-by: Namhyung Kim namhy...@kernel.org
 ---
  tools/perf/util/trace-event-info.c | 19 +++
  1 file changed, 19 insertions(+)
 
 diff --git a/tools/perf/util/trace-event-info.c 
 b/tools/perf/util/trace-event-info.c
 index ab18bf12d54a..dcfc1869c9af 100644
 --- a/tools/perf/util/trace-event-info.c
 +++ b/tools/perf/util/trace-event-info.c
 @@ -414,12 +414,31 @@ get_tracepoints_path(struct list_head *pattrs)
   if (pos-attr.type != PERF_TYPE_TRACEPOINT)
   continue;
   ++nr_tracepoints;
 +
 + if (pos-name  strchr(pos-name, ':')) {
 + char *str = strchr(pos-name, ':');

Why not make the above into:

if (pos-name  (str = strchr(pos-name, ':'))) {

?

-- Steve



--
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/