public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tools: fix set event list leader
@ 2013-01-31 12:54 Stephane Eranian
  2013-01-31 13:12 ` Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stephane Eranian @ 2013-01-31 12:54 UTC (permalink / raw)
  To: linux-kernel; +Cc: acme, mingo, peterz, jolsa, namhyung.kim


The __perf_evlist__set_leader() was setting the leader for all events
in the list except the first. Which means it assumed the first event
already had event->leader = event. Seems like this should be the role
of the function to also do this. This is a requirement for an upcoming
patch set.

Signed-off-by: Stephane Eranian <eranian@google.com>
---

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index dc8aee9..050d5bc 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -119,8 +119,7 @@ void __perf_evlist__set_leader(struct list_head *list)
 	leader = list_entry(list->next, struct perf_evsel, node);
 
 	list_for_each_entry(evsel, list, node) {
-		if (evsel != leader)
-			evsel->leader = leader;
+		evsel->leader = leader;
 	}
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf tools: fix set event list leader
  2013-01-31 12:54 [PATCH] perf tools: fix set event list leader Stephane Eranian
@ 2013-01-31 13:12 ` Jiri Olsa
  2013-02-01  1:50 ` Namhyung Kim
  2013-02-06 22:01 ` [tip:perf/core] perf evlist: Fix " tip-bot for Stephane Eranian
  2 siblings, 0 replies; 4+ messages in thread
From: Jiri Olsa @ 2013-01-31 13:12 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: linux-kernel, acme, mingo, peterz, namhyung.kim

On Thu, Jan 31, 2013 at 01:54:37PM +0100, Stephane Eranian wrote:
> 
> The __perf_evlist__set_leader() was setting the leader for all events
> in the list except the first. Which means it assumed the first event
> already had event->leader = event. Seems like this should be the role
> of the function to also do this. This is a requirement for an upcoming
> patch set.

agreed, 

Acked/Tested-by Jiri Olsa <jolsa@redhat.com>


> 
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index dc8aee9..050d5bc 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -119,8 +119,7 @@ void __perf_evlist__set_leader(struct list_head *list)
>  	leader = list_entry(list->next, struct perf_evsel, node);
>  
>  	list_for_each_entry(evsel, list, node) {
> -		if (evsel != leader)
> -			evsel->leader = leader;
> +		evsel->leader = leader;
>  	}

that's leftover from the times when leader has 'leader' set to NULL

we made the change here:

perf evsel: Set leader evsel's ->leader to itself
commit 2cfda562da7b0b1e0575507ef3efe782d4e218e4
Author: Namhyung Kim <namhyung.kim@lge.com>
Date:   Thu Nov 29 15:38:29 2012 +0900

the leader is set properly in perf_evsel__init and when
reading perf.data, so there was no harm in current code

thanks,
jirka

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf tools: fix set event list leader
  2013-01-31 12:54 [PATCH] perf tools: fix set event list leader Stephane Eranian
  2013-01-31 13:12 ` Jiri Olsa
@ 2013-02-01  1:50 ` Namhyung Kim
  2013-02-06 22:01 ` [tip:perf/core] perf evlist: Fix " tip-bot for Stephane Eranian
  2 siblings, 0 replies; 4+ messages in thread
From: Namhyung Kim @ 2013-02-01  1:50 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: linux-kernel, acme, mingo, peterz, jolsa, namhyung.kim

Hi Stephane,

On Thu, 31 Jan 2013 13:54:37 +0100, Stephane Eranian wrote:
> The __perf_evlist__set_leader() was setting the leader for all events
> in the list except the first. Which means it assumed the first event
> already had event->leader = event. Seems like this should be the role
> of the function to also do this. This is a requirement for an upcoming
> patch set.
>
> Signed-off-by: Stephane Eranian <eranian@google.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index dc8aee9..050d5bc 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -119,8 +119,7 @@ void __perf_evlist__set_leader(struct list_head *list)
>  	leader = list_entry(list->next, struct perf_evsel, node);
>  
>  	list_for_each_entry(evsel, list, node) {
> -		if (evsel != leader)
> -			evsel->leader = leader;
> +		evsel->leader = leader;
>  	}
>  }
>  

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tip:perf/core] perf evlist: Fix set event list leader
  2013-01-31 12:54 [PATCH] perf tools: fix set event list leader Stephane Eranian
  2013-01-31 13:12 ` Jiri Olsa
  2013-02-01  1:50 ` Namhyung Kim
@ 2013-02-06 22:01 ` tip-bot for Stephane Eranian
  2 siblings, 0 replies; 4+ messages in thread
From: tip-bot for Stephane Eranian @ 2013-02-06 22:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, eranian, hpa, mingo, peterz, namhyung.kim,
	namhyung, jolsa, tglx, mingo

Commit-ID:  74b2133d19e776924b2773e27dd9d6940f1cc594
Gitweb:     http://git.kernel.org/tip/74b2133d19e776924b2773e27dd9d6940f1cc594
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Thu, 31 Jan 2013 13:54:37 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 6 Feb 2013 18:09:25 -0300

perf evlist: Fix set event list leader

The __perf_evlist__set_leader() was setting the leader for all events in
the list except the first. Which means it assumed the first event
already had event->leader = event.

Seems like this should be the role of the function to also do this. This
is a requirement for an upcoming patch set.

Signed-off-by: Stephane Eranian <eranian@google.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20130131125437.GA3656@quad
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index eddd5eb..ecf123e 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -122,8 +122,7 @@ void __perf_evlist__set_leader(struct list_head *list)
 	leader->nr_members = evsel->idx - leader->idx + 1;
 
 	list_for_each_entry(evsel, list, node) {
-		if (evsel != leader)
-			evsel->leader = leader;
+		evsel->leader = leader;
 	}
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-02-06 22:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-31 12:54 [PATCH] perf tools: fix set event list leader Stephane Eranian
2013-01-31 13:12 ` Jiri Olsa
2013-02-01  1:50 ` Namhyung Kim
2013-02-06 22:01 ` [tip:perf/core] perf evlist: Fix " tip-bot for Stephane Eranian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox