All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.