All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/3] perf python: Add support to access tracepoint fields
@ 2016-07-15  7:29 Jiri Olsa
  2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Jiri Olsa @ 2016-07-15  7:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Steven Rostedt (Red Hat),
	Jiri Pirko, Songshan Gong, lkml, David Ahern, Ingo Molnar,
	Namhyung Kim, Peter Zijlstra

hi,
adding support to access tracepoint fields in python scripts.

v2 changes:
  - most of the patches is already pulled in,
    this is just leftover
  - fixed is_printable_array [Steven]
  - making is_printable_array global
  - attached unrelated fix 3/3

With this patchset it's possible to access tracepoint fields
in event python object like:

  print "time %u prev_comm=%s prev_pid=%d prev_prio=%d prev_state=0x%x ==> next_comm=%s next_pid=%d next_prio=%d" % (
         event.sample_time,
         event.prev_comm,
         event.prev_pid,
         event.prev_prio,
         event.prev_state,
         event.next_comm,
         event.next_pid,
         event.next_prio)

Also available in:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/fixes

thanks,
jirka


Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Songshan Gong <gongss@linux.vnet.ibm.com>
---
Jiri Olsa (3):
      perf script python: Fix string vs byte array resolving
      perf tools: Make is_printable_array global
      tools lib api fs: Use base 0 in filename__read_ull

 tools/lib/api/fs/fs.c                                  |  7 ++++++-
 tools/perf/util/python.c                               | 12 ------------
 tools/perf/util/scripting-engines/trace-event-python.c | 25 ++++++++++++++++++-------
 tools/perf/util/util.c                                 | 14 ++++++++++++++
 tools/perf/util/util.h                                 |  1 +
 5 files changed, 39 insertions(+), 20 deletions(-)

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

* [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15  7:29 [PATCHv2 0/3] perf python: Add support to access tracepoint fields Jiri Olsa
@ 2016-07-15  7:29 ` Jiri Olsa
  2016-07-15  7:32   ` Jiri Olsa
                     ` (3 more replies)
  2016-07-15  7:29 ` [PATCH 2/3] perf tools: Make is_printable_array global Jiri Olsa
  2016-07-15  7:29 ` [PATCH 3/3] tools lib api fs: Use base 0 in filename__read_ull Jiri Olsa
  2 siblings, 4 replies; 18+ messages in thread
From: Jiri Olsa @ 2016-07-15  7:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Steven Rostedt (Red Hat),
	Jiri Pirko, lkml, David Ahern, Ingo Molnar, Namhyung Kim,
	Peter Zijlstra

Jirka reported that python code returns all arrays as strings.
This makes impossible to get all items for byte array tracepoint
field containing 0x00 value item.

Fixing this by scanning full length of the array and returning
it as PyByteArray object in case non printable byte is found.

Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Cc: Jiri Pirko <jiri@mellanox.com>
Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../util/scripting-engines/trace-event-python.c    | 37 ++++++++++++++++++----
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 6ac6b7a33f42..1bc995de5a6d 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -386,6 +386,19 @@ exit:
 	return pylist;
 }
 
+static int is_printable_array(char *p, unsigned int len)
+{
+	unsigned int i;
+
+	if (p[len - 1] == 0)
+		len--;
+
+	for (i = 0; i < len - 1; i++)
+		if (!isprint(p[i]) && !isspace(p[i]))
+			return 0;
+
+	return 1;
+}
 
 static void python_process_tracepoint(struct perf_sample *sample,
 				      struct perf_evsel *evsel,
@@ -457,14 +470,26 @@ static void python_process_tracepoint(struct perf_sample *sample,
 		pydict_set_item_string_decref(dict, "common_callchain", callchain);
 	}
 	for (field = event->format.fields; field; field = field->next) {
-		if (field->flags & FIELD_IS_STRING) {
-			int offset;
+		unsigned int offset, len;
+		unsigned long long val;
+
+		if (field->flags & FIELD_IS_ARRAY) {
+			offset = field->offset;
+			len    = field->size;
 			if (field->flags & FIELD_IS_DYNAMIC) {
-				offset = *(int *)(data + field->offset);
+				val     = pevent_read_number(scripting_context->pevent,
+							     data + offset, len);
+				offset  = val;
+				len     = offset >> 16;
 				offset &= 0xffff;
-			} else
-				offset = field->offset;
-			obj = PyString_FromString((char *)data + offset);
+			}
+			if (field->flags & FIELD_IS_STRING &&
+			    is_printable_array(data + offset, len)) {
+				obj = PyString_FromString((char *) data + offset);
+			} else {
+				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
+				field->flags &= ~FIELD_IS_STRING;
+			}
 		} else { /* FIELD_IS_NUMERIC */
 			obj = get_field_numeric_entry(event, field, data);
 		}
-- 
2.4.11

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

* [PATCH 2/3] perf tools: Make is_printable_array global
  2016-07-15  7:29 [PATCHv2 0/3] perf python: Add support to access tracepoint fields Jiri Olsa
  2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
@ 2016-07-15  7:29 ` Jiri Olsa
  2016-07-15 15:31   ` Steven Rostedt
  2016-07-15  7:29 ` [PATCH 3/3] tools lib api fs: Use base 0 in filename__read_ull Jiri Olsa
  2 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2016-07-15  7:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Steven Rostedt (Red Hat),
	lkml, David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra

It's used from 2 objects in perf, so it's better
to keep just one copy.

Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
Link: http://lkml.kernel.org/n/tip-dss4lxnywysgpyjd8p7paffg@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/python.c                               | 12 ------------
 tools/perf/util/scripting-engines/trace-event-python.c | 14 --------------
 tools/perf/util/util.c                                 | 14 ++++++++++++++
 tools/perf/util/util.h                                 |  1 +
 4 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index d32f97033718..a5fbc012e3df 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -295,18 +295,6 @@ static bool is_tracepoint(struct pyrf_event *pevent)
 	return pevent->evsel->attr.type == PERF_TYPE_TRACEPOINT;
 }
 
