[PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel

linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel
@ 2012-01-29  8:55 Namhyung Kim
  2012-01-29  8:55 ` [PATCH 2/5] perf record: Add error message for EMFILE Namhyung Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-01-29  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

If multiple events are specified on old kernel,
perf_evlist__id2evsel() returns NULL for non-sampling events
since the sample.id doesn't contain valid value, and it triggers
assert below. If only one event is given, the function returns
the evsel regardless of sample.id, this is why most case cause
no problem on old kernel.

Fix it by using active evsel.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/builtin-top.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index e8b033c074f9..f68fba52c8d8 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -771,6 +771,14 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
 		}
 
 		evsel = perf_evlist__id2evsel(session->evlist, sample.id);
+		if (evsel == NULL && !session->sample_id_all &&
+		    event->header.type != PERF_RECORD_SAMPLE) {
+			/*
+			 * Old kernel, no sample_id_all field.
+			 * Just use active evsel.
+			 */
+			evsel = top->sym_evsel;
+		}
 		assert(evsel != NULL);
 
 		origin = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
-- 
1.7.8.2


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

* [PATCH 2/5] perf record: Add error message for EMFILE
  2012-01-29  8:55 [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Namhyung Kim
@ 2012-01-29  8:55 ` Namhyung Kim
  2012-01-29 15:28   ` David Ahern
  2012-01-29  8:55 ` [PATCH 3/5] perf stat: " Namhyung Kim
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2012-01-29  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

When a user tries to open so many events, perf_event_oen syscall
may fail with EMFILE. Provide an advice for that case.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/builtin-record.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 32870eef952f..5d3b6794d93b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -249,6 +249,10 @@ try_again:
 				ui__warning("The %s event is not supported.\n",
 					    event_name(pos));
 				exit(EXIT_FAILURE);
+			} else if (err == EMFILE) {
+				ui__warning("Too many events are opened.\n"
+					    "Try again after reducing the number of events.\n");
+				exit(EXIT_FAILURE);
 			}
 
 			printf("\n");
-- 
1.7.8.2


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

* Re: [PATCH 2/5] perf record: Add error message for EMFILE
  2012-01-29  8:55 ` [PATCH 2/5] perf record: Add error message for EMFILE Namhyung Kim
@ 2012-01-29 15:28   ` David Ahern
  2012-01-30  1:09     ` Namhyung Kim
  0 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2012-01-29 15:28 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Ingo Molnar, linux-kernel



On 01/29/2012 01:55 AM, Namhyung Kim wrote:
> When a user tries to open so many events, perf_event_oen syscall
> may fail with EMFILE. Provide an advice for that case.
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  tools/perf/builtin-record.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 32870eef952f..5d3b6794d93b 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -249,6 +249,10 @@ try_again:
>  				ui__warning("The %s event is not supported.\n",
>  					    event_name(pos));
>  				exit(EXIT_FAILURE);
> +			} else if (err == EMFILE) {
> +				ui__warning("Too many events are opened.\n"
> +					    "Try again after reducing the number of events.\n");
> +				exit(EXIT_FAILURE);
>  			}
>  
>  			printf("\n");

It's not just the number of events: an fd is opened for each specified
event on each specified cpu and for each specified task. See
__perf_evsel__open(). e.g., the new --uid option on a 16 cpu server can
hit the limit pretty fast.

David

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

* Re: [PATCH 2/5] perf record: Add error message for EMFILE
  2012-01-29 15:28   ` David Ahern
@ 2012-01-30  1:09     ` Namhyung Kim
       [not found]       ` <CAKYOsXxxmG8xQfPcn9xW+Dzd_-Qo8jUv8071UHdC18RBatq=uw@mail.gmail.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2012-01-30  1:09 UTC (permalink / raw)
  To: David Ahern
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Ingo Molnar, linux-kernel

Hello,

