All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf, tools: Add PERF_PID
@ 2014-09-10  0:03 Andi Kleen
  2014-09-10  1:08 ` Brendan Gregg
  2014-09-15 19:22 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 9+ messages in thread
From: Andi Kleen @ 2014-09-10  0:03 UTC (permalink / raw)
  To: jolsa; +Cc: linux-kernel, namhyung, acme, mingo, peterz, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

It's currently difficult to filter out perf itself using a filter.
This can give cascading effects during IO tracing when the IO
perf does itself causes more trace output.

The best way to filter is to use the pid. But it's difficult to get the pid
of perf without using hacks.

Add a PERF_PID meta variable to the perf filter that contains the current pid.

With this patch the following works

% perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...

This won't work for more complex perf pipe lines with multiple processes,
but at least solves the problem nicely for a single perf.

v2: Remove debug code. Don't use zero padding (Namhyung)
v3: Correct patch.
Acked-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-record.txt | 2 +-
 tools/perf/util/parse-events.c           | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index d460049..b6c5e51 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -41,7 +41,7 @@ OPTIONS
           'mem:0x1000:rw'.
 
 --filter=<filter>::
-        Event filter.
+        Event filter. PERF_PID represents the perf pid.
 
 -a::
 --all-cpus::
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e15df1..def8651 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -967,6 +967,7 @@ int parse_filter(const struct option *opt, const char *str,
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
 	struct perf_evsel *last = NULL;
+	char *pid;
 
 	if (evlist->nr_entries > 0)
 		last = perf_evlist__last(evlist);
@@ -983,6 +984,13 @@ int parse_filter(const struct option *opt, const char *str,
 		return -1;
 	}
 
+	/* Assume a pid has not more than 8 characters */
+	pid = strstr(last->filter, "PERF_PID");
+	if (pid) {
+		char buf[9];
+		snprintf(buf, 9, "%8d", getpid());
+		memcpy(pid, buf, 8);
+	}
 	return 0;
 }
 
-- 
1.9.3


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

* Re: [PATCH] perf, tools: Add PERF_PID
  2014-09-10  0:03 [PATCH] perf, tools: Add PERF_PID Andi Kleen
@ 2014-09-10  1:08 ` Brendan Gregg
  2014-09-15 19:22 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 9+ messages in thread
From: Brendan Gregg @ 2014-09-10  1:08 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Jiri Olsa, LKML, Namhyung Kim, Arnaldo Carvalho de Melo,
	Ingo Molnar, peterz, Andi Kleen

On Tue, Sep 9, 2014 at 5:03 PM, Andi Kleen <andi@firstfloor.org> wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> It's currently difficult to filter out perf itself using a filter.
> This can give cascading effects during IO tracing when the IO
> perf does itself causes more trace output.
>
> The best way to filter is to use the pid. But it's difficult to get the pid
> of perf without using hacks.
>
> Add a PERF_PID meta variable to the perf filter that contains the current pid.
>
> With this patch the following works
>
> % perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...
>
> This won't work for more complex perf pipe lines with multiple processes,
> but at least solves the problem nicely for a single perf.

Thank you, this will be handy. I tested it and it works.

It will mean I can avoid hacks like this:
https://github.com/brendangregg/perf-tools/blob/d30e8a1a9f136d532ba78c6d48aedb99dc316be9/syscount#L144

I think it's unrelated to this patch, but the very next test I tried
didn't work as expected:

# perf record -e 'syscalls:sys_enter_*' --filter 'common_pid !=
PERF_PID' -a sleep 5

It only filters the last event from the wildcard expansion, leaving
write() unfiltered. Looks like it's because parse_filter() is only
setting the filter for perf_evlist__last(), instead of doing an
evlist__for_each(). How best to deal with this can be addressed in a
separate patch later.

Brendan

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

