From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753579AbbCaQXA (ORCPT ); Tue, 31 Mar 2015 12:23:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56505 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752619AbbCaQW7 (ORCPT ); Tue, 31 Mar 2015 12:22:59 -0400 Date: Tue, 31 Mar 2015 12:22:57 -0400 From: Don Zickus To: David Ahern Cc: acme@kernel.org, linux-kernel@vger.kernel.org, Joe Mario , Jiri Olsa Subject: Re: [PATCH v2 1/2] perf tool: Refactor comm/tgid lookup Message-ID: <20150331162257.GC175361@redhat.com> References: <1427747758-18510-1-git-send-email-dsahern@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1427747758-18510-1-git-send-email-dsahern@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 30, 2015 at 02:35:57PM -0600, David Ahern wrote: > Rather than parsing /proc/pid/status file one line at a time, read > it into a buffer in one shot and search for all strings in one pass. > > tgid conversion also simplified -- removing the isspace walk. As > noted by Arnaldo those are not needed for atoi == strtol calls. Seeing Jiri is happy now. :-) Acked-by: Don Zickus > > Signed-off-by: David Ahern > Cc: Don Zickus > Cc: Joe Mario > Cc: Jiri Olsa > --- > v2: > - removed the isspace checks on tgid string > > tools/perf/util/event.c | 72 ++++++++++++++++++++++++++++++------------------- > 1 file changed, 44 insertions(+), 28 deletions(-) > > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c > index d5efa5092ce6..023dd3548a94 100644 > --- a/tools/perf/util/event.c > +++ b/tools/perf/util/event.c > @@ -49,48 +49,64 @@ static struct perf_sample synth_sample = { > .period = 1, > }; > > +/* > + * Assumes that the first 4095 bytes of /proc/pid/stat contains > + * the comm and tgid. > + */ > static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len) > { > char filename[PATH_MAX]; > - char bf[BUFSIZ]; > - FILE *fp; > - size_t size = 0; > + char bf[4096]; > + int fd; > + size_t size = 0, n; > pid_t tgid = -1; > + char *nl, *name, *tgids; > > 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; > } > > - 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; > - } > + n = read(fd, bf, sizeof(bf) - 1); > + close(fd); > + if (n <= 0) { > + pr_warning("Couldn't get COMM and tgid for pid %d\n", > + pid); > + return -1; > + } > + bf[n] = '\0'; > > - 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); > - } > + name = strstr(bf, "Name:"); > + tgids = strstr(bf, "Tgid:"); > + > + 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); > } > > - fclose(fp); > + if (tgids) { > + tgids += 5; /* strlen("Tgid:") */ > + tgid = atoi(tgids); > + } else { > + pr_debug("Tgid: string not found for pid %d\n", pid); > + } > > return tgid; > } > -- > 2.2.1 >