On 3/30/15 1:40 PM, Arnaldo Carvalho de Melo wrote:
@@ -81,6 +84,7 @@ static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)name = strstr(bf, "Name:"); tgids = strstr(bf, "Tgid:"); + ppids = strstr(bf, "PPid:");can't we make this: ppids = strstr(tgids, "PPid:"); To speed it up a teeny little bit? 8-)
Sure, I thought about that as well, but it puts an assumption on order of the data in the file. Why have the assumption?
if (name) { name += 5; /* strlen("Name:") */ @@ -109,32 +113,51 @@ static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len) if (nl) *nl = '\0'; - tgid = atoi(tgids); + *tgid = atoi(tgids); } else pr_debug("Tgid: string not found for pid %d\n", pid); - return tgid; + if (ppids) { + ppids += 5; /* strlen("PPid:") */ + + while (*ppids && isspace(*ppids)) + ++ppids;The above could be simplified to: while (isspace(*ppids)) ++ppids;
sure.
$ cat isspace.c #include <ctype.h> #include <stdio.h> int main(void) { return printf("isspace('\\0')=%d\n", isspace('\0')); } $ ./isspace isspace('\0')=0 $+ nl = strchr(ppids, '\n'); + if (nl) + *nl = '\0';We also don't need to find and zero this '\n', as: $ cat atoi.c #include <string.h> #include <stdio.h> int main(void) { return printf("atoi(\"1234\\n\")=%d\n", atoi("1234\n")); } $ ./atoi atoi("1234\n")=1234 $
ok. another assumption on implementation. fine with taking it out.
<SNIP>+ 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;Somebody, I think PeterZ and also Ingo, routinely asks for having {} on the else part of an if that has {}, please do so.
ack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
