linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/5] perf/urgent fixes
@ 2013-11-19 20:41 Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 1/5] perf tools: Tag thread comm as overriden Arnaldo Carvalho de Melo
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-11-19 20:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	David Ahern, Frederic Weisbecker, Jiri Olsa, Mike Galbraith,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra, Stephane Eranian,
	Steven Rostedt, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit 801a76050bcf8d4e500eb8d048ff6265f37a61c8:

  seq_file: always clear m->count when we free m->buf (2013-11-18 19:07:53 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo

for you to fetch changes up to 6b5fa0ba4f85a8499287aefaf3f1375450c40c6d:

  tools lib traceevent: Fix conversion of pointer to integer of different size (2013-11-19 16:37:59 -0300)

----------------------------------------------------------------
perf/urgent fixes:

. Tag thread comm as overriden, showing the right comm for threads after forks,
  fix from Frederic Weisbecker.

. Fix memory leak when processing perf.data file header, from Namhyung Kim.

. Don't try to free string constant used for anonymous event groups, fix from Namhyung Kim.

. Fix use of multiple options in processing field in libtraceevent, fix from Steven Rostedt.

. Fix conversion of pointer to integer of different size in libtraceevent.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Arnaldo Carvalho de Melo (1):
      tools lib traceevent: Fix conversion of pointer to integer of different size

Frederic Weisbecker (1):
      perf tools: Tag thread comm as overriden

Namhyung Kim (2):
      perf header: Fix bogus group name
      perf header: Fix possible memory leaks in process_group_desc()

Steven Rostedt (1):
      tools lib traceevent: Fix use of multiple options in processing field

 tools/lib/traceevent/event-parse.c | 25 ++++++++++++++++++++++---
 tools/perf/util/header.c           |  6 ++++--
 tools/perf/util/thread.c           | 11 +++++------
 3 files changed, 31 insertions(+), 11 deletions(-)

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

* [PATCH 1/5] perf tools: Tag thread comm as overriden
  2013-11-19 20:41 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
@ 2013-11-19 20:41 ` Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 2/5] perf header: Fix bogus group name Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-11-19 20:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Frederic Weisbecker, David Ahern, Namhyung Kim,
	Arnaldo Carvalho de Melo

From: Frederic Weisbecker <fweisbec@gmail.com>

The problem is that when a thread overrides its default ":%pid" comm, we
forget to tag the thread comm as overriden. Hence, this overriden comm
is not inherited on future forks. Fix it.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Tested-by: David Ahern <dsahern@gmail.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/20131116010207.GA18855@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index cd8e2f592719..49eaf1d7d89d 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -70,14 +70,13 @@ int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
 	/* Override latest entry if it had no specific time coverage */
 	if (!curr->start) {
 		comm__override(curr, str, timestamp);
-		return 0;
+	} else {
+		new = comm__new(str, timestamp);
+		if (!new)
+			return -ENOMEM;
+		list_add(&new->list, &thread->comm_list);
 	}
 
-	new = comm__new(str, timestamp);
-	if (!new)
-		return -ENOMEM;
-
-	list_add(&new->list, &thread->comm_list);
 	thread->comm_set = true;
 
 	return 0;
-- 
1.8.1.4


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

* [PATCH 2/5] perf header: Fix bogus group name
  2013-11-19 20:41 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 1/5] perf tools: Tag thread comm as overriden Arnaldo Carvalho de Melo
@ 2013-11-19 20:41 ` Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 3/5] perf header: Fix possible memory leaks in process_group_desc() Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-11-19 20:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Namhyung Kim, Jiri Olsa,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung.kim@lge.com>

When processing event group descriptor in perf file header, we reuse an
allocated group name but forgot to prevent it from freeing.

Reported-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1384741244-7271-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 369c03648f88..559c516f1cec 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2078,8 +2078,10 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 		if (evsel->idx == (int) desc[i].leader_idx) {
 			evsel->leader = evsel;
 			/* {anon_group} is a dummy name */
-			if (strcmp(desc[i].name, "{anon_group}"))
+			if (strcmp(desc[i].name, "{anon_group}")) {
 				evsel->group_name = desc[i].name;
+				desc[i].name = NULL;
+			}
 			evsel->nr_members = desc[i].nr_members;
 
 			if (i >= nr_groups || nr > 0) {
-- 
1.8.1.4


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

* [PATCH 3/5] perf header: Fix possible memory leaks in process_group_desc()
  2013-11-19 20:41 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 1/5] perf tools: Tag thread comm as overriden Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 2/5] perf header: Fix bogus group name Arnaldo Carvalho de Melo