2012-01-30 12:28 AM, David Ahern wrote:
> On 01/29/2012 01:55 AM, Namhyung Kim wrote:
>> When a user tries to open so many events, perf_event_oen syscall
>> may fail with EMFILE. Provide an advice for that case.
>>
>> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
>> ---
>>   tools/perf/builtin-record.c |    4 ++++
>>   1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 32870eef952f..5d3b6794d93b 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -249,6 +249,10 @@ try_again:
>>   				ui__warning("The %s event is not supported.\n",
>>   					    event_name(pos));
>>   				exit(EXIT_FAILURE);
>> +			} else if (err == EMFILE) {
>> +				ui__warning("Too many events are opened.\n"
>> +					    "Try again after reducing the number of events.\n");
>> +				exit(EXIT_FAILURE);
>>   			}
>>
>>   			printf("\n");
>
> It's not just the number of events: an fd is opened for each specified
> event on each specified cpu and for each specified task. See
> __perf_evsel__open(). e.g., the new --uid option on a 16 cpu server can
> hit the limit pretty fast.
>
> David

Right. How about this then? If it looks OK to you, I'll resend the patch 
for record, top, stat:

	Too many events are opened.
	Try again after reducing the number of events,
	tasks and/or cpus.


Thanks,
Namhyung

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

[parent not found: <CAKYOsXxxmG8xQfPcn9xW+Dzd_-Qo8jUv8071UHdC18RBatq=uw@mail.gmail.com>]

* Re: [PATCH 2/5] perf record: Add error message for EMFILE
       [not found]       ` <CAKYOsXxxmG8xQfPcn9xW+Dzd_-Qo8jUv8071UHdC18RBatq=uw@mail.gmail.com>
@ 2012-01-30 18:56         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-01-30 18:56 UTC (permalink / raw)
  To: David Ahern
  Cc: Namhyung Kim, Ingo Molnar, linux-kernel, Paul Mackerras, Peter Zijlstra

Em Sun, Jan 29, 2012 at 07:10:08PM -0700, David Ahern escreveu:
>    On Jan 29, 2012 6:09 PM, "Namhyung Kim" <namhyung@gmail.com> wrote:
>    > 2012-01-30 12:28 AM, David Ahern wrote:
>    > Right. How about this then? If it looks OK to you, I'll resend the patch
>    for record, top, stat:
>    >
>    >
>    >        Too many events are opened.
>    >        Try again after reducing the number of events,
>    >        tasks and/or cpus.
> 
>    Looks ok to me.

Ok, waiting for the resend,

- Arnaldo

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

* [PATCH 3/5] perf stat: Add error message for EMFILE
  2012-01-29  8:55 [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Namhyung Kim
  2012-01-29  8:55 ` [PATCH 2/5] perf record: Add error message for EMFILE Namhyung Kim
@ 2012-01-29  8:55 ` Namhyung Kim
  2012-01-29  8:55 ` [PATCH 4/5] perf tools: Remove unnecessary ctype.h inclusion Namhyung Kim
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-01-29  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

When a user tries to open so many events, perf_event_oen syscall
may fail with EMFILE. Provide an advice for that case.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/builtin-stat.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 459b8620a5d9..f74f9a557c9a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -477,6 +477,9 @@ static int run_perf_stat(int argc __used, const char **argv)
 				      "\t Consider tweaking"
 				      " /proc/sys/kernel/perf_event_paranoid or running as root.",
 				      system_wide ? "system-wide " : "");