* Re: [PATCH] perf, tools: Add PERF_PID
  2014-09-10  0:03 [PATCH] perf, tools: Add PERF_PID Andi Kleen
  2014-09-10  1:08 ` Brendan Gregg
@ 2014-09-15 19:22 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-09-15 19:22 UTC (permalink / raw)
  To: Andi Kleen; +Cc: jolsa, linux-kernel, namhyung, mingo, peterz, Andi Kleen

Em Tue, Sep 09, 2014 at 05:03:53PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> It's currently difficult to filter out perf itself using a filter.
> This can give cascading effects during IO tracing when the IO
> perf does itself causes more trace output.
> 
> The best way to filter is to use the pid. But it's difficult to get the pid
> of perf without using hacks.
> 
> Add a PERF_PID meta variable to the perf filter that contains the current pid.
> 
> With this patch the following works
> 
> % perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...
> 
> This won't work for more complex perf pipe lines with multiple processes,
> but at least solves the problem nicely for a single perf.
 
> @@ -983,6 +984,13 @@ int parse_filter(const struct option *opt, const char *str,
>  		return -1;
>  	}
>  
> +	/* Assume a pid has not more than 8 characters */

> +	pid = strstr(last->filter, "PERF_PID");
> +	if (pid) {
> +		char buf[9];
> +		snprintf(buf, 9, "%8d", getpid());
> +		memcpy(pid, buf, 8);

Can we please remove this assumption?

If the get to be more than 8 chars we'll be just truncating stuff and
possibly filtering some other, unrelated process, no?

       pid_t getpid(void);

typedef __pid_t pid_t;
__STD_TYPE __PID_T_TYPE __pid_t;	/* Type of process identifications.  */
/usr/include/bits/typesizes.h:#define __PID_T_TYPE __S32_TYPE
/usr/include/bits/types.h:#define	__S32_TYPE		int
[acme@zoo linux]$ uname -a
Linux zoo.ghostprotocols.net 3.17.0-rc1+ #2 SMP Thu Aug 21 12:25:42 BRT 2014 x86_64 x86_64 x86_64 GNU/Linux

[acme@zoo ~]$ cat a.c
#include <limits.h>
#include <stdio.h>

int main(int argc, char *argv[])
{
	printf("%8d", INT_MAX);
}
[acme@zoo ~]$ make a
cc     a.c   -o a
[acme@zoo ~]$ ./a | wc -c
10
[acme@zoo ~]$

The feature is needed, no doubt about it, thanks for working on it.

I guess we need the equivalent of (python regexp) s/PERF_PID/("%d" % getpid())/g
to properly implement this.

- Arnaldo

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

* [PATCH] perf, tools: Add PERF_PID
@ 2014-09-15 23:17 Andi Kleen
  0 siblings, 0 replies; 9+ messages in thread
From: Andi Kleen @ 2014-09-15 23:17 UTC (permalink / raw)
  To: jolsa; +Cc: linux-kernel, namhyung, acme, mingo, peterz, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

It's currently difficult to filter out perf itself using a filter.
This can give cascading effects during IO tracing when the IO
perf does itself causes more trace output.

The best way to filter is to use the pid. But it's difficult to get the pid
of perf without using hacks.

Add a PERF_PID meta variable to the perf filter that contains the current pid.

With this patch the following works

% perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...

This won't work for more complex perf pipe lines with multiple processes,
but at least solves the problem nicely for a single perf.

v2: Remove debug code. Don't use zero padding (Namhyung)
v3: Correct patch.
v4: Handle arbitary pid length.
Acked-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-record.txt |  2 +-
 tools/perf/util/parse-events.c           | 20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index d460049..b6c5e51 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -41,7 +41,7 @@ OPTIONS
           'mem:0x1000:rw'.
 
 --filter=<filter>::
-        Event filter.
+        Event filter. PERF_PID represents the perf pid.
 
 -a::
 --all-cpus::
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e15df1..f974941 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -967,6 +967,8 @@ int parse_filter(const struct option *opt, const char *str,
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
 	struct perf_evsel *last = NULL;
+	char *pid, buf[30], *p, *o;
+	int len, plen;
 
 	if (evlist->nr_entries > 0)
 		last = perf_evlist__last(evlist);
@@ -977,12 +979,28 @@ int parse_filter(const struct option *opt, const char *str,
 		return -1;
 	}
 
-	last->filter = strdup(str);
+	plen = sprintf(buf, "%d", getpid());
+	len = strlen(str) + 1;
+	for (pid = strstr(str, "PERF_PID"); pid;
+	     pid = strstr(pid + 1, "PERF_PID")) {
+		len += plen - 8;
+	}
+
+	last->filter = malloc(len);
 	if (last->filter == NULL) {
 		fprintf(stderr, "not enough memory to hold filter string\n");
 		return -1;
 	}
 
+	o = last->filter;
+	for (p = str; *p; ) {
+		if (*p == 'P' && !strncmp(p, "PERF_PID", 8)) {
+			strcpy(o, buf);
+			o += plen;
+			p += sizeof("PERF_PID") - 1;
+		} else
+			*o++ = *p++;
+	}
 	return 0;
 }
 
-- 
1.9.3


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

* [PATCH] perf, tools: Add PERF_PID
@ 2014-08-19 15:04 Andi Kleen
  0 siblings, 0 replies; 9+ messages in thread
From: Andi Kleen @ 2014-08-19 15:04 UTC (permalink / raw)
  To: jolsa; +Cc: linux-kernel, namhyung, acme, mingo, peterz, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

It's currently difficult to filter out perf itself using a filter.
This can give cascading effects during IO tracing when the IO
perf does itself causes more trace output.

The best way to filter is to use the pid. But it's difficult to get the pid
of perf without using hacks.

Add a PERF_PID meta variable to the perf filter that contains the current pid.

With this patch the following works

% perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...

This won't work for more complex perf pipe lines with multiple processes,
but at least solves the problem nicely for a single perf.

v2: Remove debug code. Don't use zero padding (Namhyung)
v3: Correct patch.
Acked-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-record.txt | 2 +-
 tools/perf/util/parse-events.c           | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index d460049..b6c5e51 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -41,7 +41,7 @@ OPTIONS
           'mem:0x1000:rw'.
 
 --filter=<filter>::
-        Event filter.
+        Event filter. PERF_PID represents the perf pid.
 
 -a::
 --all-cpus::
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e15df1..def8651 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -967,6 +967,7 @@ int parse_filter(const struct option *opt, const char *str,
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
 	struct perf_evsel *last = NULL;
+	char *pid;
 
 	if (evlist->nr_entries > 0)
 		last = perf_evlist__last(evlist);
@@ -983,6 +984,13 @@ int parse_filter(const struct option *opt, const char *str,
 		return -1;
 	}
 
+	/* Assume a pid has not more than 8 characters */
+	pid = strstr(last->filter, "PERF_PID");
+	if (pid) {
+		char buf[9];
+		snprintf(buf, 9, "%8d", getpid());
+		memcpy(pid, buf, 8);
+	}
 	return 0;
 }
 
-- 
1.9.3


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

* Re: [PATCH] perf, tools: Add PERF_PID
  2014-08-19 12:02 Andi Kleen
@ 2014-08-19 15:02 ` Andi Kleen
  0 siblings, 0 replies; 9+ messages in thread
