All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: acme@kernel.org
Cc: linux-kernel@vger.kernel.org, David Ahern <dsahern@gmail.com>,
	Don Zickus <dzickus@redhat.com>, Joe Mario <jmario@redhat.com>,
	Jiri Olsa <jolsa@redhat.com>
Subject: [PATCH v2] perf tool: Fix ppid for synthesized fork events
Date: Wed, 25 Mar 2015 10:51:10 -0600	[thread overview]
Message-ID: <1427302270-10178-1-git-send-email-dsahern@gmail.com> (raw)

363b785f38 added synthesized fork events and set a thread's parent id
to itself. Since we are already processing /proc/<pid>/status the ppid
can be determined properly. Make it so.

Performance impact measured on a sparc based T5-8 (1024 CPUs):
$ ps -efL | wc -l
20185

Current code:
$ time perf record -o perf-no-ppid.data -e cpu-clock -F 1000 -a -v -BN -- usleep 1
mmap size 532480B
[ perf record: Woken up 0 times to write data ]
failed to write feature 9
[ perf record: Captured and wrote 0.000 MB perf-no-ppid.data ]

real	0m26.144s
user	0m0.452s
sys	0m25.564s

With PPID patch:
$ time ./perf_ppid record -o perf-ppid.data -e cpu-clock -F 1000 -a -v -BN -- usleep 1
mmap size 532480B
[ perf record: Woken up 0 times to write data ]
failed to write feature 9
[ perf record: Captured and wrote 0.000 MB perf-ppid.data ]

real	0m25.743s
user	0m0.268s
sys	0m25.368s

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
---
v2:
- removed loop in place of 1 read and processing a buffer

 tools/perf/util/event.c | 178 ++++++++++++++++++++++++++++++------------------
 1 file changed, 110 insertions(+), 68 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index d5efa5092ce6..e98cbba56033 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -49,70 +49,115 @@ static struct perf_sample synth_sample = {
 	.period	   = 1,
 };
 