+			} else if (errno == EMFILE) {
+				error("Too many events are opened.\n"
+				      "\t Try again after reducing the number of events.\n");
 			} else {
 				error("open_counter returned with %d (%s). "
 				      "/bin/dmesg may provide additional information.\n",
-- 
1.7.8.2


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

* [PATCH 4/5] perf tools: Remove unnecessary ctype.h inclusion
  2012-01-29  8:55 [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Namhyung Kim
  2012-01-29  8:55 ` [PATCH 2/5] perf record: Add error message for EMFILE Namhyung Kim
  2012-01-29  8:55 ` [PATCH 3/5] perf stat: " Namhyung Kim
@ 2012-01-29  8:55 ` Namhyung Kim
  2012-01-31 13:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-01-29  8:55 ` [PATCH 5/5] perf lock: Document lock info subcommand Namhyung Kim
  2012-01-30 18:55 ` [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Arnaldo Carvalho de Melo
  4 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2012-01-29  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

There are unnecessary #include <ctype.h> out there, and they might
cause a nasty build failure in some environment. As we already have
most of ctype macros in util.h, just get rid of them.

A few of exceptions are util/symbol.c which needs isupper() macro
util.h doesn't provide and perl scripting support code which includes
ctype.h internally.

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/bench/mem-memcpy.c                      |    1 -
 tools/perf/bench/mem-memset.c                      |    1 -
 tools/perf/util/probe-finder.c                     |    1 -
 .../util/scripting-engines/trace-event-python.c    |    1 -
 tools/perf/util/trace-event-parse.c                |    1 -
 tools/perf/util/trace-event-read.c                 |    1 -
 tools/perf/util/trace-event-scripting.c            |    1 -
 tools/perf/util/ui/browsers/map.c                  |    2 +-
 8 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/tools/perf/bench/mem-memcpy.c b/tools/perf/bench/mem-memcpy.c
index 6ad2b1c6b27b..71557225bf92 100644
--- a/tools/perf/bench/mem-memcpy.c
+++ b/tools/perf/bench/mem-memcpy.c
@@ -5,7 +5,6 @@
  *
  * Written by Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
  */
-#include <ctype.h>
 
 #include "../perf.h"
 #include "../util/util.h"
diff --git a/tools/perf/bench/mem-memset.c b/tools/perf/bench/mem-memset.c
index 59d4933eff44..e9079185bd72 100644
--- a/tools/perf/bench/mem-memset.c
+++ b/tools/perf/bench/mem-memset.c
@@ -5,7 +5,6 @@
  *
  * Trivial clone of mem-memcpy.c.
  */
-#include <ctype.h>
 
 #include "../perf.h"
 #include "../util/util.h"
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 5d732621a462..67dc4aed721c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -30,7 +30,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdarg.h>
-#include <ctype.h>
 #include <dwarf-regs.h>
 
 #include <linux/bitops.h>
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 0b2a48783172..c2623c6f9b51 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -24,7 +24,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../../perf.h"
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 1a8d4dc4f386..e0a4f652f289 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -25,7 +25,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../perf.h"
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index f55cc3a765a1..b9592e0de8d7 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -33,7 +33,6 @@
 #include <pthread.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../perf.h"
diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
index a3fdf55f317b..18ae6c1831d3 100644
--- a/tools/perf/util/trace-event-scripting.c
+++ b/tools/perf/util/trace-event-scripting.c
@@ -22,7 +22,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../perf.h"
diff --git a/tools/perf/util/ui/browsers/map.c b/tools/perf/util/ui/browsers/map.c
index 6905bcc8be2d..eca6575abfd0 100644
--- a/tools/perf/util/ui/browsers/map.c
+++ b/tools/perf/util/ui/browsers/map.c
@@ -3,9 +3,9 @@
 #include <newt.h>
 #include <inttypes.h>
 #include <sys/ttydefaults.h>
-#include <ctype.h>
 #include <string.h>
 #include <linux/bitops.h>
+#include "../../util.h"
 #include "../../debug.h"
 #include "../../symbol.h"
 #include "../browser.h"
-- 
1.7.8.2


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

* [tip:perf/core] perf tools: Remove unnecessary ctype.h inclusion
  2012-01-29  8:55 ` [PATCH 4/5] perf tools: Remove unnecessary ctype.h inclusion Namhyung Kim
@ 2012-01-31 13:22   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-01-31 13:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
	tglx, mingo

Commit-ID:  d30d4a080d195892091ad7d014fc9293cc08ea0f
Gitweb:     http://git.kernel.org/tip/d30d4a080d195892091ad7d014fc9293cc08ea0f
Author:     Namhyung Kim <namhyung@gmail.com>
AuthorDate: Sun, 29 Jan 2012 17:55:55 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 30 Jan 2012 18:37:35 -0200

perf tools: Remove unnecessary ctype.h inclusion

There are unnecessary #include <ctype.h> out there, and they might cause
a nasty build failure in some environment. As we already have most of
ctype macros in util.h, just get rid of them.

A few of exceptions are util/symbol.c which needs isupper() macro util.h
doesn't provide and perl scripting support code which includes ctype.h
internally.

Suggested-by: Ingo Molnar <mingo@elte.hu>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1327827356-8786-4-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/bench/mem-memcpy.c                      |    1 -
 tools/perf/bench/mem-memset.c                      |    1 -
 tools/perf/util/probe-finder.c                     |    1 -
 .../util/scripting-engines/trace-event-python.c    |    1 -
 tools/perf/util/trace-event-parse.c                |    1 -
 tools/perf/util/trace-event-read.c                 |    1 -
 tools/perf/util/trace-event-scripting.c            |    1 -
 tools/perf/util/ui/browsers/map.c                  |    2 +-
 8 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/tools/perf/bench/mem-memcpy.c b/tools/perf/bench/mem-memcpy.c
index 6ad2b1c..7155722 100644
--- a/tools/perf/bench/mem-memcpy.c
+++ b/tools/perf/bench/mem-memcpy.c
@@ -5,7 +5,6 @@
  *
  * Written by Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
  */
-#include <ctype.h>
 
 #include "../perf.h"
 #include "../util/util.h"
diff --git a/tools/perf/bench/mem-memset.c b/tools/perf/bench/mem-memset.c
index 59d4933..e907918 100644
--- a/tools/perf/bench/mem-memset.c
+++ b/tools/perf/bench/mem-memset.c
@@ -5,7 +5,6 @@
  *
  * Trivial clone of mem-memcpy.c.
  */
-#include <ctype.h>
 
 #include "../perf.h"
 #include "../util/util.h"
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 5d73262..67dc4ae 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -30,7 +30,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdarg.h>
-#include <ctype.h>
 #include <dwarf-regs.h>
 
 #include <linux/bitops.h>
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 0b2a487..c2623c6 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -24,7 +24,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../../perf.h"
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 1a8d4dc..e0a4f65 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -25,7 +25,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../perf.h"
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index f55cc3a..b9592e0 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -33,7 +33,6 @@
 #include <pthread.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../perf.h"
diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
index a3fdf55..18ae6c1 100644
--- a/tools/perf/util/trace-event-scripting.c
+++ b/tools/perf/util/trace-event-scripting.c
@@ -22,7 +22,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "../perf.h"
diff --git a/tools/perf/util/ui/browsers/map.c b/tools/perf/util/ui/browsers/map.c
index 6905bcc..eca6575 100644
--- a/tools/perf/util/ui/browsers/map.c
+++ b/tools/perf/util/ui/browsers/map.c
@@ -3,9 +3,9 @@
 #include <newt.h>
 #include <inttypes.h>
 #include <sys/ttydefaults.h>
-#include <ctype.h>
 #include <string.h>
 #include <linux/bitops.h>
+#include "../../util.h"
 #include "../../debug.h"
 #include "../../symbol.h"
 #include "../browser.h"

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

* [PATCH 5/5] perf lock: Document lock info subcommand
  2012-01-29  8:55 [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Namhyung Kim
                   ` (2 preceding siblings ...)
  2012-01-29  8:55 ` [PATCH 4/5] perf tools: Remove unnecessary ctype.h inclusion Namhyung Kim
@ 2012-01-29  8:55 ` Namhyung Kim
  2012-01-31 13:21   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-01-30 18:55 ` [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Arnaldo Carvalho de Melo
  4 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2012-01-29  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel,
	Hitoshi Mitake

The commit 26242d859c9be ("perf lock: Add "info" subcommand for dumping
misc information") added the subcommand but missed documentation. Add
it. Also update stale 'trace' subcommand to 'script'.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
---
 tools/perf/Documentation/perf-lock.txt |   20 +++++++++++++++++---
 tools/perf/builtin-lock.c              |    4 ++--
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
index d6b2a4f2108b..c7f5f55634ac 100644
--- a/tools/perf/Documentation/perf-lock.txt
+++ b/tools/perf/Documentation/perf-lock.txt
@@ -8,7 +8,7 @@ perf-lock - Analyze lock events
 SYNOPSIS
 --------
 [verse]
-'perf lock' {record|report|trace}
+'perf lock' {record|report|script|info}
 
 DESCRIPTION
 -----------
@@ -20,10 +20,13 @@ and statistics with this 'perf lock' command.
   produces the file "perf.data" which contains tracing
   results of lock events.
 
-  'perf lock trace' shows raw lock events.
-
   'perf lock report' reports statistical data.
 
+  'perf lock script' shows raw lock events.
+
+  'perf lock info' shows metadata like threads or addresses
+  of lock instances.
+
 COMMON OPTIONS
 --------------
 
@@ -47,6 +50,17 @@ REPORT OPTIONS
         Sorting key. Possible values: acquired (default), contended,
         wait_total, wait_max, wait_min.
 
+INFO OPTIONS
+------------
+
+-t::
+--threads::
+	dump thread list in perf.data
+
+-m::
+--map::
+	dump map of lock instances (address:name table)
+
 SEE ALSO
 --------
 linkperf:perf[1]
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 2296c391d0f5..12c814838993 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -922,12 +922,12 @@ static const struct option info_options[] = {
 	OPT_BOOLEAN('t', "threads", &info_threads,
 		    "dump thread list in perf.data"),
 	OPT_BOOLEAN('m', "map", &info_map,
-		    "map of lock instances (name:address table)"),
+		    "map of lock instances (address:name table)"),
 	OPT_END()
 };
 
 static const char * const lock_usage[] = {
-	"perf lock [<options>] {record|trace|report}",
+	"perf lock [<options>] {record|report|script|info}",
 	NULL
 };
 
-- 
1.7.8.2


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

* [tip:perf/core] perf lock: Document lock info subcommand
  2012-01-29  8:55 ` [PATCH 5/5] perf lock: Document lock info subcommand Namhyung Kim
@ 2012-01-31 13:21   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-01-31 13:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, mitake,
	namhyung, tglx, mingo

Commit-ID:  d1eec3ecaef083affaf3210246b01b6e80d3a44e
Gitweb:     http://git.kernel.org/tip/d1eec3ecaef083affaf3210246b01b6e80d3a44e
Author:     Namhyung Kim <namhyung@gmail.com>
AuthorDate: Sun, 29 Jan 2012 17:55:56 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 30 Jan 2012 18:30:48 -0200

perf lock: Document lock info subcommand

The commit 26242d859c9be ("perf lock: Add "info" subcommand for dumping
misc information") added the subcommand but missed documentation. Add
it. Also update stale 'trace' subcommand to 'script'.

Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1327827356-8786-5-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-lock.txt |   20 +++++++++++++++++---
 tools/perf/builtin-lock.c              |    4 ++--
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt
index d6b2a4f..c7f5f55 100644
--- a/tools/perf/Documentation/perf-lock.txt
+++ b/tools/perf/Documentation/perf-lock.txt
@@ -8,7 +8,7 @@ perf-lock - Analyze lock events
 SYNOPSIS
 --------
 [verse]
-'perf lock' {record|report|trace}
+'perf lock' {record|report|script|info}
 
 DESCRIPTION
 -----------
@@ -20,10 +20,13 @@ and statistics with this 'perf lock' command.
   produces the file "perf.data" which contains tracing
   results of lock events.
 
-  'perf lock trace' shows raw lock events.
-
   'perf lock report' reports statistical data.
 
+  'perf lock script' shows raw lock events.
+
+  'perf lock info' shows metadata like threads or addresses
+  of lock instances.
+
 COMMON OPTIONS
 --------------
 
@@ -47,6 +50,17 @@ REPORT OPTIONS
         Sorting key. Possible values: acquired (default), contended,
         wait_total, wait_max, wait_min.
 
+INFO OPTIONS
+------------
+
+-t::
+--threads::
+	dump thread list in perf.data
+
+-m::
+--map::
+	dump map of lock instances (address:name table)
+
 SEE ALSO
 --------
 linkperf:perf[1]
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 2296c39..12c8148 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -922,12 +922,12 @@ static const struct option info_options[] = {
 	OPT_BOOLEAN('t', "threads", &info_threads,
 		    "dump thread list in perf.data"),
 	OPT_BOOLEAN('m', "map", &info_map,
-		    "map of lock instances (name:address table)"),
+		    "map of lock instances (address:name table)"),
 	OPT_END()
 };
 
 static const char * const lock_usage[] = {
-	"perf lock [<options>] {record|trace|report}",
+	"perf lock [<options>] {record|report|script|info}",
 	NULL
 };
 

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

* Re: [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel
  2012-01-29  8:55 [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Namhyung Kim
                   ` (3 preceding siblings ...)
  2012-01-29  8:55 ` [PATCH 5/5] perf lock: Document lock info subcommand Namhyung Kim
@ 2012-01-30 18:55 ` Arnaldo Carvalho de Melo
  2012-01-31  0:31   ` Namhyung Kim
  4 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-01-30 18:55 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

Em Sun, Jan 29, 2012 at 05:55:52PM +0900, Namhyung Kim escreveu:
> If multiple events are specified on old kernel,
> perf_evlist__id2evsel() returns NULL for non-sampling events
> since the sample.id doesn't contain valid value, and it triggers
> assert below. If only one event is given, the function returns
> the evsel regardless of sample.id, this is why most case cause
> no problem on old kernel.
> 
> Fix it by using active evsel.

How about this one instead:

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a6d50e3..3ffb320 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -349,6 +349,10 @@ struct perf_evsel *perf_evlist__id2evsel(struct perf_evlist *evlist, u64 id)
 	hlist_for_each_entry(sid, pos, head, node)
 		if (sid->id == id)
 			return sid->evsel;
+
+	if (!perf_evlist__sample_id_all(evlist))
+		return list_entry(evlist->entries.next, struct perf_evsel, node);
+
 	return NULL;
 }

That way we won't have to patch other tools that would get the same
problem in such new tool/old kernel combos, right?

Do you see any problem with not checking the header type?

Also please try to avoid using perf_session fields, prefer to use
perf_{evlist,evsel} when possible, in this case you could use
perf_evlist__sample_id_all(evlist), for instance.

- Arnaldo

> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  tools/perf/builtin-top.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index e8b033c074f9..f68fba52c8d8 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -771,6 +771,14 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
>  		}
>  
>  		evsel = perf_evlist__id2evsel(session->evlist, sample.id);
> +		if (evsel == NULL && !session->sample_id_all &&
> +		    event->header.type != PERF_RECORD_SAMPLE) {
> +			/*
> +			 * Old kernel, no sample_id_all field.
> +			 * Just use active evsel.
> +			 */
> +			evsel = top->sym_evsel;
> +		}
>  		assert(evsel != NULL);
>  
>  		origin = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
> -- 
> 1.7.8.2

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

* Re: [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel
  2012-01-30 18:55 ` [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Arnaldo Carvalho de Melo
@ 2012-01-31  0:31   ` Namhyung Kim
  2012-01-31  0:34     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2012-01-31  0:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

2012-01-31 3:55 AM, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jan 29, 2012 at 05:55:52PM +0900, Namhyung Kim escreveu:
>> If multiple events are specified on old kernel,
>> perf_evlist__id2evsel() returns NULL for non-sampling events
>> since the sample.id doesn't contain valid value, and it triggers
>> assert below. If only one event is given, the function returns
>> the evsel regardless of sample.id, this is why most case cause
>> no problem on old kernel.
>>
>> Fix it by using active evsel.
>
> How about this one instead:
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index a6d50e3..3ffb320 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -349,6 +349,10 @@ struct perf_evsel *perf_evlist__id2evsel(struct perf_evlist *evlist, u64 id)
>   	hlist_for_each_entry(sid, pos, head, node)
>   		if (sid->id == id)
>   			return sid->evsel;
> +
> +	if (!perf_evlist__sample_id_all(evlist))
> +		return list_entry(evlist->entries.next, struct perf_evsel, node);
> +
>   	return NULL;
>   }
>
> That way we won't have to patch other tools that would get the same
> problem in such new tool/old kernel combos, right?
>

Right, looks good to me.


> Do you see any problem with not checking the header type?
>

No, I don't, at least for now :)


> Also please try to avoid using perf_session fields, prefer to use
> perf_{evlist,evsel} when possible, in this case you could use
> perf_evlist__sample_id_all(evlist), for instance.
>
> - Arnaldo
>

OK, will do that later.

Thanks,
Namhyung

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

* Re: [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel
  2012-01-31  0:31   ` Namhyung Kim
@ 2012-01-31  0:34     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-01-31  0:34 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel

Em Tue, Jan 31, 2012 at 09:31:13AM +0900, Namhyung Kim escreveu:
> 2012-01-31 3:55 AM, Arnaldo Carvalho de Melo wrote:
> >Em Sun, Jan 29, 2012 at 05:55:52PM +0900, Namhyung Kim escreveu:
> >>If multiple events are specified on old kernel,
> >>perf_evlist__id2evsel() returns NULL for non-sampling events
> >>since the sample.id doesn't contain valid value, and it triggers
> >>assert below. If only one event is given, the function returns
> >>the evsel regardless of sample.id, this is why most case cause
> >>no problem on old kernel.
> >>
> >>Fix it by using active evsel.
> >
> >How about this one instead:
> >
> >diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> >index a6d50e3..3ffb320 100644
> >--- a/tools/perf/util/evlist.c
> >+++ b/tools/perf/util/evlist.c
> >@@ -349,6 +349,10 @@ struct perf_evsel *perf_evlist__id2evsel(struct perf_evlist *evlist, u64 id)
> >  	hlist_for_each_entry(sid, pos, head, node)
> >  		if (sid->id == id)
> >  			return sid->evsel;
> >+
> >+	if (!perf_evlist__sample_id_all(evlist))
> >+		return list_entry(evlist->entries.next, struct perf_evsel, node);
> >+
> >  	return NULL;
> >  }
> >
> >That way we won't have to patch other tools that would get the same
> >problem in such new tool/old kernel combos, right?
> >
> 
> Right, looks good to me.

Please resend that way and I'll apply, or lemme know if you prefer for
me to do it and just stick your 'Acked-by: ' or 'Tested-by:' on it.
 
> 
> >Do you see any problem with not checking the header type?
> >
> 
> No, I don't, at least for now :)
> 
> 
> >Also please try to avoid using perf_session fields, prefer to use
> >perf_{evlist,evsel} when possible, in this case you could use
> >perf_evlist__sample_id_all(evlist), for instance.
> >
> >- Arnaldo
> >
> 
> OK, will do that later.
> 
> Thanks,
> Namhyung

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

end of thread, other threads:[~2012-01-31 13:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-29  8:55 [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Namhyung Kim
2012-01-29  8:55 ` [PATCH 2/5] perf record: Add error message for EMFILE Namhyung Kim
2012-01-29 15:28   ` David Ahern
2012-01-30  1:09     ` Namhyung Kim
     [not found]       ` <CAKYOsXxxmG8xQfPcn9xW+Dzd_-Qo8jUv8071UHdC18RBatq=uw@mail.gmail.com>
2012-01-30 18:56         ` Arnaldo Carvalho de Melo
2012-01-29  8:55 ` [PATCH 3/5] perf stat: " Namhyung Kim
2012-01-29  8:55 ` [PATCH 4/5] perf tools: Remove unnecessary ctype.h inclusion Namhyung Kim
2012-01-31 13:22   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-01-29  8:55 ` [PATCH 5/5] perf lock: Document lock info subcommand Namhyung Kim
2012-01-31 13:21   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-01-30 18:55 ` [PATCH 1/5] perf top: Use active evsel for non-sample events on old kernel Arnaldo Carvalho de Melo
2012-01-31  0:31   ` Namhyung Kim
2012-01-31  0:34     ` Arnaldo Carvalho de Melo

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