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

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

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

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 <kan.liang@intel.com>
---
 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

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

[Jiri Olsa]

On Wed, Dec 14, 2016 at 12:48:05PM -0500, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> 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

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

[Liang, Kan]

> 
> On Wed, Dec 14, 2016 at 12:48:05PM -0500, kan.liang@intel.com wrote:
> > From: Kan Liang <kan.liang@intel.com>
> >
> > 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

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

[acme]

Em Wed, Dec 14, 2016 at 06:26:02PM +0000, Liang, Kan escreveu:
> > On Wed, Dec 14, 2016 at 12:48:05PM -0500, kan.liang@intel.com wrote:
> > > From: Kan Liang <kan.liang@intel.com>
> > > 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

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

[Liang, Kan]

.
> Em Wed, Dec 14, 2016 at 06:26:02PM +0000, Liang, Kan escreveu:
> > > On Wed, Dec 14, 2016 at 12:48:05PM -0500, kan.liang@intel.com wrote:
> > > > From: Kan Liang <kan.liang@intel.com> 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