2016-12-14 17:49:54

by Liang, Kan

[permalink] [raw]
Subject: [PATCH] perf tools: ignore zombie process for user profile

From: Kan Liang <[email protected]>

If user has zombie process, the perf record -u will error out.
Here is an example.
$ ./testd &
[1] 23796
$ sudo perf record -e cycles -u kan
Error:
The sys_perf_event_open() syscall returned with 3 (No such process) for
event (cycles).
/bin/dmesg may provide additional information.
No CONFIG_PERF_EVENTS=y kernel support configured?

The source code of testd is as below.
int main() {

if (fork())
{
while (1);
}
return 0;
}

Zombie process is dead process. It is meaningless to profile it.
It's better to ignore it for user profile.

Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/util/thread_map.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 40585f5..4643b69 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -89,6 +89,39 @@ struct thread_map *thread_map__new_by_tid(pid_t tid)
return threads;
}

+static bool is_zombie_process(pid_t pid)
+{
+ char filename[PATH_MAX];
+ char comm[PATH_MAX];
+ char bf[BUFSIZ];
+ char s_state;
+ int s_pid;
+ FILE *fp;
+ ssize_t n;
+
+ snprintf(filename, sizeof(filename), "/proc/%d/stat", pid);
+ fp = fopen(filename, "r");
+ if (!fp) {
+ pr_warning("couldn't open %s\n", filename);
+ return false;
+ }
+ if (fgets(bf, sizeof(bf), fp) == NULL) {
+ pr_warning("Couldn't read stat for pid %d\n", pid);
+ fclose(fp);
+ return false;
+ }
+ fclose(fp);
+
+ n = sscanf(bf, "%d %s %c ", &s_pid, comm, &s_state);
+ if (n < 3)
+ return false;
+
+ if (s_state == 'Z')
+ return true;
+
+ return false;
+}
+
struct thread_map *thread_map__new_by_uid(uid_t uid)
{
DIR *proc;
@@ -124,6 +157,9 @@ struct thread_map *thread_map__new_by_uid(uid_t uid)
if (st.st_uid != uid)
continue;

+ if (is_zombie_process(pid))
+ continue;
+
snprintf(path, sizeof(path), "/proc/%d/task", pid);
items = scandir(path, &namelist, filter, NULL);
if (items <= 0)
--
2.4.3


2016-12-14 17:59:24

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf tools: ignore zombie process for user profile

On Wed, Dec 14, 2016 at 12:48:05PM -0500, [email protected] wrote:
> From: Kan Liang <[email protected]>
>
> If user has zombie process, the perf record -u will error out.
> Here is an example.
> $ ./testd &
> [1] 23796
> $ sudo perf record -e cycles -u kan
> Error:
> The sys_perf_event_open() syscall returned with 3 (No such process) for
> event (cycles).
> /bin/dmesg may provide additional information.
> No CONFIG_PERF_EVENTS=y kernel support configured?
>
> The source code of testd is as below.
> int main() {
>
> if (fork())
> {
> while (1);
> }
> return 0;
> }
>
> Zombie process is dead process. It is meaningless to profile it.
> It's better to ignore it for user profile.

I recently posted different patch for same issue:
http://marc.info/?l=linux-kernel&m=148153895827359&w=2

jirka

2016-12-14 18:31:07

by Liang, Kan

[permalink] [raw]
Subject: RE: [PATCH] perf tools: ignore zombie process for user profile



>
> On Wed, Dec 14, 2016 at 12:48:05PM -0500, [email protected] wrote:
> > From: Kan Liang <[email protected]>
> >
> > If user has zombie process, the perf record -u will error out.
> > Here is an example.
> > $ ./testd &
> > [1] 23796
> > $ sudo perf record -e cycles -u kan
> > Error:
> > The sys_perf_event_open() syscall returned with 3 (No such process)
> > for event (cycles).
> > /bin/dmesg may provide additional information.
> > No CONFIG_PERF_EVENTS=y kernel support configured?
> >
> > The source code of testd is as below.
> > int main() {
> >
> > if (fork())
> > {
> > while (1);
> > }
> > return 0;
> > }
> >
> > Zombie process is dead process. It is meaningless to profile it.
> > It's better to ignore it for user profile.
>
> I recently posted different patch for same issue:
> http://marc.info/?l=linux-kernel&m=148153895827359&w=2

The change as below make me confuse.
+ /* The system wide setup does not work with threads. */
+ if (!evsel->system_wide)
+ return false;
It looks the meaning of the comments is inconsistent with the code.


Your original patch doesn't work well with the issue.
But if I change the above code as below, the issue is fixed.
if (evsel->system_wide)
return false;

Thanks,
Kan

2016-12-14 19:13:38

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf tools: ignore zombie process for user profile

Em Wed, Dec 14, 2016 at 06:26:02PM +0000, Liang, Kan escreveu:
> > On Wed, Dec 14, 2016 at 12:48:05PM -0500, [email protected] wrote:
> > > From: Kan Liang <[email protected]>
> > > Zombie process is dead process. It is meaningless to profile it.
> > > It's better to ignore it for user profile.

> > I recently posted different patch for same issue:
> > http://marc.info/?l=linux-kernel&m=148153895827359&w=2

> The change as below make me confuse.
> + /* The system wide setup does not work with threads. */
> + if (!evsel->system_wide)
> + return false;
> It looks the meaning of the comments is inconsistent with the code.

> Your original patch doesn't work well with the issue.
> But if I change the above code as below, the issue is fixed.
> if (evsel->system_wide)
> return false;

yeah, Namhyung noticed that, Jiri sent a fixed patch, I have it already
in my perf/core branch.

- Arnaldo

2016-12-14 19:19:57

by Liang, Kan

[permalink] [raw]
Subject: RE: [PATCH] perf tools: ignore zombie process for user profile



.
> Em Wed, Dec 14, 2016 at 06:26:02PM +0000, Liang, Kan escreveu:
> > > On Wed, Dec 14, 2016 at 12:48:05PM -0500, [email protected] wrote:
> > > > From: Kan Liang <[email protected]> Zombie process is dead
> > > > process. It is meaningless to profile it.
> > > > It's better to ignore it for user profile.
>
> > > I recently posted different patch for same issue:
> > > http://marc.info/?l=linux-kernel&m=148153895827359&w=2
>
> > The change as below make me confuse.
> > + /* The system wide setup does not work with threads. */
> > + if (!evsel->system_wide)
> > + return false;
> > It looks the meaning of the comments is inconsistent with the code.
>
> > Your original patch doesn't work well with the issue.
> > But if I change the above code as below, the issue is fixed.
> > if (evsel->system_wide)
> > return false;
>
> yeah, Namhyung noticed that, Jiri sent a fixed patch, I have it already in my
> perf/core branch.
>

That's great. Then Jiri's patch would work on my case.
Please ignore this patch.

Thanks,
Kan