-static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)
+/*
+ * Assumes that the first 4095 bytes of /proc/pid/stat contains
+ * the comm, tgid and ppid.
+ */
+static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
+				    pid_t *tgid, pid_t *ppid)
 {
 	char filename[PATH_MAX];
-	char bf[BUFSIZ];
-	FILE *fp;
-	size_t size = 0;
-	pid_t tgid = -1;
+	char bf[4096];
+	int fd;
+	size_t size = 0, n;
+	char *nl, *name, *tgids, *ppids;
+
+	*tgid = -1;
+	*ppid = -1;
 
 	snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
 
-	fp = fopen(filename, "r");
-	if (fp == NULL) {
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
 		pr_debug("couldn't open %s\n", filename);
-		return 0;
+		return -1;
 	}
 
-	while (!comm[0] || (tgid < 0)) {
-		if (fgets(bf, sizeof(bf), fp) == NULL) {
-			pr_warning("couldn't get COMM and pgid, malformed %s\n",
-				   filename);
-			break;
-		}
-
-		if (memcmp(bf, "Name:", 5) == 0) {
-			char *name = bf + 5;
-			while (*name && isspace(*name))
-				++name;
-			size = strlen(name) - 1;
-			if (size >= len)
-				size = len - 1;
-			memcpy(comm, name, size);
-			comm[size] = '\0';
-
-		} else if (memcmp(bf, "Tgid:", 5) == 0) {
-			char *tgids = bf + 5;
-			while (*tgids && isspace(*tgids))
-				++tgids;
-			tgid = atoi(tgids);
-		}
+	n = read(fd, bf, sizeof(bf) - 1);
+	close(fd);
+	if (n <= 0) {
+		pr_warning("Couldn't get COMM, tigd and ppid for pid %d\n",
+			   pid);
+		return -1;
 	}
+	bf[n] = '\0';
 
-	fclose(fp);
+	name = strstr(bf, "Name:");
+	tgids = strstr(bf, "Tgid:");
+	ppids = strstr(bf, "PPid:");
+
+	if (name) {
+		name += 5;  /* strlen("Name:") */
+
+		while (*name && isspace(*name))
+			++name;
+
+		nl = strchr(name, '\n');
+		if (nl)
+			*nl = '\0';
+
+		size = strlen(name);
+		if (size >= len)
+			size = len - 1;
+		memcpy(comm, name, size);
+		comm[size] = '\0';
+	} else
+		pr_debug("Name: string not found for pid %d\n", pid);
+
+	if (tgids) {
+		tgids += 5;  /* strlen("Tgid:") */
+		while (*tgids && isspace(*tgids))
+			++tgids;
+
+		nl = strchr(tgids, '\n');
+		if (nl)
+			*nl = '\0';
+
+		*tgid = atoi(tgids);
+
+	} else
+		pr_debug("Tgid: string not found for pid %d\n", pid);
+
+	if (ppids) {
+		ppids += 5;  /* strlen("PPid:") */
+
+		while (*ppids && isspace(*ppids))
+			++ppids;
+
+		nl = strchr(ppids, '\n');
+		if (nl)
+			*nl = '\0';
+
+		*ppid = atoi(ppids);
+	} else
+		pr_debug("PPid: string not found for pid %d\n", pid);
 
-	return tgid;
+	return 0;
 }
 
-static pid_t perf_event__prepare_comm(union perf_event *event, pid_t pid,
-					 struct machine *machine)
+static int perf_event__prepare_comm(union perf_event *event, pid_t pid,
+				    struct machine *machine,
+				    pid_t *tgid, pid_t *ppid)
 {
 	size_t size;
-	pid_t tgid;
+
+	*ppid = -1;
 
 	memset(&event->comm, 0, sizeof(event->comm));
 
-	if (machine__is_host(machine))
-		tgid = perf_event__get_comm_tgid(pid, event->comm.comm,
-						 sizeof(event->comm.comm));
-	else
-		tgid = machine->pid;
+	if (machine__is_host(machine)) {
+		if (perf_event__get_comm_ids(pid, event->comm.comm,
+					     sizeof(event->comm.comm),
+					     tgid, ppid) != 0) {
+			return -1;
+		}
+	} else
+		*tgid = machine->pid;
 
-	if (tgid < 0)
-		goto out;
+	if (*tgid < 0)
+		return -1;
 
-	event->comm.pid = tgid;
+	event->comm.pid = *tgid;
 	event->comm.header.type = PERF_RECORD_COMM;
 
 	size = strlen(event->comm.comm) + 1;
@@ -122,36 +167,35 @@ static pid_t perf_event__prepare_comm(union perf_event *event, pid_t pid,
 				(sizeof(event->comm.comm) - size) +
 				machine->id_hdr_size);
 	event->comm.tid = pid;
-out:
-	return tgid;
+
+	return 0;
 }
 
-static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
-					 union perf_event *event, pid_t pid,
-					 perf_event__handler_t process,
-					 struct machine *machine)
+static int perf_event__synthesize_comm(struct perf_tool *tool,
+				       union perf_event *event, pid_t pid,
+				       perf_event__handler_t process,
+				       struct machine *machine,
+				       pid_t *tgid, pid_t *ppid)
 {
-	pid_t tgid = perf_event__prepare_comm(event, pid, machine);
-
-	if (tgid == -1)
-		goto out;
+	if (perf_event__prepare_comm(event, pid, machine, tgid, ppid) != 0)
+		return -1;
 
 	if (process(tool, event, &synth_sample, machine) != 0)
 		return -1;
 
-out:
-	return tgid;
+	return 0;
 }
 
 static int perf_event__synthesize_fork(struct perf_tool *tool,
-				       union perf_event *event, pid_t pid,
-				       pid_t tgid, perf_event__handler_t process,
+				       union perf_event *event,
+				       pid_t pid, pid_t tgid, pid_t ppid,
+				       perf_event__handler_t process,
 				       struct machine *machine)
 {
 	memset(&event->fork, 0, sizeof(event->fork) + machine->id_hdr_size);
 
-	event->fork.ppid = tgid;
-	event->fork.ptid = tgid;
+	event->fork.ppid = ppid;
+	event->fork.ptid = ppid;
 	event->fork.pid  = tgid;
 	event->fork.tid  = pid;
 	event->fork.header.type = PERF_RECORD_FORK;
@@ -343,14 +387,12 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 	char filename[PATH_MAX];
 	DIR *tasks;
 	struct dirent dirent, *next;
-	pid_t tgid;
+	pid_t tgid, ppid;
 
 	/* special case: only send one comm event using passed in pid */
 	if (!full) {
-		tgid = perf_event__synthesize_comm(tool, comm_event, pid,
-						   process, machine);
-
-		if (tgid == -1)
+		if (perf_event__synthesize_comm(tool, comm_event, pid,
+						process, machine, &tgid, &ppid) != 0)
 			return -1;
 
 		return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
@@ -378,12 +420,12 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 		if (*end)
 			continue;
 
-		tgid = perf_event__prepare_comm(comm_event, _pid, machine);
-		if (tgid == -1)
+		if (perf_event__prepare_comm(comm_event, _pid, machine,
+					     &tgid, &ppid) != 0)
 			return -1;
 
 		if (perf_event__synthesize_fork(tool, fork_event, _pid, tgid,
-						process, machine) < 0)
+						ppid, process, machine) < 0)
 			return -1;
 		/*
 		 * Send the prepared comm event
-- 
2.1.0


             reply	other threads:[~2015-03-25 16:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-25 16:51 David Ahern [this message]
2015-03-25 19:15 ` [PATCH v2] perf tool: Fix ppid for synthesized fork events Don Zickus
2015-03-25 19:55   ` David Ahern
2015-03-25 20:26     ` Don Zickus
2015-03-26 21:11     ` Don Zickus
2015-03-26 21:37       ` David Ahern
2015-03-27 13:10         ` Don Zickus
2015-03-27 14:03           ` David Ahern
2015-03-27 14:20             ` Arnaldo Carvalho de Melo
2015-03-27 19:49               ` Don Zickus
2015-03-27 20:09                 ` Arnaldo Carvalho de Melo
2015-03-27 20:10                 ` David Ahern
2015-03-27 20:25                   ` Don Zickus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1427302270-10178-1-git-send-email-dsahern@gmail.com \
    --to=dsahern@gmail.com \
    --cc=acme@kernel.org \
    --cc=dzickus@redhat.com \
    --cc=jmario@redhat.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.