@ 2013-11-19 20:41 ` Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 4/5] tools lib traceevent: Fix use of multiple options in processing field Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-11-19 20:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Namhyung Kim, Jiri Olsa,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung.kim@lge.com>

After processing all group descriptors or encountering an error, it
frees all descriptors.  However, current logic can leak memory since it
might not traverse all descriptors.

Note that the 'i' can have different value than nr_groups when an error
occurred and it's safe to call free(desc[i].name) for every desc since
we already make it NULL when it's reused for group names.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1384741244-7271-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 559c516f1cec..1cd035708931 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2107,7 +2107,7 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 
 	ret = 0;
 out_free:
-	while ((int) --i >= 0)
+	for (i = 0; i < nr_groups; i++)
 		free(desc[i].name);
 	free(desc);
 
-- 
1.8.1.4


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

* [PATCH 4/5] tools lib traceevent: Fix use of multiple options in processing field
  2013-11-19 20:41 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2013-11-19 20:41 ` [PATCH 3/5] perf header: Fix possible memory leaks in process_group_desc() Arnaldo Carvalho de Melo
@ 2013-11-19 20:41 ` Arnaldo Carvalho de Melo
  2013-11-19 20:41 ` [PATCH 5/5] tools lib traceevent: Fix conversion of pointer to integer of different size Arnaldo Carvalho de Melo
  2013-11-20 13:32 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar
  5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-11-19 20:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Steven Rostedt, Frederic Weisbecker, Jiri Olsa,
	Namhyung Kim, Arnaldo Carvalho de Melo

From: Steven Rostedt <rostedt@goodmis.org>

Jiri Olsa reported that the scsi_dispatch_cmd_done event failed to parse
with:

  Error: expected type 5 but read 4
  Error: expected type 5 but read 4

The problem is with this part of the print_fmt:

  __print_symbolic(((REC->result) >> 24) & 0xff, ...

The __print_symbolic() helper function's first parameter is the field to
use to determine what symbol to print based on the value of the result.
The parser can handle one operation, but it can not handle multiple
operations ('>>' and '&').

Add code to process all operations for the field argument for
__print_symbolic() as well as __print_flags().

Reported-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/20131118142314.27ca334b@gandalf.local.home
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/event-parse.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 0362d575de7d..8a5b65df6efb 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1606,6 +1606,24 @@ process_arg(struct event_format *event, struct print_arg *arg, char **tok)
 static enum event_type
 process_op(struct event_format *event, struct print_arg *arg, char **tok);
 
+/*
+ * For __print_symbolic() and __print_flags, we need to completely
+ * evaluate the first argument, which defines what to print next.
+ */
+static enum event_type
+process_field_arg(struct event_format *event, struct print_arg *arg, char **tok)
+{
+	enum event_type type;
+
+	type = process_arg(event, arg, tok);
+
+	while (type == EVENT_OP) {
+		type = process_op(event, arg, tok);
+	}
+
+	return type;
+}
+
 static enum event_type
 process_cond(struct event_format *event, struct print_arg *top, char **tok)
 {
@@ -2371,7 +2389,7 @@ process_flags(struct event_format *event, struct print_arg *arg, char **tok)
 		goto out_free;
 	}
 
-	type = process_arg(event, field, &token);
+	type = process_field_arg(event, field, &token);
 
 	/* Handle operations in the first argument */
 	while (type == EVENT_OP)
@@ -2424,7 +2442,8 @@ process_symbols(struct event_format *event, struct print_arg *arg, char **tok)
 		goto out_free;
 	}
 
-	type = process_arg(event, field, &token);
+	type = process_field_arg(event, field, &token);
+
 	if (test_type_token(type, token, EVENT_DELIM, ","))
 		goto out_free_field;
 
-- 
1.8.1.4


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