From: Andi Kleen @ 2014-08-19 15:02 UTC (permalink / raw)
  To: Andi Kleen; +Cc: jolsa, linux-kernel, namhyung, acme, mingo, peterz

On Tue, Aug 19, 2014 at 05:02:22AM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>

Please disregard. I posted the wrong patch.

-Andi

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

* [PATCH] perf, tools: Add PERF_PID
@ 2014-08-19 12:02 Andi Kleen
  2014-08-19 15:02 ` Andi Kleen
  0 siblings, 1 reply; 9+ messages in thread
From: Andi Kleen @ 2014-08-19 12:02 UTC (permalink / raw)
  To: jolsa; +Cc: linux-kernel, namhyung, acme, mingo, peterz, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

It's currently difficult to filter out perf itself using a filter.
This can give cascading effects during IO tracing when the IO
perf does itself causes more trace output.

The best way to filter is to use the pid. But it's difficult to get the pid
of perf without using hacks.

Add a PERF_PID meta variable to the perf filter that contains the current pid.

With this patch the following works

% perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...

This won't work for more complex perf pipe lines with multiple processes,
but at least solves the problem nicely for a single perf.

v2: Remove debug code. Don't use zero padding (Namhyung)
Acked-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-record.txt | 2 +-
 tools/perf/util/parse-events.c           | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index d460049..b6c5e51 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -41,7 +41,7 @@ OPTIONS
           'mem:0x1000:rw'.
 
 --filter=<filter>::
-        Event filter.
+        Event filter. PERF_PID represents the perf pid.
 
 -a::
 --all-cpus::
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e15df1..90ed63f 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -967,6 +967,7 @@ int parse_filter(const struct option *opt, const char *str,
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
 	struct perf_evsel *last = NULL;
+	char *pid;
 
 	if (evlist->nr_entries > 0)
 		last = perf_evlist__last(evlist);
@@ -983,6 +984,14 @@ int parse_filter(const struct option *opt, const char *str,
 		return -1;
 	}
 
+	/* Assume a pid has not more than 8 characters */
+	pid = strstr(last->filter, "PERF_PID");
+	if (pid) {
+		char buf[9];
+		snprintf(buf, 9, "%08d", getpid());
+		memcpy(pid, buf, 8);
+	}
+	fprintf(stderr, "filter |%s|\n", last->filter);
 	return 0;
 }
 
-- 
1.9.3


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

* Re: [PATCH] perf, tools: Add PERF_PID
  2014-08-13 21:12 Andi Kleen
@ 2014-08-19  6:23 ` Namhyung Kim
  0 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2014-08-19  6:23 UTC (permalink / raw)
  To: Andi Kleen; +Cc: jolsa, linux-kernel, acme, mingo, peterz, Andi Kleen

