All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools lib traceevent: Split pevent_print_event() into specific functionality functions
@ 2016-02-25 17:20 Steven Rostedt
  2016-02-25 20:14 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2016-02-25 17:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: LKML, Ingo Molnar, Namhyung Kim

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 <rostedt@goodmis.org>
---
diff --git a/event-parse.c b/event-parse.c
index 18e93e430b8b..559d77381a60 100644
--- a/event-parse.c
+++ b/event-parse.c
@@ -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);

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools lib traceevent: Split pevent_print_event() into specific functionality functions
  2016-02-25 17:20 [PATCH] tools lib traceevent: Split pevent_print_event() into specific functionality functions Steven Rostedt
@ 2016-02-25 20:14 ` Arnaldo Carvalho de Melo
  2016-02-25 20:17   ` Steven Rostedt
  0 siblings, 1 reply; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-02-25 20:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: LKML, Ingo Molnar, Namhyung Kim

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 <rostedt@goodmis.org>
> ---
> 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);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools lib traceevent: Split pevent_print_event() into specific functionality functions
  2016-02-25 20:14 ` Arnaldo Carvalho de Melo
@ 2016-02-25 20:17   ` Steven Rostedt
  0 siblings, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2016-02-25 20:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: LKML, Ingo Molnar, Namhyung Kim

On Thu, 25 Feb 2016 17:14:13 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> 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 <rostedt@goodmis.org>
> > ---
> > 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?
> 

Oops, I did the change from trace-cmd. I'll repost.

Sorry ;-)

-- Steve

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-02-25 20:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-25 17:20 [PATCH] tools lib traceevent: Split pevent_print_event() into specific functionality functions Steven Rostedt
2016-02-25 20:14 ` Arnaldo Carvalho de Melo
2016-02-25 20:17   ` Steven Rostedt

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.