-static int is_printable_array(char *p, unsigned int len)
-{
-	unsigned int i;
-
-	for (i = 0; i < len; i++) {
-		if (!isprint(p[i]) && !isspace(p[i]))
-			return 0;
-	}
-
-	return 1;
-}
-
 static PyObject*
 tracepoint_field(struct pyrf_event *pe, struct format_field *field)
 {
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 1bc995de5a6d..e0203b979474 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -386,20 +386,6 @@ exit:
 	return pylist;
 }
 
-static int is_printable_array(char *p, unsigned int len)
-{
-	unsigned int i;
-
-	if (p[len - 1] == 0)
-		len--;
-
-	for (i = 0; i < len - 1; i++)
-		if (!isprint(p[i]) && !isspace(p[i]))
-			return 0;
-
-	return 1;
-}
-
 static void python_process_tracepoint(struct perf_sample *sample,
 				      struct perf_evsel *evsel,
 				      struct addr_location *al)
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 5f44a21955cd..7ca4719787d0 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -746,3 +746,17 @@ void print_binary(unsigned char *data, size_t len,
 	}
 	printer(BINARY_PRINT_DATA_END, -1, extra);
 }
+
+int is_printable_array(char *p, unsigned int len)
+{
+	unsigned int i;
+
+	if (p[len - 1] == 0)
+		len--;
+
+	for (i = 0; i < len - 1; i++)
+		if (!isprint(p[i]) && !isspace(p[i]))
+			return 0;
+
+	return 1;
+}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 6178cab82374..66d24bdbac33 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -364,4 +364,5 @@ void print_binary(unsigned char *data, size_t len,
 extern int sched_getcpu(void);
 #endif
 
+int is_printable_array(char *p, unsigned int len);
 #endif /* GIT_COMPAT_UTIL_H */
-- 
2.4.11

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

* [PATCH 3/3] tools lib api fs: Use base 0 in filename__read_ull
  2016-07-15  7:29 [PATCHv2 0/3] perf python: Add support to access tracepoint fields Jiri Olsa
  2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
  2016-07-15  7:29 ` [PATCH 2/3] perf tools: Make is_printable_array global Jiri Olsa
@ 2016-07-15  7:29 ` Jiri Olsa
  2016-07-16 20:45   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2016-07-15  7:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Songshan Gong, lkml, David Ahern, Ingo Molnar, Namhyung Kim,
	Peter Zijlstra

By using 0 for base, the strtoull detects the
base automatically (see man strtoull).

ATM we have just one user of this function, the
cpu__get_max_freq function reading cpuinfo_max_freq
sysfs. It should not get affected by this change.

Cc: Songshan Gong <gongss@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/n/tip-f56zxnejrzt9gfernqb034z8@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/api/fs/fs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 08556cf2c70d..ba7094b945ff 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -283,6 +283,11 @@ int filename__read_int(const char *filename, int *value)
 	return err;
 }
 
