* [BUG] perf stat: hangs with -p and process completes @ 2018-10-12 21:26 Stephane Eranian 2018-10-16 11:03 ` Jiri Olsa 0 siblings, 1 reply; 4+ messages in thread From: Stephane Eranian @ 2018-10-12 21:26 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, LKML Hi, I am running into a perf stat issue with the -p option which allows you to attach to a running process. If that process happens to terminate while under monitoring perf hangs in there and never terminates. The proper behavior would be to stop. I can see the issue in that the attached process is not a child, so wait() would not work. To reproduce: $ sleep 10 & $ perf stat -p $! doing the same with perf record works, so there is a solution to this problem. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [BUG] perf stat: hangs with -p and process completes 2018-10-12 21:26 [BUG] perf stat: hangs with -p and process completes Stephane Eranian @ 2018-10-16 11:03 ` Jiri Olsa 2018-10-16 16:25 ` Jiri Olsa 0 siblings, 1 reply; 4+ messages in thread From: Jiri Olsa @ 2018-10-16 11:03 UTC (permalink / raw) To: Stephane Eranian Cc: Arnaldo Carvalho de Melo, Jiri Olsa, LKML, Peter Zijlstra, Ingo Molnar, Namhyung Kim, Alexander Shishkin On Fri, Oct 12, 2018 at 02:26:09PM -0700, Stephane Eranian wrote: > Hi, > > I am running into a perf stat issue with the -p option which allows you > to attach to a running process. If that process happens to terminate > while under monitoring > perf hangs in there and never terminates. The proper behavior would be to stop. > I can see the issue in that the attached process is not a child, so > wait() would not work. > > To reproduce: > $ sleep 10 & > $ perf stat -p $! > > doing the same with perf record works, so there is a solution to this problem. yea, we don't poll for the event state change in perf stat, but we do that in perf record.. also because the perf poll code in kernel is originaly meant for tracking the ring buffer state maybe we could return EPOLLIN for alive events without ring buffer.. like below (totaly untested) and add polling for event state into perf stat cc-ing perf folks jirka --- diff --git a/kernel/events/core.c b/kernel/events/core.c index 5a97f34bc14c..b9ee7e7803bf 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -4865,12 +4865,12 @@ static __poll_t perf_poll(struct file *file, poll_table *wait) { struct perf_event *event = file->private_data; struct ring_buffer *rb; - __poll_t events = EPOLLHUP; + __poll_t events = EPOLLIN; poll_wait(file, &event->waitq, wait); if (is_event_hup(event)) - return events; + return EPOLLHUP; /* * Pin the event->rb by taking event->mmap_mutex; otherwise ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [BUG] perf stat: hangs with -p and process completes 2018-10-16 11:03 ` Jiri Olsa @ 2018-10-16 16:25 ` Jiri Olsa 2018-10-16 18:10 ` Stephane Eranian 0 siblings, 1 reply; 4+ messages in thread From: Jiri Olsa @ 2018-10-16 16:25 UTC (permalink / raw) To: Stephane Eranian Cc: Arnaldo Carvalho de Melo, Jiri Olsa, LKML, Peter Zijlstra, Ingo Molnar, Namhyung Kim, Alexander Shishkin On Tue, Oct 16, 2018 at 01:03:41PM +0200, Jiri Olsa wrote: > On Fri, Oct 12, 2018 at 02:26:09PM -0700, Stephane Eranian wrote: > > Hi, > > > > I am running into a perf stat issue with the -p option which allows you > > to attach to a running process. If that process happens to terminate > > while under monitoring > > perf hangs in there and never terminates. The proper behavior would be to stop. > > I can see the issue in that the attached process is not a child, so > > wait() would not work. > > > > To reproduce: > > $ sleep 10 & > > $ perf stat -p $! > > > > doing the same with perf record works, so there is a solution to this problem. > > yea, we don't poll for the event state change in perf stat, > but we do that in perf record.. also because the perf poll > code in kernel is originaly meant for tracking the ring > buffer state > > maybe we could return EPOLLIN for alive events without ring > buffer.. like below (totaly untested) and add polling for > event state into perf stat > > cc-ing perf folks > on the second thought attached patch works as well without kernel change jirka --- diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index b86aba1c8028..d1028d7755bb 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -409,6 +409,28 @@ static struct perf_evsel *perf_evsel__reset_weak_group(struct perf_evsel *evsel) return leader; } +static bool is_target_alive(struct target *_target, + struct thread_map *threads) +{ + struct stat st; + int i; + + if (!target__has_task(_target)) + return true; + + for (i = 0; i < threads->nr; i++) { + char path[PATH_MAX]; + + scnprintf(path, PATH_MAX, "%s/%d", procfs__mountpoint(), + threads->map[i].pid); + + if (!stat(path, &st)) + return true; + } + + return false; +} + static int __run_perf_stat(int argc, const char **argv, int run_idx) { int interval = stat_config.interval; @@ -579,6 +601,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx) enable_counters(); while (!done) { nanosleep(&ts, NULL); + if (!is_target_alive(&target, evsel_list->threads)) + break; if (timeout) break; if (interval) { ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [BUG] perf stat: hangs with -p and process completes 2018-10-16 16:25 ` Jiri Olsa @ 2018-10-16 18:10 ` Stephane Eranian 0 siblings, 0 replies; 4+ messages in thread From: Stephane Eranian @ 2018-10-16 18:10 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, Jiri Olsa, LKML, Peter Zijlstra, Ingo Molnar, Namhyung Kim, Alexander Shishkin Jiri, Thanks for looking into this. Yeah, I don't think you need a kernel patch to make this work. You can poll in the app. Let me try this out. On Tue, Oct 16, 2018 at 9:25 AM Jiri Olsa <jolsa@redhat.com> wrote: > > On Tue, Oct 16, 2018 at 01:03:41PM +0200, Jiri Olsa wrote: > > On Fri, Oct 12, 2018 at 02:26:09PM -0700, Stephane Eranian wrote: > > > Hi, > > > > > > I am running into a perf stat issue with the -p option which allows you > > > to attach to a running process. If that process happens to terminate > > > while under monitoring > > > perf hangs in there and never terminates. The proper behavior would be to stop. > > > I can see the issue in that the attached process is not a child, so > > > wait() would not work. > > > > > > To reproduce: > > > $ sleep 10 & > > > $ perf stat -p $! > > > > > > doing the same with perf record works, so there is a solution to this problem. > > > > yea, we don't poll for the event state change in perf stat, > > but we do that in perf record.. also because the perf poll > > code in kernel is originaly meant for tracking the ring > > buffer state > > > > maybe we could return EPOLLIN for alive events without ring > > buffer.. like below (totaly untested) and add polling for > > event state into perf stat > > > > cc-ing perf folks > > > > on the second thought attached patch works as well > without kernel change > > jirka > > > --- > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index b86aba1c8028..d1028d7755bb 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -409,6 +409,28 @@ static struct perf_evsel *perf_evsel__reset_weak_group(struct perf_evsel *evsel) > return leader; > } > > +static bool is_target_alive(struct target *_target, > + struct thread_map *threads) > +{ > + struct stat st; > + int i; > + > + if (!target__has_task(_target)) > + return true; > + > + for (i = 0; i < threads->nr; i++) { > + char path[PATH_MAX]; > + > + scnprintf(path, PATH_MAX, "%s/%d", procfs__mountpoint(), > + threads->map[i].pid); > + > + if (!stat(path, &st)) > + return true; > + } > + > + return false; > +} > + > static int __run_perf_stat(int argc, const char **argv, int run_idx) > { > int interval = stat_config.interval; > @@ -579,6 +601,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx) > enable_counters(); > while (!done) { > nanosleep(&ts, NULL); > + if (!is_target_alive(&target, evsel_list->threads)) > + break; > if (timeout) > break; > if (interval) { ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-16 18:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-12 21:26 [BUG] perf stat: hangs with -p and process completes Stephane Eranian 2018-10-16 11:03 ` Jiri Olsa 2018-10-16 16:25 ` Jiri Olsa 2018-10-16 18:10 ` Stephane Eranian
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).