* [PATCH v2] Poll for monitored tasks being alive in fork mode
@ 2019-01-07 1:00 Jin Yao
2019-01-07 9:05 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Jin Yao @ 2019-01-07 1:00 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
Following test shows the stat keeps running even if no longer
task to monitor (mgen exits at ~5s).
perf stat -e cycles -p `pgrep mgen` -I1000 -- sleep 10
time counts unit events
1.000148916 1,308,365,864 cycles
2.000379171 1,297,269,875 cycles
3.000556719 1,297,187,078 cycles
4.000914241 761,261,827 cycles
5.001306091 <not counted> cycles
6.001676881 <not counted> cycles
7.002046336 <not counted> cycles
8.002405651 <not counted> cycles
9.002766625 <not counted> cycles
10.001395827 <not counted> cycles
We'd better finish stat immediately if there's no longer task to
monitor.
After:
perf stat -e cycles -p `pgrep mgen` -I1000 -- sleep 10
time counts unit events
1.000180062 1,236,592,661 cycles
2.000421539 1,223,733,572 cycles
3.000609910 1,297,047,663 cycles
4.000807545 1,297,215,816 cycles
5.001001578 1,297,208,032 cycles
6.001390345 582,343,659 cycles
sleep: Terminated
Now the stat exits immediately when the monitored tasks ends.
v2:
---
There is a potential race condition window, that is the
child_pid set with -1 and kill(-1, SIGTERM) may happen.
Jiri provides a simple method which stores the child_pid to
a local variable and check the local variable before kill().
At least, we can avoid the kill(-1, SIGTERM) case.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/builtin-stat.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 63a3afc..bd5fb8e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -553,6 +553,15 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
if (interval || timeout) {
while (!waitpid(child_pid, &status, WNOHANG)) {
+ if (!is_target_alive(&target,
+ evsel_list->threads)) {
+ int pid = child_pid;
+
+ if (pid != -1)
+ kill(pid, SIGTERM);
+ break;
+ }
+
nanosleep(&ts, NULL);
if (timeout)
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Poll for monitored tasks being alive in fork mode
2019-01-07 1:00 [PATCH v2] Poll for monitored tasks being alive in fork mode Jin Yao
@ 2019-01-07 9:05 ` Jiri Olsa
2019-01-21 1:40 ` Jin, Yao
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2019-01-07 9:05 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Mon, Jan 07, 2019 at 09:00:50AM +0800, Jin Yao wrote:
> Following test shows the stat keeps running even if no longer
> task to monitor (mgen exits at ~5s).
>
> perf stat -e cycles -p `pgrep mgen` -I1000 -- sleep 10
> time counts unit events
> 1.000148916 1,308,365,864 cycles
> 2.000379171 1,297,269,875 cycles
> 3.000556719 1,297,187,078 cycles
> 4.000914241 761,261,827 cycles
> 5.001306091 <not counted> cycles
> 6.001676881 <not counted> cycles
> 7.002046336 <not counted> cycles
> 8.002405651 <not counted> cycles
> 9.002766625 <not counted> cycles
> 10.001395827 <not counted> cycles
>
> We'd better finish stat immediately if there's no longer task to
> monitor.
>
> After:
>
> perf stat -e cycles -p `pgrep mgen` -I1000 -- sleep 10
> time counts unit events
> 1.000180062 1,236,592,661 cycles
> 2.000421539 1,223,733,572 cycles
> 3.000609910 1,297,047,663 cycles
> 4.000807545 1,297,215,816 cycles
> 5.001001578 1,297,208,032 cycles
> 6.001390345 582,343,659 cycles
> sleep: Terminated
>
> Now the stat exits immediately when the monitored tasks ends.
>
> v2:
> ---
> There is a potential race condition window, that is the
> child_pid set with -1 and kill(-1, SIGTERM) may happen.
> Jiri provides a simple method which stores the child_pid to
> a local variable and check the local variable before kill().
> At least, we can avoid the kill(-1, SIGTERM) case.
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
> ---
> tools/perf/builtin-stat.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 63a3afc..bd5fb8e 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -553,6 +553,15 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>
> if (interval || timeout) {
> while (!waitpid(child_pid, &status, WNOHANG)) {
> + if (!is_target_alive(&target,
> + evsel_list->threads)) {
> + int pid = child_pid;
> +
> + if (pid != -1)
> + kill(pid, SIGTERM);
> + break;
> + }
> +
> nanosleep(&ts, NULL);
> if (timeout)
> break;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Poll for monitored tasks being alive in fork mode
2019-01-07 9:05 ` Jiri Olsa
@ 2019-01-21 1:40 ` Jin, Yao
0 siblings, 0 replies; 3+ messages in thread
From: Jin, Yao @ 2019-01-21 1:40 UTC (permalink / raw)
To: Jiri Olsa, acme
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
Hi Arnaldo,
Is this patch acceptable?
Thanks
Jin Yao
On 1/7/2019 5:05 PM, Jiri Olsa wrote:
> On Mon, Jan 07, 2019 at 09:00:50AM +0800, Jin Yao wrote:
>> Following test shows the stat keeps running even if no longer
>> task to monitor (mgen exits at ~5s).
>>
>> perf stat -e cycles -p `pgrep mgen` -I1000 -- sleep 10
>> time counts unit events
>> 1.000148916 1,308,365,864 cycles
>> 2.000379171 1,297,269,875 cycles
>> 3.000556719 1,297,187,078 cycles
>> 4.000914241 761,261,827 cycles
>> 5.001306091 <not counted> cycles
>> 6.001676881 <not counted> cycles
>> 7.002046336 <not counted> cycles
>> 8.002405651 <not counted> cycles
>> 9.002766625 <not counted> cycles
>> 10.001395827 <not counted> cycles
>>
>> We'd better finish stat immediately if there's no longer task to
>> monitor.
>>
>> After:
>>
>> perf stat -e cycles -p `pgrep mgen` -I1000 -- sleep 10
>> time counts unit events
>> 1.000180062 1,236,592,661 cycles
>> 2.000421539 1,223,733,572 cycles
>> 3.000609910 1,297,047,663 cycles
>> 4.000807545 1,297,215,816 cycles
>> 5.001001578 1,297,208,032 cycles
>> 6.001390345 582,343,659 cycles
>> sleep: Terminated
>>
>> Now the stat exits immediately when the monitored tasks ends.
>>
>> v2:
>> ---
>> There is a potential race condition window, that is the
>> child_pid set with -1 and kill(-1, SIGTERM) may happen.
>> Jiri provides a simple method which stores the child_pid to
>> a local variable and check the local variable before kill().
>> At least, we can avoid the kill(-1, SIGTERM) case.
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>
> Reviewed-by: Jiri Olsa <jolsa@kernel.org>
>
> thanks,
> jirka
>
>> ---
>> tools/perf/builtin-stat.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 63a3afc..bd5fb8e 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -553,6 +553,15 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>>
>> if (interval || timeout) {
>> while (!waitpid(child_pid, &status, WNOHANG)) {
>> + if (!is_target_alive(&target,
>> + evsel_list->threads)) {
>> + int pid = child_pid;
>> +
>> + if (pid != -1)
>> + kill(pid, SIGTERM);
>> + break;
>> + }
>> +
>> nanosleep(&ts, NULL);
>> if (timeout)
>> break;
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-21 8:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 1:00 [PATCH v2] Poll for monitored tasks being alive in fork mode Jin Yao
2019-01-07 9:05 ` Jiri Olsa
2019-01-21 1:40 ` Jin, Yao
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.