From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751464Ab1IZNnI (ORCPT ); Mon, 26 Sep 2011 09:43:08 -0400 Received: from mail-gx0-f174.google.com ([209.85.161.174]:44353 "EHLO mail-gx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750933Ab1IZNnG (ORCPT ); Mon, 26 Sep 2011 09:43:06 -0400 Message-ID: <4E808166.7090807@gmail.com> Date: Mon, 26 Sep 2011 07:43:02 -0600 From: David Ahern User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0.2) Gecko/20110906 Thunderbird/6.0.2 MIME-Version: 1.0 To: Jiri Olsa CC: acme@redhat.com, eric.dumazet@gmail.com, fweisbec@gmail.com, a.p.zijlstra@chello.nl, mingo@elte.hu, paulus@samba.org, linux-kernel@vger.kernel.org, rostedt@goodmis.org, nhorman@tuxdriver.com Subject: Re: [PATCHv2 1/2] perf tools: Collect tracing event data files directly References: <20110925133406.GB2702@jolsa.brq.redhat.com> <1317028312-5156-1-git-send-email-jolsa@redhat.com> <1317028312-5156-2-git-send-email-jolsa@redhat.com> In-Reply-To: <1317028312-5156-2-git-send-email-jolsa@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/26/2011 03:11 AM, Jiri Olsa wrote: > Changing the way the event files are searched by quering specified > event files directly, instead of walking the events directory. > > Hopefully this way is more straightforward and faster. > > Signed-off-by: Jiri Olsa > --- > tools/perf/util/parse-events.h | 1 + > tools/perf/util/trace-event-info.c | 231 +++++++++++++++-------------------- > 2 files changed, 100 insertions(+), 132 deletions(-) > > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > index 2f8e375..1ea02ad 100644 > --- a/tools/perf/util/parse-events.h > +++ b/tools/perf/util/parse-events.h > @@ -15,6 +15,7 @@ struct option; > struct tracepoint_path { > char *system; > char *name; > + bool done; > struct tracepoint_path *next; > }; > > diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c > index 3403f81..cf8f89e 100644 > --- a/tools/perf/util/trace-event-info.c > +++ b/tools/perf/util/trace-event-info.c > @@ -238,136 +238,98 @@ static void read_header_files(void) > put_tracing_file(path); > } > > -static bool name_in_tp_list(char *sys, struct tracepoint_path *tps) > +static char *get_format_file(char *sys, char *name) > { > - while (tps) { > - if (!strcmp(sys, tps->name)) > - return true; > - tps = tps->next; > - } > - > - return false; > -} > - > -static void copy_event_system(const char *sys, struct tracepoint_path *tps) > -{ > - struct dirent *dent; > - struct stat st; > - char *format; > - DIR *dir; > - int count = 0; > - int ret; > - > - dir = opendir(sys); > - if (!dir) > - die("can't read directory '%s'", sys); > - > - while ((dent = readdir(dir))) { > - if (dent->d_type != DT_DIR || > - strcmp(dent->d_name, ".") == 0 || > - strcmp(dent->d_name, "..") == 0 || > - !name_in_tp_list(dent->d_name, tps)) > - continue; > - format = malloc_or_die(strlen(sys) + strlen(dent->d_name) + 10); > - sprintf(format, "%s/%s/format", sys, dent->d_name); > - ret = stat(format, &st); > - free(format); > - if (ret < 0) > - continue; > - count++; > - } > - > - write_or_die(&count, 4); > + char *file, *path; > > - rewinddir(dir); > - while ((dent = readdir(dir))) { > - if (dent->d_type != DT_DIR || > - strcmp(dent->d_name, ".") == 0 || > - strcmp(dent->d_name, "..") == 0 || > - !name_in_tp_list(dent->d_name, tps)) > - continue; > - format = malloc_or_die(strlen(sys) + strlen(dent->d_name) + 10); > - sprintf(format, "%s/%s/format", sys, dent->d_name); > - ret = stat(format, &st); > + path = get_tracing_file("events"); > + if (!path) > + return NULL; > > - if (ret >= 0) > - record_file(format, 8); > + /* > + * The '+ 5' is for '/' we add to the patch, plus NULL byte, > + * and one safety byte ;) > + */ > + file = malloc_or_die(strlen(path) + strlen(sys) + > + strlen(name) + strlen("format") + 5); Why not go the simple route and just use PATH_MAX? David > > - free(format); > - } > - closedir(dir); > + sprintf(file, "%s/%s/%s/format", path, sys, name); > + return file; > } > > -static void read_ftrace_files(struct tracepoint_path *tps) > +static void put_format_file(char *file) > { > - char *path; > - > - path = get_tracing_file("events/ftrace"); > - > - copy_event_system(path, tps); > - > - put_tracing_file(path); > + free(file); > } > > -static bool system_in_tp_list(char *sys, struct tracepoint_path *tps) > +/* > + * Walk tracepoint event objects and store them. > + * Only those matching the sys parameter are stored > + * and marked as done. > + */ > +static void read_event_files_system(struct tracepoint_path *tps, > + const char *sys) > { > + off_t count_pos; > + u32 count = 0; > + > + /* Place for number of events under single system. */ > + count_pos = lseek(output_fd, 0, SEEK_CUR); > + write_or_die(&count, 4); > + > while (tps) { > - if (!strcmp(sys, tps->system)) > - return true; > + if ((!tps->done) && > + (!strcmp(sys, tps->system))) { > + char *file; > + > + file = get_format_file(tps->system, tps->name); > + if (file) { > + record_file(file, 8); > + count++; > + } > + > + put_format_file(file); > + tps->done = 1; > + } > + > tps = tps->next; > } > > - return false; > + if (pwrite(output_fd, &count, 4, count_pos) < 0) > + die("pwrite"); > } > > +/* > + * We have all needed tracepoint event files stored in > + * the tracepoint_path objects. > + * > + * - first we store ftrace system events > + * - then we walk all '!done' event objects > + * and process them > + */ > static void read_event_files(struct tracepoint_path *tps) > { > - struct dirent *dent; > - struct stat st; > - char *path; > - char *sys; > - DIR *dir; > - int count = 0; > - int ret; > - > - path = get_tracing_file("events"); > + off_t count_pos; > + u32 count = 0; > > - dir = opendir(path); > - if (!dir) > - die("can't read directory '%s'", path); > - > - while ((dent = readdir(dir))) { > - if (dent->d_type != DT_DIR || > - strcmp(dent->d_name, ".") == 0 || > - strcmp(dent->d_name, "..") == 0 || > - strcmp(dent->d_name, "ftrace") == 0 || > - !system_in_tp_list(dent->d_name, tps)) > - continue; > - count++; > - } > + read_event_files_system(tps, "ftrace"); > > + /* systems count */ > + count_pos = lseek(output_fd, 0, SEEK_CUR); > write_or_die(&count, 4); > > - rewinddir(dir); > - while ((dent = readdir(dir))) { > - if (dent->d_type != DT_DIR || > - strcmp(dent->d_name, ".") == 0 || > - strcmp(dent->d_name, "..") == 0 || > - strcmp(dent->d_name, "ftrace") == 0 || > - !system_in_tp_list(dent->d_name, tps)) > - continue; > - sys = malloc_or_die(strlen(path) + strlen(dent->d_name) + 2); > - sprintf(sys, "%s/%s", path, dent->d_name); > - ret = stat(sys, &st); > - if (ret >= 0) { > - write_or_die(dent->d_name, strlen(dent->d_name) + 1); > - copy_event_system(sys, tps); > + while (tps) { > + if (!tps->done) { > + write_or_die(tps->system, strlen(tps->system) + 1); > + read_event_files_system(tps, tps->system); > + count++; > } > - free(sys); > + > + tps = tps->next; > } > > - closedir(dir); > - put_tracing_file(path); > + if (pwrite(output_fd, &count, 4, count_pos) < 0) > + die("write"); > } > > static void read_proc_kallsyms(void) > @@ -387,6 +349,37 @@ static void read_proc_kallsyms(void) > record_file(path, 4); > } > > +static void tracing_data_header(void) > +{ > + char buf[50]; > + > + /* just guessing this is someone's birthday.. ;) */ > + buf[0] = 23; > + buf[1] = 8; > + buf[2] = 68; > + memcpy(buf + 3, "tracing", 7); > + > + write_or_die(buf, 10); > + > + write_or_die(VERSION, strlen(VERSION) + 1); > + > + /* save endian */ > + if (bigendian()) > + buf[0] = 1; > + else > + buf[0] = 0; > + > + write_or_die(buf, 1); > + > + /* save size of long */ > + buf[0] = sizeof(long); > + write_or_die(buf, 1); > + > + /* save page_size */ > + page_size = sysconf(_SC_PAGESIZE); > + write_or_die(&page_size, 4); > +} > + > static void read_ftrace_printk(void) > { > unsigned int size; > @@ -441,7 +434,6 @@ bool have_tracepoints(struct list_head *pattrs) > > int read_tracing_data(int fd, struct list_head *pattrs) > { > - char buf[BUFSIZ]; > struct tracepoint_path *tps = get_tracepoints_path(pattrs); > > /* > @@ -452,33 +444,8 @@ int read_tracing_data(int fd, struct list_head *pattrs) > > output_fd = fd; > > - buf[0] = 23; > - buf[1] = 8; > - buf[2] = 68; > - memcpy(buf + 3, "tracing", 7); > - > - write_or_die(buf, 10); > - > - write_or_die(VERSION, strlen(VERSION) + 1); > - > - /* save endian */ > - if (bigendian()) > - buf[0] = 1; > - else > - buf[0] = 0; > - > - write_or_die(buf, 1); > - > - /* save size of long */ > - buf[0] = sizeof(long); > - write_or_die(buf, 1); > - > - /* save page_size */ > - page_size = sysconf(_SC_PAGESIZE); > - write_or_die(&page_size, 4); > - > + tracing_data_header(); > read_header_files(); > - read_ftrace_files(tps); > read_event_files(tps); > read_proc_kallsyms(); > read_ftrace_printk();