Hi Andi,

On Wed, 13 Aug 2014 14:12:17 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> It's currently difficult to filter out perf itself using a filter.
> This can give cascading effects during IO tracing when the IO
> perf does itself causes more trace output.
>
> The best way to filter is to use the pid. But it's difficult to get the pid
> of perf without using hacks.
>
> Add a PERF_PID meta variable to the perf filter that contains the current pid.
>
> With this patch the following works
>
> % perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...
>
> This won't work for more complex perf pipe lines with multiple processes,
> but at least solves the problem nicely for a single perf.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

I like the idea, so with comments below fixed:

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


[SNIP]
> +	/* Assume a pid has not more than 8 characters */
> +	pid = strstr(last->filter, "PERF_PID");
> +	if (pid) {
> +		char buf[9];
> +		snprintf(buf, 9, "%08d", getpid());

Why do you add zero-paddings?  I guess it'd confuse the parser as if
it's an octal digits.  What about just using "%8d"?  It seems the parser
ignores whitespaces between tokens..


> +		memcpy(pid, buf, 8);
> +	}
> +	fprintf(stderr, "filter |%s|\n", last->filter);

Isn't it a debug message?

Thanks,
Namhyung


>  	return 0;
>  }

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

* [PATCH] perf, tools: Add PERF_PID
@ 2014-08-13 21:12 Andi Kleen
  2014-08-19  6:23 ` Namhyung Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Andi Kleen @ 2014-08-13 21:12 UTC (permalink / raw)
  To: jolsa; +Cc: linux-kernel, namhyung, acme, mingo, peterz, Andi Kleen

From: Andi Kleen <ak@linux.intel.com>

It's currently difficult to filter out perf itself using a filter.
This can give cascading effects during IO tracing when the IO
perf does itself causes more trace output.

The best way to filter is to use the pid. But it's difficult to get the pid
of perf without using hacks.

Add a PERF_PID meta variable to the perf filter that contains the current pid.

With this patch the following works

% perf record -e syscalls:sys_enter_write -a --filter 'common_pid != PERF_PID' ...

This won't work for more complex perf pipe lines with multiple processes,
but at least solves the problem nicely for a single perf.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-record.txt | 2 +-
 tools/perf/util/parse-events.c           | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index d460049..b6c5e51 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -41,7 +41,7 @@ OPTIONS
           'mem:0x1000:rw'.
 
 --filter=<filter>::
-        Event filter.
+        Event filter. PERF_PID represents the perf pid.
 
 -a::
 --all-cpus::
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e15df1..90ed63f 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -967,6 +967,7 @@ int parse_filter(const struct option *opt, const char *str,
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
 	struct perf_evsel *last = NULL;
+	char *pid;
 
 	if (evlist->nr_entries > 0)
 		last = perf_evlist__last(evlist);
@@ -983,6 +984,14 @@ int parse_filter(const struct option *opt, const char *str,
 		return -1;
 	}
 
+	/* Assume a pid has not more than 8 characters */
+	pid = strstr(last->filter, "PERF_PID");
+	if (pid) {
+		char buf[9];
+		snprintf(buf, 9, "%08d", getpid());
+		memcpy(pid, buf, 8);
+	}
+	fprintf(stderr, "filter |%s|\n", last->filter);
 	return 0;
 }
 
-- 
1.9.3


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

end of thread, other threads:[~2014-09-16  0:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-10  0:03 [PATCH] perf, tools: Add PERF_PID Andi Kleen
2014-09-10  1:08 ` Brendan Gregg
2014-09-15 19:22 ` Arnaldo Carvalho de Melo
  -- strict thread matches above, loose matches on Subject: below --
2014-09-15 23:17 Andi Kleen
2014-08-19 15:04 Andi Kleen
2014-08-19 12:02 Andi Kleen
2014-08-19 15:02 ` Andi Kleen
2014-08-13 21:12 Andi Kleen
2014-08-19  6:23 ` Namhyung Kim

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.