From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933762AbcBYUOV (ORCPT ); Thu, 25 Feb 2016 15:14:21 -0500 Received: from mail.kernel.org ([198.145.29.136]:51822 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933737AbcBYUOU (ORCPT ); Thu, 25 Feb 2016 15:14:20 -0500 Date: Thu, 25 Feb 2016 17:14:13 -0300 From: Arnaldo Carvalho de Melo To: Steven Rostedt Cc: LKML , Ingo Molnar , Namhyung Kim Subject: Re: [PATCH] tools lib traceevent: Split pevent_print_event() into specific functionality functions Message-ID: <20160225201413.GS8720@kernel.org> References: <20160225122033.5adf3863@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160225122033.5adf3863@gandalf.local.home> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Thu, Feb 25, 2016 at 12:20:33PM -0500, Steven Rostedt escreveu: > Currently there's a single function that is used to display a record's > data in human readable format. That's pevent_print_event(). > Unfortunately, this gives little room for adding other output within > the line without updating that function call. > > I've decided to split that function into 3 parts. > > pevent_print_event_task() which prints the task comm, pid and the CPU > pevent_print_event_time() which outputs the record's timestamp > pevent_print_event_data() which outputs the rest of the event data. > > pevent_print_event() now simply calls these three functions. > > To save time from doing the search for event from the record's type, I > created a new helper function called pevent_find_event_by_record(), > which returns the record's event, and this event has to be passed to > the above functions. > > Signed-off-by: Steven Rostedt > --- > diff --git a/event-parse.c b/event-parse.c > index 18e93e430b8b..559d77381a60 100644 > --- a/event-parse.c > +++ b/event-parse.c Isn't this on tools/lib/traceevent? - Arnaldo > @@ -5364,41 +5364,45 @@ static bool is_timestamp_in_us(char *trace_clock, bool use_trace_clock) > return false; > } > > -void pevent_print_event(struct pevent *pevent, struct trace_seq *s, > - struct pevent_record *record, bool use_trace_clock) > +/** > + * pevent_find_event_by_record - return the event from a given record > + * @pevent: a handle to the pevent > + * @record: The record to get the event from > + * > + * Returns the associated event for a given record, or NULL if non is > + * is found. > + */ > +struct event_format * > +pevent_find_event_by_record(struct pevent *pevent, struct pevent_record *record) > { > - static const char *spaces = " "; /* 20 spaces */ > - struct event_format *event; > - unsigned long secs; > - unsigned long usecs; > - unsigned long nsecs; > - const char *comm; > - void *data = record->data; > int type; > - int pid; > - int len; > - int p; > - bool use_usec_format; > - > - use_usec_format = is_timestamp_in_us(pevent->trace_clock, > - use_trace_clock); > - if (use_usec_format) { > - secs = record->ts / NSECS_PER_SEC; > - nsecs = record->ts - secs * NSECS_PER_SEC; > - } > > if (record->size < 0) { > do_warning("ug! negative record size %d", record->size); > - return; > + return NULL; > } > > - type = trace_parse_common_type(pevent, data); > + type = trace_parse_common_type(pevent, record->data); > > - event = pevent_find_event(pevent, type); > - if (!event) { > - do_warning("ug! no event found for type %d", type); > - return; > - } > + return pevent_find_event(pevent, type); > +} > + > +/** > + * pevent_print_event_task - Write the event task comm, pid and CPU > + * @pevent: a handle to the pevent > + * @s: the trace_seq to write to > + * @event: the handle to the record's event > + * @record: The record to get the event from > + * > + * Writes the tasks comm, pid and CPU to @s. > + */ > +void pevent_print_event_task(struct pevent *pevent, struct trace_seq *s, > + struct event_format *event, > + struct pevent_record *record) > +{ > + void *data = record->data; > + const char *comm; > + int pid; > > pid = parse_common_pid(pevent, data); > comm = find_cmdline(pevent, pid); > @@ -5406,9 +5410,43 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s, > if (pevent->latency_format) { > trace_seq_printf(s, "%8.8s-%-5d %3d", > comm, pid, record->cpu); > - pevent_data_lat_fmt(pevent, s, record); > } else > trace_seq_printf(s, "%16s-%-5d [%03d]", comm, pid, record->cpu); > +} > + > +/** > + * pevent_print_event_time - Write the event timestamp > + * @pevent: a handle to the pevent > + * @s: the trace_seq to write to > + * @event: the handle to the record's event > + * @record: The record to get the event from > + * @use_trace_clock: Set to parse according to the @pevent->trace_clock > + * > + * Writes the timestamp of the record into @s. > + */ > +void pevent_print_event_time(struct pevent *pevent, struct trace_seq *s, > + struct event_format *event, > + struct pevent_record *record, > + bool use_trace_clock) > +{ > + unsigned long secs; > + unsigned long usecs; > + unsigned long nsecs; > + int p; > + bool use_usec_format; > + > + use_usec_format = is_timestamp_in_us(pevent->trace_clock, > + use_trace_clock); > + if (use_usec_format) { > + secs = record->ts / NSECS_PER_SEC; > + nsecs = record->ts - secs * NSECS_PER_SEC; > + } > + > + if (pevent->latency_format) { > + trace_seq_printf(s, " %3d", record->cpu); > + pevent_data_lat_fmt(pevent, s, record); > + } else > + trace_seq_printf(s, " [%03d]", record->cpu); > > if (use_usec_format) { > if (pevent->flags & PEVENT_NSEC_OUTPUT) { > @@ -5424,11 +5462,28 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s, > p = 6; > } > > - trace_seq_printf(s, " %5lu.%0*lu: %s: ", > - secs, p, usecs, event->name); > + trace_seq_printf(s, " %5lu.%0*lu:", secs, p, usecs); > } else > - trace_seq_printf(s, " %12llu: %s: ", > - record->ts, event->name); > + trace_seq_printf(s, " %12llu:", record->ts); > +} > + > +/** > + * pevent_print_event_data - Write the event data section > + * @pevent: a handle to the pevent > + * @s: the trace_seq to write to > + * @event: the handle to the record's event > + * @record: The record to get the event from > + * > + * Writes the parsing of the record's data to @s. > + */ > +void pevent_print_event_data(struct pevent *pevent, struct trace_seq *s, > + struct event_format *event, > + struct pevent_record *record) > +{ > + static const char *spaces = " "; /* 20 spaces */ > + int len; > + > + trace_seq_printf(s, " %s: ", event->name); > > /* Space out the event names evenly. */ > len = strlen(event->name); > @@ -5438,6 +5493,23 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s, > pevent_event_info(s, event, record); > } > > +void pevent_print_event(struct pevent *pevent, struct trace_seq *s, > + struct pevent_record *record, bool use_trace_clock) > +{ > + struct event_format *event; > + > + event = pevent_find_event_by_record(pevent, record); > + if (!event) { > + do_warning("ug! no event found for type %d", > + trace_parse_common_type(pevent, record->data)); > + return; > + } > + > + pevent_print_event_task(pevent, s, event, record); > + pevent_print_event_time(pevent, s, event, record, use_trace_clock); > + pevent_print_event_data(pevent, s, event, record); > +} > + > static int events_id_cmp(const void *a, const void *b) > { > struct event_format * const * ea = a; > diff --git a/event-parse.h b/event-parse.h > index ea653580c197..a3fd9b28a5eb 100644 > --- a/event-parse.h > +++ b/event-parse.h > @@ -632,6 +632,16 @@ int pevent_register_print_string(struct pevent *pevent, const char *fmt, > unsigned long long addr); > int pevent_pid_is_registered(struct pevent *pevent, int pid); > > +void pevent_print_event_task(struct pevent *pevent, struct trace_seq *s, > + struct event_format *event, > + struct pevent_record *record); > +void pevent_print_event_time(struct pevent *pevent, struct trace_seq *s, > + struct event_format *event, > + struct pevent_record *record, > + bool use_trace_clock); > +void pevent_print_event_data(struct pevent *pevent, struct trace_seq *s, > + struct event_format *event, > + struct pevent_record *record); > void pevent_print_event(struct pevent *pevent, struct trace_seq *s, > struct pevent_record *record, bool use_trace_clock); > > @@ -698,6 +708,9 @@ struct event_format *pevent_find_event(struct pevent *pevent, int id); > struct event_format * > pevent_find_event_by_name(struct pevent *pevent, const char *sys, const char *name); > > +struct event_format * > +pevent_find_event_by_record(struct pevent *pevent, struct pevent_record *record); > + > void pevent_data_lat_fmt(struct pevent *pevent, > struct trace_seq *s, struct pevent_record *record); > int pevent_data_type(struct pevent *pevent, struct pevent_record *rec);