* [PATCH 5/5] tools lib traceevent: Fix conversion of pointer to integer of different size
  2013-11-19 20:41 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2013-11-19 20:41 ` [PATCH 4/5] tools lib traceevent: Fix use of multiple options in processing field Arnaldo Carvalho de Melo
@ 2013-11-19 20:41 ` Arnaldo Carvalho de Melo
  2013-11-20 13:32 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar
  5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-11-19 20:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	David Ahern, Frederic Weisbecker, Jiri Olsa, Mike Galbraith,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

gcc complaint on 32-bit system:

  /home/acme/git/linux/tools/lib/traceevent/event-parse.c: In function ‘eval_num_arg’:
  /home/acme/git/linux/tools/lib/traceevent/event-parse.c:3468:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

This is because the eval_num_arg returns everything as an 'unsigned long long',
so it converts a void pointer to a wider integer, fix it by converting the void
pointer to an integer of the same size, 'unsigned long', before casting it to
'unsigned long long'.

Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-yllx4aqcg06v5n4vjpwiiuld@git.kernel.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 8a5b65df6efb..217c82ee3665 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3465,7 +3465,7 @@ eval_num_arg(void *data, int size, struct event_format *event, struct print_arg
 		 * is in the bottom half of the 32 bit field.
 		 */
 		offset &= 0xffff;
-		val = (unsigned long long)(data + offset);
+		val = (unsigned long long)((unsigned long)data + offset);
 		break;
 	default: /* not sure what to do there */
 		return 0;
-- 
1.8.1.4


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

* Re: [GIT PULL 0/5] perf/urgent fixes
  2013-11-19 20:41 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2013-11-19 20:41 ` [PATCH 5/5] tools lib traceevent: Fix conversion of pointer to integer of different size Arnaldo Carvalho de Melo
@ 2013-11-20 13:32 ` Ingo Molnar
  5 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2013-11-20 13:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	David Ahern, Frederic Weisbecker, Jiri Olsa, Mike Galbraith,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra, Stephane Eranian,
	Steven Rostedt, Arnaldo Carvalho de Melo


* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> 
> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit 801a76050bcf8d4e500eb8d048ff6265f37a61c8:
> 
>   seq_file: always clear m->count when we free m->buf (2013-11-18 19:07:53 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo
> 
> for you to fetch changes up to 6b5fa0ba4f85a8499287aefaf3f1375450c40c6d:
> 
>   tools lib traceevent: Fix conversion of pointer to integer of different size (2013-11-19 16:37:59 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes:
> 
> . Tag thread comm as overriden, showing the right comm for threads after forks,
>   fix from Frederic Weisbecker.
> 
> . Fix memory leak when processing perf.data file header, from Namhyung Kim.
> 
> . Don't try to free string constant used for anonymous event groups, fix from Namhyung Kim.
> 
> . Fix use of multiple options in processing field in libtraceevent, fix from Steven Rostedt.
> 
> . Fix conversion of pointer to integer of different size in libtraceevent.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (1):
>       tools lib traceevent: Fix conversion of pointer to integer of different size
> 
> Frederic Weisbecker (1):
>       perf tools: Tag thread comm as overriden
> 
> Namhyung Kim (2):
>       perf header: Fix bogus group name
>       perf header: Fix possible memory leaks in process_group_desc()
> 
> Steven Rostedt (1):
>       tools lib traceevent: Fix use of multiple options in processing field
> 
>  tools/lib/traceevent/event-parse.c | 25 ++++++++++++++++++++++---
>  tools/perf/util/header.c           |  6 ++++--
>  tools/perf/util/thread.c           | 11 +++++------
>  3 files changed, 31 insertions(+), 11 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

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

end of thread, other threads:[~2013-11-20 13:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-19 20:41 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
2013-11-19 20:41 ` [PATCH 1/5] perf tools: Tag thread comm as overriden Arnaldo Carvalho de Melo
2013-11-19 20:41 ` [PATCH 2/5] perf header: Fix bogus group name Arnaldo Carvalho de Melo
2013-11-19 20:41 ` [PATCH 3/5] perf header: Fix possible memory leaks in process_group_desc() Arnaldo Carvalho de Melo
2013-11-19 20:41 ` [PATCH 4/5] tools lib traceevent: Fix use of multiple options in processing field Arnaldo Carvalho de Melo
2013-11-19 20:41 ` [PATCH 5/5] tools lib traceevent: Fix conversion of pointer to integer of different size Arnaldo Carvalho de Melo
2013-11-20 13:32 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).