* [PATCH] perf stat: Set child_pid after perf_evlist__prepare_workload()
@ 2013-09-30 9:01 Namhyung Kim
2013-09-30 13:47 ` David Ahern
2013-10-08 10:40 ` [tip:perf/urgent] " tip-bot for Namhyung Kim
0 siblings, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2013-09-30 9:01 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Jiri Olsa, David Ahern, Stephane Eranian
From: Namhyung Kim <namhyung.kim@lge.com>
The commit acf2892270dc ("perf stat: Use perf_evlist__prepare/
start_workload()") converted to use the function but forgot to update
child_pid. Fix it.
Cc: David Ahern <dsahern@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f686d5ff594e..5098f144b92d 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -457,6 +457,7 @@ static int __run_perf_stat(int argc, const char **argv)
perror("failed to prepare workload");
return -1;
}
+ child_pid = evsel_list->workload.pid;
}
if (group)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf stat: Set child_pid after perf_evlist__prepare_workload()
2013-09-30 9:01 [PATCH] perf stat: Set child_pid after perf_evlist__prepare_workload() Namhyung Kim
@ 2013-09-30 13:47 ` David Ahern
2013-10-01 16:51 ` Arnaldo Carvalho de Melo
2013-10-08 10:40 ` [tip:perf/urgent] " tip-bot for Namhyung Kim
1 sibling, 1 reply; 5+ messages in thread
From: David Ahern @ 2013-09-30 13:47 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML, Jiri Olsa, Stephane Eranian
On 9/30/13 3:01 AM, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
>
> The commit acf2892270dc ("perf stat: Use perf_evlist__prepare/
> start_workload()") converted to use the function but forgot to update
> child_pid. Fix it.
>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-stat.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f686d5ff594e..5098f144b92d 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -457,6 +457,7 @@ static int __run_perf_stat(int argc, const char **argv)
> perror("failed to prepare workload");
> return -1;
> }
> + child_pid = evsel_list->workload.pid;
> }
>
> if (group)
>
Interesting. I can't find a test case where is matters ... perhaps a
convenience of waitpid taking -1.
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf stat: Set child_pid after perf_evlist__prepare_workload()
2013-09-30 13:47 ` David Ahern
@ 2013-10-01 16:51 ` Arnaldo Carvalho de Melo
2013-10-02 2:58 ` Namhyung Kim
0 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-10-01 16:51 UTC (permalink / raw)
To: David Ahern
Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Namhyung Kim, LKML, Jiri Olsa, Stephane Eranian
Em Mon, Sep 30, 2013 at 07:47:25AM -0600, David Ahern escreveu:
> On 9/30/13 3:01 AM, Namhyung Kim wrote:
> >From: Namhyung Kim <namhyung.kim@lge.com>
> >
> >The commit acf2892270dc ("perf stat: Use perf_evlist__prepare/
> >start_workload()") converted to use the function but forgot to update
> >child_pid. Fix it.
> >
> >Cc: David Ahern <dsahern@gmail.com>
> >Cc: Stephane Eranian <eranian@google.com>
> >Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >---
> > tools/perf/builtin-stat.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> >index f686d5ff594e..5098f144b92d 100644
> >--- a/tools/perf/builtin-stat.c
> >+++ b/tools/perf/builtin-stat.c
> >@@ -457,6 +457,7 @@ static int __run_perf_stat(int argc, const char **argv)
> > perror("failed to prepare workload");
> > return -1;
> > }
> >+ child_pid = evsel_list->workload.pid;
> > }
> >
> > if (group)
> >
>
> Interesting. I can't find a test case where is matters ... perhaps a
> convenience of waitpid taking -1.
Yeah, the interval part initially used the fork return, then after
acf2892270dc used the default value of child_pid, -1, which is ok as it
waits for any children, but I wonder what would happen in workloads with
multiple children.
But for correctness acf2892270dc should have kept the existing
semantics, even if using -1 could in the end be ok.
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf stat: Set child_pid after perf_evlist__prepare_workload()
2013-10-01 16:51 ` Arnaldo Carvalho de Melo
@ 2013-10-02 2:58 ` Namhyung Kim
0 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2013-10-02 2:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Namhyung Kim, LKML, Jiri Olsa, Stephane Eranian
On Tue, 1 Oct 2013 13:51:33 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Sep 30, 2013 at 07:47:25AM -0600, David Ahern escreveu:
>> On 9/30/13 3:01 AM, Namhyung Kim wrote:
>> >From: Namhyung Kim <namhyung.kim@lge.com>
>> >
>> >The commit acf2892270dc ("perf stat: Use perf_evlist__prepare/
>> >start_workload()") converted to use the function but forgot to update
>> >child_pid. Fix it.
>> >
>> >Cc: David Ahern <dsahern@gmail.com>
>> >Cc: Stephane Eranian <eranian@google.com>
>> >Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>> >---
>> > tools/perf/builtin-stat.c | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> >diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> >index f686d5ff594e..5098f144b92d 100644
>> >--- a/tools/perf/builtin-stat.c
>> >+++ b/tools/perf/builtin-stat.c
>> >@@ -457,6 +457,7 @@ static int __run_perf_stat(int argc, const char **argv)
>> > perror("failed to prepare workload");
>> > return -1;
>> > }
>> >+ child_pid = evsel_list->workload.pid;
>> > }
>> >
>> > if (group)
>> >
>>
>> Interesting. I can't find a test case where is matters ... perhaps a
>> convenience of waitpid taking -1.
>
> Yeah, the interval part initially used the fork return, then after
> acf2892270dc used the default value of child_pid, -1, which is ok as it
> waits for any children, but I wonder what would happen in workloads with
> multiple children.
Hmm.. right. And in fact what I actually concern was when killing child
if create_perf_stat_counter() failed.
if (child_pid != -1)
kill(child_pid, SIGTERM);
>
> But for correctness acf2892270dc should have kept the existing
> semantics, even if using -1 could in the end be ok.
Agreed.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/urgent] perf stat: Set child_pid after perf_evlist__prepare_workload()
2013-09-30 9:01 [PATCH] perf stat: Set child_pid after perf_evlist__prepare_workload() Namhyung Kim
2013-09-30 13:47 ` David Ahern
@ 2013-10-08 10:40 ` tip-bot for Namhyung Kim
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Namhyung Kim @ 2013-10-08 10:40 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, eranian, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, jolsa, dsahern, tglx
Commit-ID: d20a47e70b57a77f4d1b0dba98a668e14c2f9bf9
Gitweb: http://git.kernel.org/tip/d20a47e70b57a77f4d1b0dba98a668e14c2f9bf9
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 30 Sep 2013 18:01:11 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 4 Oct 2013 15:16:05 -0300
perf stat: Set child_pid after perf_evlist__prepare_workload()
The commit acf2892270dc ("perf stat: Use perf_evlist__prepare/
start_workload()") converted to use the function but forgot to update
child_pid. Fix it.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1380531671-28076-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-stat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f686d5f..5098f14 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -457,6 +457,7 @@ static int __run_perf_stat(int argc, const char **argv)
perror("failed to prepare workload");
return -1;
}
+ child_pid = evsel_list->workload.pid;
}
if (group)
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-08 10:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-30 9:01 [PATCH] perf stat: Set child_pid after perf_evlist__prepare_workload() Namhyung Kim
2013-09-30 13:47 ` David Ahern
2013-10-01 16:51 ` Arnaldo Carvalho de Melo
2013-10-02 2:58 ` Namhyung Kim
2013-10-08 10:40 ` [tip:perf/urgent] " tip-bot for Namhyung Kim
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.