+/*
+ * Parses @value out of @filename with strtoull.
+ * By using 0 for base, the strtoull detects the
+ * base automatically (see man strtoull).
+ */
 int filename__read_ull(const char *filename, unsigned long long *value)
 {
 	char line[64];
@@ -292,7 +297,7 @@ int filename__read_ull(const char *filename, unsigned long long *value)
 		return -1;
 
 	if (read(fd, line, sizeof(line)) > 0) {
-		*value = strtoull(line, NULL, 10);
+		*value = strtoull(line, NULL, 0);
 		if (*value != ULLONG_MAX)
 			err = 0;
 	}
-- 
2.4.11

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
@ 2016-07-15  7:32   ` Jiri Olsa
  2016-07-15 15:30   ` Steven Rostedt
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2016-07-15  7:32 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Arnaldo Carvalho de Melo, Steven Rostedt (Red Hat),
	lkml, David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra

On Fri, Jul 15, 2016 at 09:29:55AM +0200, Jiri Olsa wrote:
> Jirka reported that python code returns all arrays as strings.
> This makes impossible to get all items for byte array tracepoint
> field containing 0x00 value item.
> 
> Fixing this by scanning full length of the array and returning
> it as PyByteArray object in case non printable byte is found.
> 
> Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> Cc: Jiri Pirko <jiri@mellanox.com>

forgot:

Reported-by: Jiri Pirko <jiri@mellanox.com>

Jirka,
could I please also have your tested-by on this version?

thanks,
jirka

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
  2016-07-15  7:32   ` Jiri Olsa
@ 2016-07-15 15:30   ` Steven Rostedt
  2016-07-15 15:51   ` Jiri Pirko
  2016-07-15 17:18   ` Steven Rostedt
  3 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2016-07-15 15:30 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Jiri Pirko, lkml, David Ahern,
	Ingo Molnar, Namhyung Kim, Peter Zijlstra

On Fri, 15 Jul 2016 09:29:55 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> Jirka reported that python code returns all arrays as strings.
> This makes impossible to get all items for byte array tracepoint
> field containing 0x00 value item.
> 
> Fixing this by scanning full length of the array and returning
> it as PyByteArray object in case non printable byte is found.
> 
> Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> Cc: Jiri Pirko <jiri@mellanox.com>
> Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

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

* Re: [PATCH 2/3] perf tools: Make is_printable_array global
  2016-07-15  7:29 ` [PATCH 2/3] perf tools: Make is_printable_array global Jiri Olsa
@ 2016-07-15 15:31   ` Steven Rostedt
  0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2016-07-15 15:31 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, David Ahern, Ingo Molnar,
	Namhyung Kim, Peter Zijlstra

On Fri, 15 Jul 2016 09:29:56 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> It's used from 2 objects in perf, so it's better
> to keep just one copy.
> 
> Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> Link: http://lkml.kernel.org/n/tip-dss4lxnywysgpyjd8p7paffg@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
  2016-07-15  7:32   ` Jiri Olsa
  2016-07-15 15:30   ` Steven Rostedt
@ 2016-07-15 15:51   ` Jiri Pirko
  2016-07-15 16:02     ` Steven Rostedt
  2016-07-15 17:18   ` Steven Rostedt
  3 siblings, 1 reply; 18+ messages in thread
From: Jiri Pirko @ 2016-07-15 15:51 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Steven Rostedt (Red Hat),
	lkml, David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra

Fri, Jul 15, 2016 at 09:29:55AM CEST, jolsa@kernel.org wrote:
>Jirka reported that python code returns all arrays as strings.
>This makes impossible to get all items for byte array tracepoint
>field containing 0x00 value item.
>
>Fixing this by scanning full length of the array and returning
>it as PyByteArray object in case non printable byte is found.
>
>Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
>Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>---
> .../util/scripting-engines/trace-event-python.c    | 37 ++++++++++++++++++----
> 1 file changed, 31 insertions(+), 6 deletions(-)
>
>diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
>index 6ac6b7a33f42..1bc995de5a6d 100644
>--- a/tools/perf/util/scripting-engines/trace-event-python.c
>+++ b/tools/perf/util/scripting-engines/trace-event-python.c
>@@ -386,6 +386,19 @@ exit:
> 	return pylist;
> }
> 
>+static int is_printable_array(char *p, unsigned int len)
>+{
>+	unsigned int i;
>+
>+	if (p[len - 1] == 0)
>+		len--;
>+
>+	for (i = 0; i < len - 1; i++)
>+		if (!isprint(p[i]) && !isspace(p[i]))
>+			return 0;


for "AA\1\0" this returns "1" although that should return "0".

orig len 4
decremented len 3
for:
0 1

index 2 would not be inspected. Or am I missing something?

I think that the for check should be "i < len"

Thanks.



>+
>+	return 1;
>+}
> 
> static void python_process_tracepoint(struct perf_sample *sample,
> 				      struct perf_evsel *evsel,
>@@ -457,14 +470,26 @@ static void python_process_tracepoint(struct perf_sample *sample,
> 		pydict_set_item_string_decref(dict, "common_callchain", callchain);
> 	}
> 	for (field = event->format.fields; field; field = field->next) {
>-		if (field->flags & FIELD_IS_STRING) {
>-			int offset;
>+		unsigned int offset, len;
>+		unsigned long long val;
>+
>+		if (field->flags & FIELD_IS_ARRAY) {
>+			offset = field->offset;
>+			len    = field->size;
> 			if (field->flags & FIELD_IS_DYNAMIC) {
>-				offset = *(int *)(data + field->offset);
>+				val     = pevent_read_number(scripting_context->pevent,
>+							     data + offset, len);
>+				offset  = val;
>+				len     = offset >> 16;
> 				offset &= 0xffff;
>-			} else
>-				offset = field->offset;
>-			obj = PyString_FromString((char *)data + offset);
>+			}
>+			if (field->flags & FIELD_IS_STRING &&
>+			    is_printable_array(data + offset, len)) {
>+				obj = PyString_FromString((char *) data + offset);
>+			} else {
>+				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
>+				field->flags &= ~FIELD_IS_STRING;
>+			}
> 		} else { /* FIELD_IS_NUMERIC */
> 			obj = get_field_numeric_entry(event, field, data);
> 		}
>-- 
>2.4.11
>

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15 15:51   ` Jiri Pirko
@ 2016-07-15 16:02     ` Steven Rostedt
  2016-07-15 16:13       ` Jiri Olsa
  0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2016-07-15 16:02 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, David Ahern,
	Ingo Molnar, Namhyung Kim, Peter Zijlstra

On Fri, 15 Jul 2016 17:51:10 +0200
Jiri Pirko <jiri@mellanox.com> wrote:

> Fri, Jul 15, 2016 at 09:29:55AM CEST, jolsa@kernel.org wrote:
> >Jirka reported that python code returns all arrays as strings.
> >This makes impossible to get all items for byte array tracepoint
> >field containing 0x00 value item.
> >
> >Fixing this by scanning full length of the array and returning
> >it as PyByteArray object in case non printable byte is found.
> >
> >Cc: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> >Cc: Jiri Pirko <jiri@mellanox.com>
> >Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
> >Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >---
> > .../util/scripting-engines/trace-event-python.c    | 37 ++++++++++++++++++----
> > 1 file changed, 31 insertions(+), 6 deletions(-)
> >
> >diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> >index 6ac6b7a33f42..1bc995de5a6d 100644
> >--- a/tools/perf/util/scripting-engines/trace-event-python.c
> >+++ b/tools/perf/util/scripting-engines/trace-event-python.c
> >@@ -386,6 +386,19 @@ exit:
> > 	return pylist;
> > }
> > 
> >+static int is_printable_array(char *p, unsigned int len)
> >+{
> >+	unsigned int i;
> >+
> >+	if (p[len - 1] == 0)
> >+		len--;
> >+
> >+	for (i = 0; i < len - 1; i++)
> >+		if (!isprint(p[i]) && !isspace(p[i]))
> >+			return 0;  
> 
> 
> for "AA\1\0" this returns "1" although that should return "0".
> 
> orig len 4
> decremented len 3
> for:
> 0 1
> 
> index 2 would not be inspected. Or am I missing something?
> 
> I think that the for check should be "i < len"

Yes it should be. I think we got the two solutions mixed up.

With the above len--, it should be i < len, but when we did the check
for zero at the end, we needed the i < len - 1

-- Steve

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15 16:02     ` Steven Rostedt
@ 2016-07-15 16:13       ` Jiri Olsa
  2016-07-15 16:37         ` Arnaldo Carvalho de Melo
  2016-07-15 17:16         ` Steven Rostedt
  0 siblings, 2 replies; 18+ messages in thread
From: Jiri Olsa @ 2016-07-15 16:13 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jiri Pirko, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
	David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra

On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:

SNIP

> > for "AA\1\0" this returns "1" although that should return "0".
> > 
> > orig len 4
> > decremented len 3
> > for:
> > 0 1
> > 
> > index 2 would not be inspected. Or am I missing something?
> > 
> > I think that the for check should be "i < len"
> 
> Yes it should be. I think we got the two solutions mixed up.
> 
> With the above len--, it should be i < len, but when we did the check
> for zero at the end, we needed the i < len - 1

ugh right.. should be 'i < len' check in the for loop,

there's also the patch 2/3 that needs to be changed

Arnaldo,
please let me know if you need me to resend this.

thanks,
jirka

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15 16:13       ` Jiri Olsa
@ 2016-07-15 16:37         ` Arnaldo Carvalho de Melo
  2016-07-15 16:38           ` Arnaldo Carvalho de Melo
  2016-07-15 17:16         ` Steven Rostedt
  1 sibling, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-07-15 16:37 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Steven Rostedt, Jiri Pirko, Jiri Olsa, lkml, David Ahern,
	Ingo Molnar, Namhyung Kim, Peter Zijlstra

Em Fri, Jul 15, 2016 at 06:13:10PM +0200, Jiri Olsa escreveu:
> On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:
> 
> SNIP
> 
> > > for "AA\1\0" this returns "1" although that should return "0".
> > > 
> > > orig len 4
> > > decremented len 3
> > > for:
> > > 0 1
> > > 
> > > index 2 would not be inspected. Or am I missing something?
> > > 
> > > I think that the for check should be "i < len"
> > 
> > Yes it should be. I think we got the two solutions mixed up.
> > 
> > With the above len--, it should be i < len, but when we did the check
> > for zero at the end, we needed the i < len - 1
> 
> ugh right.. should be 'i < len' check in the for loop,
> 
> there's also the patch 2/3 that needs to be changed
> 
> Arnaldo,
> please let me know if you need me to resend this.

So I need to drop those, even with Rostedt's acked-by? Ok, please
resend, hopefully this time with a Tested-by from the reporter?

- Arnaldo

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15 16:37         ` Arnaldo Carvalho de Melo
@ 2016-07-15 16:38           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-07-15 16:38 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Steven Rostedt, Jiri Pirko, Jiri Olsa, lkml, David Ahern,
	Ingo Molnar, Namhyung Kim, Peter Zijlstra

Em Fri, Jul 15, 2016 at 01:37:31PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jul 15, 2016 at 06:13:10PM +0200, Jiri Olsa escreveu:
> > On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:
> > 
> > SNIP
> > 
> > > > for "AA\1\0" this returns "1" although that should return "0".
> > > > 
> > > > orig len 4
> > > > decremented len 3
> > > > for:
> > > > 0 1
> > > > 
> > > > index 2 would not be inspected. Or am I missing something?
> > > > 
> > > > I think that the for check should be "i < len"
> > > 
> > > Yes it should be. I think we got the two solutions mixed up.
> > > 
> > > With the above len--, it should be i < len, but when we did the check
> > > for zero at the end, we needed the i < len - 1
> > 
> > ugh right.. should be 'i < len' check in the for loop,
> > 
> > there's also the patch 2/3 that needs to be changed
> > 
> > Arnaldo,
> > please let me know if you need me to resend this.
> 
> So I need to drop those, even with Rostedt's acked-by? Ok, please
> resend, hopefully this time with a Tested-by from the reporter?

I removed the first two, as the second needs context from the first,
kept the third one, so please resend just the first two.

- Arnaldo

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15 16:13       ` Jiri Olsa
  2016-07-15 16:37         ` Arnaldo Carvalho de Melo
@ 2016-07-15 17:16         ` Steven Rostedt
  1 sibling, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2016-07-15 17:16 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Pirko, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
	David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra

On Fri, 15 Jul 2016 18:13:10 +0200
Jiri Olsa <jolsa@redhat.com> wrote:

> On Fri, Jul 15, 2016 at 12:02:31PM -0400, Steven Rostedt wrote:
> 
> SNIP
> 
> > > for "AA\1\0" this returns "1" although that should return "0".
> > > 
> > > orig len 4
> > > decremented len 3
> > > for:
> > > 0 1
> > > 
> > > index 2 would not be inspected. Or am I missing something?
> > > 
> > > I think that the for check should be "i < len"  
> > 
> > Yes it should be. I think we got the two solutions mixed up.
> > 
> > With the above len--, it should be i < len, but when we did the check
> > for zero at the end, we needed the i < len - 1  
> 
> ugh right.. should be 'i < len' check in the for loop,
> 
> there's also the patch 2/3 that needs to be changed
> 

I'm wondering if we should also add at the beginning:

	if (!len)
		return 0;

Otherwise we will be accessing out of bounds with the len-1.

-- Steve

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
                     ` (2 preceding siblings ...)
  2016-07-15 15:51   ` Jiri Pirko
@ 2016-07-15 17:18   ` Steven Rostedt
  2016-07-16 15:58     ` Jiri Olsa
  3 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2016-07-15 17:18 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Jiri Pirko, lkml, David Ahern,
	Ingo Molnar, Namhyung Kim, Peter Zijlstra

On Fri, 15 Jul 2016 09:29:55 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> +			if (field->flags & FIELD_IS_STRING &&
> +			    is_printable_array(data + offset, len)) {
> +				obj = PyString_FromString((char *) data + offset);

Hmm. As I stated, It is possible that strings can be non nul
terminated. But I'm looking here and thinking we need to make sure that
it is nul terminated.

Can PyString_FromString() handle a non nul terminated string?

-- Steve

> +			} else {
> +				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
> +				field->flags &= ~FIELD_IS_STRING;
> +			}
>  		} else { /* FIELD_IS_NUMERIC */
>  			obj = get_field_numeric_entry(event, field, data);
>  		}

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-15 17:18   ` Steven Rostedt
@ 2016-07-16 15:58     ` Jiri Olsa
  0 siblings, 0 replies; 18+ messages in thread
