[PATCH 0/4] tools lib traceevent: Updates from trace-cmd

[PATCH 0/4] tools lib traceevent: Updates from trace-cmd

[Steven Rostedt]

Hi Arnaldo,

I did a diff of what I have in my trace-cmd library and found there
were a few patches that need to be brought over to tools/lib/traceevent
as well.

I have a repo based off of 4.5-rc3, but feel free to pull the patches
in directly.

-- Steve


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
traceevent/updates

Head SHA1: b5c65b951fe641c6a82667c11d617c3c26240d17


Chaos.Chen (1):
      tools lib traceevent: Fix time stamp rounding issue

Steven Rostedt (Red Hat) (3):
      tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number
      tools lib traceevent: Set int_array fields to NULL if freeing from error
      tools lib traceevent: Fix output of %llu for 64 bit values read on 32 bit machines

----
 tools/lib/traceevent/event-parse.c | 10 +++++++++-
 tools/lib/traceevent/event-parse.h |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

[PATCH 1/4] tools lib traceevent: Fix time stamp rounding issue

[Steven Rostedt]

[-- Attachment #1: 0001-tools-lib-traceevent-Fix-time-stamp-rounding-issue.patch --]
[-- Type: text/plain, Size: 1095 bytes --]

From: "Chaos.Chen" <rainboy1215@gmail.com>

When rounding to microseconds, if the timestamp subsecond is between
.999999500 and .999999999, it is rounded to .1000000, when it should instead
increment the second counter due to the overflow.

For example, if the timestamp is 1234.999999501 instead of seeing:

 1235.000000

we see

 1234.1000000

Signed-off-by: Chaos.Chen <rainboy1215@gmail.com>
[ fixed incrementing "secs" instead of decrementing it ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index c3bd294a63d1..a6f1ce779e05 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5387,6 +5387,11 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
 			p = 9;
 		} else {
 			usecs = (nsecs + 500) / NSECS_PER_USEC;
+			/* To avoid usecs larger than 1 sec */
+			if (usecs >= 1000000) {
+				usecs -= 1000000;
+				secs++;
+			}
 			p = 6;
 		}
 
-- 
2.6.4

[PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Steven Rostedt]

[-- Attachment #1: 0002-tools-lib-traceevent-Use-USECS_PER_SEC-instead-of-ha.patch --]
[-- Type: text/plain, Size: 1314 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Instead of using 1000000, define a USECS_PER_SEC macro and use that instead.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 4 ++--
 tools/lib/traceevent/event-parse.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index a6f1ce779e05..f49b266b3978 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5388,8 +5388,8 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
 		} else {
 			usecs = (nsecs + 500) / NSECS_PER_USEC;
 			/* To avoid usecs larger than 1 sec */
-			if (usecs >= 1000000) {
-				usecs -= 1000000;
+			if (usecs >= USECS_PER_SEC) {
+				usecs -= USECS_PER_SEC;
 				secs++;
 			}
 			p = 6;
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 706d9bc24066..4373c5190076 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -175,6 +175,8 @@ struct pevent_plugin_option {
 #define NSECS_PER_SEC		1000000000ULL
 #define NSECS_PER_USEC		1000ULL
 
+#define USECS_PER_SEC		1000000ULL
+
 enum format_flags {
 	FIELD_IS_ARRAY		= 1,
 	FIELD_IS_POINTER	= 2,
-- 
2.6.4

[PATCH 3/4] tools lib traceevent: Set int_array fields to NULL if freeing from error

[Steven Rostedt]

[-- Attachment #1: 0003-tools-lib-traceevent-Set-int_array-fields-to-NULL-if.patch --]
[-- Type: text/plain, Size: 1185 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Had a bug where on error of parsing __print_array() where the
fields are freed after they were allocated, but since they were not
set to NULL, the freeing of the arg also tried to free the already
freed fields causing a double free.

Fix process_hex() while at it.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index f49b266b3978..e39a616f695b 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2634,6 +2634,7 @@ process_hex(struct event_format *event, struct print_arg *arg, char **tok)
 
 free_field:
 	free_arg(arg->hex.field);
+	arg->hex.field = NULL;
 out:
 	*tok = NULL;
 	return EVENT_ERROR;
@@ -2658,8 +2659,10 @@ process_int_array(struct event_format *event, struct print_arg *arg, char **tok)
 
 free_size:
 	free_arg(arg->int_array.count);
+	arg->int_array.count = NULL;
 free_field:
 	free_arg(arg->int_array.field);
+	arg->int_array.field = NULL;
 out:
 	*tok = NULL;
 	return EVENT_ERROR;
-- 
2.6.4

[PATCH 4/4] tools lib traceevent: Fix output of %llu for 64 bit values read on 32 bit machines

[Steven Rostedt]

[-- Attachment #1: 0004-tools-lib-traceevent-Fix-output-of-llu-for-64-bit-va.patch --]
[-- Type: text/plain, Size: 940 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

When a long value is read on 32 bit machines for 64 bit output, the parsing
needs to change "%lu" into "%llu", as the value is read natively.

Unfortunately, if "%llu" is already there, the code will add another "l" to
it and fail to parse it properly.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index e39a616f695b..81a2efadafb2 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4974,7 +4974,7 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 						break;
 					}
 				}
-				if (pevent->long_size == 8 && ls &&
+				if (pevent->long_size == 8 && ls == 1 &&
 				    sizeof(long) != 8) {
 					char *p;
 
-- 
2.6.4

Re: [PATCH 0/4] tools lib traceevent: Updates from trace-cmd

[Steven Rostedt]

Hi Arnaldo,

I think these got missed.

-- Steve


On Tue, 09 Feb 2016 15:40:13 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Hi Arnaldo,
> 
> I did a diff of what I have in my trace-cmd library and found there
> were a few patches that need to be brought over to tools/lib/traceevent
> as well.
> 
> I have a repo based off of 4.5-rc3, but feel free to pull the patches
> in directly.
> 
> -- Steve
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> traceevent/updates
> 
> Head SHA1: b5c65b951fe641c6a82667c11d617c3c26240d17
> 
> 
> Chaos.Chen (1):
>       tools lib traceevent: Fix time stamp rounding issue
> 
> Steven Rostedt (Red Hat) (3):
>       tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number
>       tools lib traceevent: Set int_array fields to NULL if freeing from error
>       tools lib traceevent: Fix output of %llu for 64 bit values read on 32 bit machines
> 
> ----
>  tools/lib/traceevent/event-parse.c | 10 +++++++++-
>  tools/lib/traceevent/event-parse.h |  2 ++
>  2 files changed, 11 insertions(+), 1 deletion(-)

Re: [PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Arnaldo Carvalho de Melo]

Em Tue, Feb 09, 2016 at 03:40:15PM -0500, Steven Rostedt escreveu:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> Instead of using 1000000, define a USECS_PER_SEC macro and use that instead.

Applying, but the kernel uses USEC_PER_SEC, NSEC_PER_SEC, etc, at some
point I'll try and get those same headers in tools/include/ and make all
of tools/ use the same convention as the kernel.

- Arnaldo
 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  tools/lib/traceevent/event-parse.c | 4 ++--
>  tools/lib/traceevent/event-parse.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index a6f1ce779e05..f49b266b3978 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -5388,8 +5388,8 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
>  		} else {
>  			usecs = (nsecs + 500) / NSECS_PER_USEC;
>  			/* To avoid usecs larger than 1 sec */
> -			if (usecs >= 1000000) {
> -				usecs -= 1000000;
> +			if (usecs >= USECS_PER_SEC) {
> +				usecs -= USECS_PER_SEC;
>  				secs++;
>  			}
>  			p = 6;
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index 706d9bc24066..4373c5190076 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -175,6 +175,8 @@ struct pevent_plugin_option {
>  #define NSECS_PER_SEC		1000000000ULL
>  #define NSECS_PER_USEC		1000ULL
>  
> +#define USECS_PER_SEC		1000000ULL
> +
>  enum format_flags {
>  	FIELD_IS_ARRAY		= 1,
>  	FIELD_IS_POINTER	= 2,
> -- 
> 2.6.4
> 

[tip:perf/core] tools lib traceevent: Fix time stamp rounding issue

[tip-bot for Chaos.Chen]

Commit-ID:  21a30100453516004905d4d5f0806ebaffa95131
Gitweb:     http://git.kernel.org/tip/21a30100453516004905d4d5f0806ebaffa95131
Author:     Chaos.Chen <rainboy1215@gmail.com>
AuthorDate: Tue, 9 Feb 2016 15:40:14 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 3 Mar 2016 11:10:37 -0300

tools lib traceevent: Fix time stamp rounding issue

When rounding to microseconds, if the timestamp subsecond is between
.999999500 and .999999999, it is rounded to .1000000, when it should
instead increment the second counter due to the overflow.

For example, if the timestamp is 1234.999999501 instead of seeing:

  1235.000000

we see:

  1234.1000000

Signed-off-by: Chaos.Chen <rainboy1215@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20160209204236.824426460@goodmis.org
[ fixed incrementing "secs" instead of decrementing it ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 9a1e48a..ce59f48 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5429,6 +5429,11 @@ void pevent_print_event_time(struct pevent *pevent, struct trace_seq *s,
 			p = 9;
 		} else {
 			usecs = (nsecs + 500) / NSECS_PER_USEC;
+			/* To avoid usecs larger than 1 sec */
+			if (usecs >= 1000000) {
+				usecs -= 1000000;
+				secs++;
+			}
 			p = 6;
 		}
 

[tip:perf/core] tools lib traceevent: Set int_array fields to NULL if freeing from error

[tip-bot for Steven Rostedt (Red Hat)]

Commit-ID:  9ec72eafee61f68cd57310a99db129ffb71302f3
Gitweb:     http://git.kernel.org/tip/9ec72eafee61f68cd57310a99db129ffb71302f3
Author:     Steven Rostedt (Red Hat) <rostedt@goodmis.org>
AuthorDate: Tue, 9 Feb 2016 15:40:16 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 3 Mar 2016 11:10:38 -0300

tools lib traceevent: Set int_array fields to NULL if freeing from error

Had a bug where on error of parsing __print_array() where the fields are
freed after they were allocated, but since they were not set to NULL,
the freeing of the arg also tried to free the already freed fields
causing a double free.

Fix process_hex() while at it.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20160209204237.188327674@goodmis.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index ce59f48..fb790aa 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2635,6 +2635,7 @@ process_hex(struct event_format *event, struct print_arg *arg, char **tok)
 
 free_field:
 	free_arg(arg->hex.field);
+	arg->hex.field = NULL;
 out:
 	*tok = NULL;
 	return EVENT_ERROR;
@@ -2659,8 +2660,10 @@ process_int_array(struct event_format *event, struct print_arg *arg, char **tok)
 
 free_size:
 	free_arg(arg->int_array.count);
+	arg->int_array.count = NULL;
 free_field:
 	free_arg(arg->int_array.field);
+	arg->int_array.field = NULL;
 out:
 	*tok = NULL;
 	return EVENT_ERROR;

[tip:perf/core] tools lib traceevent: Fix output of %llu for 64 bit values read on 32 bit machines

[tip-bot for Steven Rostedt (Red Hat)]

Commit-ID:  a66673a07e260807f570db8f08a9c1207932c665
Gitweb:     http://git.kernel.org/tip/a66673a07e260807f570db8f08a9c1207932c665
Author:     Steven Rostedt (Red Hat) <rostedt@goodmis.org>
AuthorDate: Tue, 9 Feb 2016 15:40:17 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 3 Mar 2016 11:10:38 -0300

tools lib traceevent: Fix output of %llu for 64 bit values read on 32 bit machines

When a long value is read on 32 bit machines for 64 bit output, the
parsing needs to change "%lu" into "%llu", as the value is read
natively.

Unfortunately, if "%llu" is already there, the code will add another "l"
to it and fail to parse it properly.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/20160209204237.337024613@goodmis.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index fb790aa..865dea5 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4978,7 +4978,7 @@ static void pretty_print(struct trace_seq *s, void *data, int size, struct event
 						break;
 					}
 				}
-				if (pevent->long_size == 8 && ls &&
+				if (pevent->long_size == 8 && ls == 1 &&
 				    sizeof(long) != 8) {
 					char *p;
 

Re: [PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Steven Rostedt]

On Wed, 2 Mar 2016 09:20:04 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Tue, Feb 09, 2016 at 03:40:15PM -0500, Steven Rostedt escreveu:
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 
> > Instead of using 1000000, define a USECS_PER_SEC macro and use that instead.  
> 
> Applying, but the kernel uses USEC_PER_SEC, NSEC_PER_SEC, etc, at some
> point I'll try and get those same headers in tools/include/ and make all
> of tools/ use the same convention as the kernel.

Was this ever applied? Anyway, I'll also go ahead and convert this to
the non plural versions to match the kernel.

-- Steve

> 
> - Arnaldo

Re: [PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Arnaldo Carvalho de Melo]

Em Fri, Aug 05, 2016 at 02:19:42PM -0400, Steven Rostedt escreveu:
> On Wed, 2 Mar 2016 09:20:04 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Tue, Feb 09, 2016 at 03:40:15PM -0500, Steven Rostedt escreveu:
> > > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > > 
> > > Instead of using 1000000, define a USECS_PER_SEC macro and use that instead.  
> > 
> > Applying, but the kernel uses USEC_PER_SEC, NSEC_PER_SEC, etc, at some
> > point I'll try and get those same headers in tools/include/ and make all
> > of tools/ use the same convention as the kernel.
> 
> Was this ever applied? Anyway, I'll also go ahead and convert this to
> the non plural versions to match the kernel.

Fell thru the cracks :-\ The other patches in the series were applied
tho.

I'm introducing this and removing the definitions I have in
tools/perf/perf.h for NSEC_PER_SEC and NSEC_PER_USEC:

[acme@jouet linux]$ cat tools/include/linux/time64.h 
#ifndef _TOOLS_LINUX_TIME64_H
#define _TOOLS_LINUX_TIME64_H

#define MSEC_PER_SEC	1000L
#define USEC_PER_MSEC	1000L
#define NSEC_PER_USEC	1000L
#define NSEC_PER_MSEC	1000000L
#define USEC_PER_SEC	1000000L
#define NSEC_PER_SEC	1000000000L
#define FSEC_PER_SEC	1000000000000000LL

#endif /* _TOOLS_LINUX_TIME64_H */
[acme@jouet linux]$ 

So the header to include is the same as in the kernel, the constants as
well. We can go on adding more stuff from include/linux/time64.h as
tools use it.

- Arnaldo

Re: [PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Steven Rostedt]

On Fri, 5 Aug 2016 15:36:55 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:


> [acme@jouet linux]$ cat tools/include/linux/time64.h 
> #ifndef _TOOLS_LINUX_TIME64_H
> #define _TOOLS_LINUX_TIME64_H
> 
> #define MSEC_PER_SEC	1000L
> #define USEC_PER_MSEC	1000L
> #define NSEC_PER_USEC	1000L
> #define NSEC_PER_MSEC	1000000L
> #define USEC_PER_SEC	1000000L
> #define NSEC_PER_SEC	1000000000L
> #define FSEC_PER_SEC	1000000000000000LL
> 
> #endif /* _TOOLS_LINUX_TIME64_H */
> [acme@jouet linux]$ 
> 
> So the header to include is the same as in the kernel, the constants as
> well. We can go on adding more stuff from include/linux/time64.h as
> tools use it.

OK, can you modify the scripting-engines/trace-event-*.c to use that
too. I'm going to move the macros locally into event-parse.c, as I work
to make that ready to be a separate library.

Thanks!

-- Steve

Re: [PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Arnaldo Carvalho de Melo]

Em Fri, Aug 05, 2016 at 03:15:28PM -0400, Steven Rostedt escreveu:
> On Fri, 5 Aug 2016 15:36:55 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> 
> > [acme@jouet linux]$ cat tools/include/linux/time64.h 
> > #ifndef _TOOLS_LINUX_TIME64_H
> > #define _TOOLS_LINUX_TIME64_H
> > 
> > #define MSEC_PER_SEC	1000L
> > #define USEC_PER_MSEC	1000L
> > #define NSEC_PER_USEC	1000L
> > #define NSEC_PER_MSEC	1000000L
> > #define USEC_PER_SEC	1000000L
> > #define NSEC_PER_SEC	1000000000L
> > #define FSEC_PER_SEC	1000000000000000LL
> > 
> > #endif /* _TOOLS_LINUX_TIME64_H */
> > [acme@jouet linux]$ 
> > 
> > So the header to include is the same as in the kernel, the constants as
> > well. We can go on adding more stuff from include/linux/time64.h as
> > tools use it.
> 
> OK, can you modify the scripting-engines/trace-event-*.c to use that
> too. I'm going to move the macros locally into event-parse.c, as I work
> to make that ready to be a separate library.

Ok, and I fix a few more, pushing to perf/core.
 
> Thanks!
> 
> -- Steve