[PATCH] perf stat: Fix endless wait for child process

[PATCH] perf stat: Fix endless wait for child process

[Jin Yao]

We hit a perf stat issue by using following script.

  #!/bin/bash

  sleep 1000 &
  exec perf stat -a -e cycles -I1000 -- sleep 5

Since "perf stat" is launched by exec, so the "sleep 1000" would be
the child process of "perf stat". The wait4() will not return because
it's waiting for the child process "sleep 1000" to be end. So perf
stat doesn't return even 5s passed.

This patch lets the perf stat return when the specified child process
is end (in this case, specified child process is "sleep 5").

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/builtin-stat.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1410d66..63a3afc 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -561,7 +561,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 					break;
 			}
 		}
-		wait4(child_pid, &status, 0, &stat_config.ru_data);
+		if (child_pid != -1)
+			wait4(child_pid, &status, 0, &stat_config.ru_data);
 
 		if (workload_exec_errno) {
 			const char *emsg = str_error_r(workload_exec_errno, msg, sizeof(msg));
-- 
2.7.4

Re: [PATCH] perf stat: Fix endless wait for child process

[Jiri Olsa]

On Thu, Jan 03, 2019 at 03:40:45PM +0800, Jin Yao wrote:
> We hit a perf stat issue by using following script.
> 
>   #!/bin/bash
> 
>   sleep 1000 &
>   exec perf stat -a -e cycles -I1000 -- sleep 5
> 
> Since "perf stat" is launched by exec, so the "sleep 1000" would be
> the child process of "perf stat". The wait4() will not return because
> it's waiting for the child process "sleep 1000" to be end. So perf
> stat doesn't return even 5s passed.
> 
> This patch lets the perf stat return when the specified child process
> is end (in this case, specified child process is "sleep 5").
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>

Reviewed-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/builtin-stat.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 1410d66..63a3afc 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -561,7 +561,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>  					break;
>  			}
>  		}
> -		wait4(child_pid, &status, 0, &stat_config.ru_data);
> +		if (child_pid != -1)
> +			wait4(child_pid, &status, 0, &stat_config.ru_data);
>  
>  		if (workload_exec_errno) {
>  			const char *emsg = str_error_r(workload_exec_errno, msg, sizeof(msg));
> -- 
> 2.7.4
> 

Re: [PATCH] perf stat: Fix endless wait for child process

[Arnaldo Carvalho de Melo]

Em Thu, Jan 03, 2019 at 12:12:05PM +0100, Jiri Olsa escreveu:
> On Thu, Jan 03, 2019 at 03:40:45PM +0800, Jin Yao wrote:
> > We hit a perf stat issue by using following script.
> > 
> >   #!/bin/bash
> > 
> >   sleep 1000 &
> >   exec perf stat -a -e cycles -I1000 -- sleep 5
> > 
> > Since "perf stat" is launched by exec, so the "sleep 1000" would be
> > the child process of "perf stat". The wait4() will not return because
> > it's waiting for the child process "sleep 1000" to be end. So perf
> > stat doesn't return even 5s passed.
> > 
> > This patch lets the perf stat return when the specified child process
> > is end (in this case, specified child process is "sleep 5").
> > 
> > Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> 
> Reviewed-by: Jiri Olsa <jolsa@kernel.org>

Thanks, tested and applied.

- Arnaldo

[tip:perf/urgent] perf stat: Fix endless wait for child process

[tip-bot for Jin Yao]

Commit-ID:  8a99255a50c0b4c2a449b96fd8d45fcc8d72c701
Gitweb:     https://git.kernel.org/tip/8a99255a50c0b4c2a449b96fd8d45fcc8d72c701
Author:     Jin Yao <yao.jin@linux.intel.com>
AuthorDate: Thu, 3 Jan 2019 15:40:45 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 3 Jan 2019 12:12:18 -0300

perf stat: Fix endless wait for child process

We hit a 'perf stat' issue by using following script:

  #!/bin/bash

  sleep 1000 &
  exec perf stat -a -e cycles -I1000 -- sleep 5

Since "perf stat" is launched by exec, the "sleep 1000" would be the
child process of "perf stat". The wait4() call will not return because
it's waiting for the child process "sleep 1000" to end. So 'perf stat'
doesn't return even after 5s passes.

This patch lets 'perf stat' return when the specified child process ends
(in this case, the specified child process is "sleep 5").

Committer testing:

  # cat test.sh
  #!/bin/bash

  sleep 10 &
  exec perf stat -a -e cycles -I1000 -- sleep 5
  #

Before:

  # time ./test.sh
  #           time             counts unit events
       1.001113090        108,453,351      cycles
       2.002062196        142,075,435      cycles
       3.002896194        164,801,068      cycles
       4.003731666        107,062,140      cycles
       5.002068867        112,241,832      cycles

  real	0m10.066s
  user	0m0.016s
  sys	0m0.101s
  #

After:

  # time ./test.sh
  #           time             counts unit events
       1.001016096         91,412,027      cycles
       2.002014963        124,063,708      cycles
       3.002883964        125,993,929      cycles
       4.003706470        120,465,734      cycles
       5.002006778        163,560,355      cycles

  real	0m5.123s
  user	0m0.014s
  sys	0m0.105s
  #

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1546501245-4512-1-git-send-email-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1410d66192f7..63a3afc7f32b 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -561,7 +561,8 @@ try_again:
 					break;
 			}
 		}
-		wait4(child_pid, &status, 0, &stat_config.ru_data);
+		if (child_pid != -1)
+			wait4(child_pid, &status, 0, &stat_config.ru_data);
 
 		if (workload_exec_errno) {
 			const char *emsg = str_error_r(workload_exec_errno, msg, sizeof(msg));