From: Jiri Olsa @ 2016-07-16 15:58 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Jiri Pirko, lkml,
	David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra

On Fri, Jul 15, 2016 at 01:18:40PM -0400, Steven Rostedt wrote:
> On Fri, 15 Jul 2016 09:29:55 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
> 
> > +			if (field->flags & FIELD_IS_STRING &&
> > +			    is_printable_array(data + offset, len)) {
> > +				obj = PyString_FromString((char *) data + offset);
> 
> Hmm. As I stated, It is possible that strings can be non nul
> terminated. But I'm looking here and thinking we need to make sure that
> it is nul terminated.
> 
> Can PyString_FromString() handle a non nul terminated string?

couldn't find in the doc.. but I think it's necessary,
there's no other way it could tell the end ;-)

I'll make the is_printable_array in case the final 0 is missing

thanks,
jirka

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

* [tip:perf/core] tools lib api fs: Use base 0 in filename__read_ull
  2016-07-15  7:29 ` [PATCH 3/3] tools lib api fs: Use base 0 in filename__read_ull Jiri Olsa
@ 2016-07-16 20:45   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Jiri Olsa @ 2016-07-16 20:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, namhyung, gongss, linux-kernel, acme, tglx, a.p.zijlstra,
	hpa, mingo, dsahern

Commit-ID:  db49120a32b347fb93aea3319305932657dd118c
Gitweb:     http://git.kernel.org/tip/db49120a32b347fb93aea3319305932657dd118c
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Fri, 15 Jul 2016 09:29:57 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 15 Jul 2016 13:38:05 -0300

tools lib api fs: Use base 0 in filename__read_ull

By using 0 for base, the strtoull() detects the base automatically (see
'man strtoull').

ATM we have just one user of this function, the cpu__get_max_freq
function reading the "cpuinfo_max_freq" sysfs file. It should not get
affected by this change.

Committer note:

This change seems motivated by this discussion:

"[PATCH] [RFC V1]s390/perf: fix 'start' address of module's map"
http://lkml.kernel.org/r/20160711120155.GA29929@krava

I.e. this patches paves the way for filename__read_ull() to be used in a
S/390 related fix.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Songshan Gong <gongss@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1468567797-27564-4-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/fs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 08556cf..ba7094b 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -283,6 +283,11 @@ int filename__read_int(const char *filename, int *value)
 	return err;
 }
 
+/*
+ * Parses @value out of @filename with strtoull.
+ * By using 0 for base, the strtoull detects the
+ * base automatically (see man strtoull).
+ */
 int filename__read_ull(const char *filename, unsigned long long *value)
 {
 	char line[64];
@@ -292,7 +297,7 @@ int filename__read_ull(const char *filename, unsigned long long *value)
 		return -1;
 
 	if (read(fd, line, sizeof(line)) > 0) {
-		*value = strtoull(line, NULL, 10);
+		*value = strtoull(line, NULL, 0);
 		if (*value != ULLONG_MAX)
 			err = 0;
 	}

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

* Re: [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-16 16:11 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
@ 2016-07-18 11:44   ` Jiri Pirko
  0 siblings, 0 replies; 18+ messages in thread
From: Jiri Pirko @ 2016-07-18 11:44 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, David Ahern, Ingo Molnar,
	Namhyung Kim, Peter Zijlstra, Steven Rostedt (Red Hat)

Sat, Jul 16, 2016 at 06:11:18PM CEST, jolsa@kernel.org wrote:
>Jirka reported that python code returns all arrays as strings.
>This makes impossible to get all items for byte array tracepoint
>field containing 0x00 value item.
>
>Fixing this by scanning full length of the array and returning
>it as PyByteArray object in case non printable byte is found.
>
>Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
>Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Tested-by: Jiri Pirko <jiri@mellanox.com>



>---
> .../util/scripting-engines/trace-event-python.c    | 39 ++++++++++++++++++----
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
>diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
>index 6ac6b7a33f42..7bd6da80533e 100644
>--- a/tools/perf/util/scripting-engines/trace-event-python.c
>+++ b/tools/perf/util/scripting-engines/trace-event-python.c
>@@ -386,6 +386,21 @@ exit:
> 	return pylist;
> }
> 
>+static int is_printable_array(char *p, unsigned int len)
>+{
>+	unsigned int i;
>+
>+	if (!p || !len || p[len - 1] != 0)
>+		return 0;
>+
>+	len--;
>+
>+	for (i = 0; i < len; i++) {
>+		if (!isprint(p[i]) && !isspace(p[i]))
>+			return 0;
>+	}
>+	return 1;
>+}
> 
> static void python_process_tracepoint(struct perf_sample *sample,
> 				      struct perf_evsel *evsel,
>@@ -457,14 +472,26 @@ static void python_process_tracepoint(struct perf_sample *sample,
> 		pydict_set_item_string_decref(dict, "common_callchain", callchain);
> 	}
> 	for (field = event->format.fields; field; field = field->next) {
>-		if (field->flags & FIELD_IS_STRING) {
>-			int offset;
>+		unsigned int offset, len;
>+		unsigned long long val;
>+
>+		if (field->flags & FIELD_IS_ARRAY) {
>+			offset = field->offset;
>+			len    = field->size;
> 			if (field->flags & FIELD_IS_DYNAMIC) {
>-				offset = *(int *)(data + field->offset);
>+				val     = pevent_read_number(scripting_context->pevent,
>+							     data + offset, len);
>+				offset  = val;
>+				len     = offset >> 16;
> 				offset &= 0xffff;
>-			} else
>-				offset = field->offset;
>-			obj = PyString_FromString((char *)data + offset);
>+			}
>+			if (field->flags & FIELD_IS_STRING &&
>+			    is_printable_array(data + offset, len)) {
>+				obj = PyString_FromString((char *) data + offset);
>+			} else {
>+				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
>+				field->flags &= ~FIELD_IS_STRING;
>+			}
> 		} else { /* FIELD_IS_NUMERIC */
> 			obj = get_field_numeric_entry(event, field, data);
> 		}
>-- 
>2.4.11
>

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

* [PATCH 1/3] perf script python: Fix string vs byte array resolving
  2016-07-16 16:11 [PATCHv3 0/3] perf python: Add support to access tracepoint fields Jiri Olsa
@ 2016-07-16 16:11 ` Jiri Olsa
  2016-07-18 11:44   ` Jiri Pirko
  0 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2016-07-16 16:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra,
	Steven Rostedt (Red Hat),
	Jiri Pirko

Jirka reported that python code returns all arrays as strings.
This makes impossible to get all items for byte array tracepoint
field containing 0x00 value item.

Fixing this by scanning full length of the array and returning
it as PyByteArray object in case non printable byte is found.

Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../util/scripting-engines/trace-event-python.c    | 39 ++++++++++++++++++----
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 6ac6b7a33f42..7bd6da80533e 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -386,6 +386,21 @@ exit:
 	return pylist;
 }
 
+static int is_printable_array(char *p, unsigned int len)
+{
+	unsigned int i;
+
+	if (!p || !len || p[len - 1] != 0)
+		return 0;
+
+	len--;
+
+	for (i = 0; i < len; i++) {
+		if (!isprint(p[i]) && !isspace(p[i]))
+			return 0;
+	}
+	return 1;
+}
 
 static void python_process_tracepoint(struct perf_sample *sample,
 				      struct perf_evsel *evsel,
@@ -457,14 +472,26 @@ static void python_process_tracepoint(struct perf_sample *sample,
 		pydict_set_item_string_decref(dict, "common_callchain", callchain);
 	}
 	for (field = event->format.fields; field; field = field->next) {
-		if (field->flags & FIELD_IS_STRING) {
-			int offset;
+		unsigned int offset, len;
+		unsigned long long val;
+
+		if (field->flags & FIELD_IS_ARRAY) {
+			offset = field->offset;
+			len    = field->size;
 			if (field->flags & FIELD_IS_DYNAMIC) {
-				offset = *(int *)(data + field->offset);
+				val     = pevent_read_number(scripting_context->pevent,
+							     data + offset, len);
+				offset  = val;
+				len     = offset >> 16;
 				offset &= 0xffff;
-			} else
-				offset = field->offset;
-			obj = PyString_FromString((char *)data + offset);
+			}
+			if (field->flags & FIELD_IS_STRING &&
+			    is_printable_array(data + offset, len)) {
+				obj = PyString_FromString((char *) data + offset);
+			} else {
+				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
+				field->flags &= ~FIELD_IS_STRING;
+			}
 		} else { /* FIELD_IS_NUMERIC */
 			obj = get_field_numeric_entry(event, field, data);
 		}
-- 
2.4.11

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

end of thread, other threads:[~2016-07-18 12:17 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-15  7:29 [PATCHv2 0/3] perf python: Add support to access tracepoint fields Jiri Olsa
2016-07-15  7:29 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
2016-07-15  7:32   ` Jiri Olsa
2016-07-15 15:30   ` Steven Rostedt
2016-07-15 15:51   ` Jiri Pirko
2016-07-15 16:02     ` Steven Rostedt
2016-07-15 16:13       ` Jiri Olsa
2016-07-15 16:37         ` Arnaldo Carvalho de Melo
2016-07-15 16:38           ` Arnaldo Carvalho de Melo
2016-07-15 17:16         ` Steven Rostedt
2016-07-15 17:18   ` Steven Rostedt
2016-07-16 15:58     ` Jiri Olsa
2016-07-15  7:29 ` [PATCH 2/3] perf tools: Make is_printable_array global Jiri Olsa
2016-07-15 15:31   ` Steven Rostedt
2016-07-15  7:29 ` [PATCH 3/3] tools lib api fs: Use base 0 in filename__read_ull Jiri Olsa
2016-07-16 20:45   ` [tip:perf/core] " tip-bot for Jiri Olsa
2016-07-16 16:11 [PATCHv3 0/3] perf python: Add support to access tracepoint fields Jiri Olsa
2016-07-16 16:11 ` [PATCH 1/3] perf script python: Fix string vs byte array resolving Jiri Olsa
2016-07-18 11:44   ` Jiri Pirko

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.