tools lib traceevent: Updates from trace-cmd

PATCH In review Steven Rostedt 3 participants 14 messages Feb 9, 2016 – Aug 6, 2016

lore

Share & tools

lore

Steven Rostedt

author

2016-02-09 20:42 UTC #0 lore raw

cover tools lib traceevent: Updates from trace-cmd

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(-)

Steven Rostedt

author

2016-02-09 20:43 UTC #1 lore raw

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

From: "Chaos.Chen" <[email protected]>

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 <[email protected]>

[ fixed incrementing "secs" instead of decrementing it ]

Signed-off-by Steven Rostedt <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 5 +++++
 1 file changed, 5 insertions(+)

tools/lib/traceevent/event-parse.c +5 -0

index c3bd294a63d1..a6f1ce779e05 100644
@@ -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;
 		}

Signature · 1 line

2.6.4

tip-bot for Chaos.Chen

merge bot

2016-03-05 08:19 UTC #2 lore raw

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

tools lib traceevent: Fix time stamp rounding issue

Commit-ID:  21a30100453516004905d4d5f0806ebaffa95131
Gitweb:     http://git.kernel.org/tip/21a30100453516004905d4d5f0806ebaffa95131
Author:     Chaos.Chen <[email protected]>
AuthorDate: Tue, 9 Feb 2016 15:40:14 -0500
Committer:  Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>

Cc Andrew Morton <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ fixed incrementing "secs" instead of decrementing it ]

Signed-off-by Steven Rostedt <[email protected]>

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 5 +++++
 1 file changed, 5 insertions(+)

tools/lib/traceevent/event-parse.c +5 -0

index 9a1e48a..ce59f48 100644
@@ -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;
 		}

Steven Rostedt

author

2016-02-09 20:42 UTC #3 lore raw

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

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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

Signed-off-by Steven Rostedt <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 4 ++--
 tools/lib/traceevent/event-parse.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

tools/lib/traceevent/event-parse.c +2 -2

index a6f1ce779e05..f49b266b3978 100644
@@ -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;

tools/lib/traceevent/event-parse.h +2 -0

index 706d9bc24066..4373c5190076 100644
@@ -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,

Signature · 1 line

2.6.4

Arnaldo Carvalho de Melo

2016-03-02 12:20 UTC #4 lore raw

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

Em Tue, Feb 09, 2016 at 03:40:15PM -0500, Steven Rostedt escreveu:

Quoted 3 lines

From: "Steven Rostedt (Red Hat)" <[email protected]>
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

Quoted 37 lines

Signed-off-by Steven Rostedt <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 4 ++--
 tools/lib/traceevent/event-parse.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

tools/lib/traceevent/event-parse.c +2 -2

index a6f1ce779e05..f49b266b3978 100644
@@ -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;

tools/lib/traceevent/event-parse.h +2 -0

index 706d9bc24066..4373c5190076 100644
@@ -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,

Signature · 1 line

2.6.4

Steven Rostedt

author

2016-08-05 18:20 UTC #5 lore raw

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

Quoted 8 lines · On Wed, 2 Mar 2016 09:20:04 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Tue, Feb 09, 2016 at 03:40:15PM -0500, Steven Rostedt escreveu:
Quoted 3 lines
From: "Steven Rostedt (Red Hat)" <[email protected]>
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

Quoted 2 lines

- Arnaldo

Arnaldo Carvalho de Melo

2016-08-05 18:37 UTC #6 lore raw

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

Em Fri, Aug 05, 2016 at 02:19:42PM -0400, Steven Rostedt escreveu:

Quoted 6 lines

Quoted 3 lines · On Wed, 2 Mar 2016 09:20:04 -0300 Arnaldo Carvalho de Melo <[email protected]>…
Em Tue, Feb 09, 2016 at 03:40:15PM -0500, Steven Rostedt escreveu:
Quoted 2 lines
From: "Steven Rostedt (Red Hat)" <[email protected]>

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
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

Steven Rostedt

author

2016-08-05 19:15 UTC #7 lore raw

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

Quoted 12 lines · On Fri, 5 Aug 2016 15:36:55 -0300 Arnaldo Carvalho de Melo <[email protected]>…

[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

Quoted 6 lines

#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

Arnaldo Carvalho de Melo

2016-08-06 20:11 UTC #8 lore raw

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

Em Fri, Aug 05, 2016 at 03:15:28PM -0400, Steven Rostedt escreveu:

Quoted 19 lines

Quoted 15 lines · On Fri, 5 Aug 2016 15:36:55 -0300 Arnaldo Carvalho de Melo <[email protected]>…
[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]$

Quoted 7 lines

Quoted 3 lines
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.

Quoted 3 lines

Thanks!
-- Steve

Steven Rostedt

author

2016-02-09 20:43 UTC #9 lore raw

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

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 3 +++
 1 file changed, 3 insertions(+)

tools/lib/traceevent/event-parse.c +3 -0

index f49b266b3978..e39a616f695b 100644
@@ -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;

Signature · 1 line

2.6.4

tip-bot for Steven Rostedt (Red Hat)

merge bot

2016-03-05 08:19 UTC #10 lore raw

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

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

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

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

Fix process_hex() while at it.

Signed-off-by Steven Rostedt <[email protected]>

Cc Andrew Morton <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 3 +++
 1 file changed, 3 insertions(+)

tools/lib/traceevent/event-parse.c +3 -0

index ce59f48..fb790aa 100644
@@ -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;

Steven Rostedt

author

2016-02-09 20:42 UTC #11 lore raw

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

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/lib/traceevent/event-parse.c +1 -1

index e39a616f695b..81a2efadafb2 100644
@@ -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;

Signature · 1 line

2.6.4

tip-bot for Steven Rostedt (Red Hat)

merge bot

2016-03-05 08:20 UTC #12 lore raw

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

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

Commit-ID:  a66673a07e260807f570db8f08a9c1207932c665
Gitweb:     http://git.kernel.org/tip/a66673a07e260807f570db8f08a9c1207932c665
Author:     Steven Rostedt (Red Hat) <[email protected]>
AuthorDate: Tue, 9 Feb 2016 15:40:17 -0500
Committer:  Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>

Cc Andrew Morton <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/lib/traceevent/event-parse.c +1 -1

index fb790aa..865dea5 100644
@@ -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;

Steven Rostedt

author

2016-03-01 21:48 UTC #13 lore raw

In reply to Steven Rostedt

Hi Arnaldo,

I think these got missed.

-- Steve

Quoted 30 lines · On Tue, 09 Feb 2016 15:40:13 -0500 Steven Rostedt <[email protected]> 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(-)