2012-05-31 05:54:24

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 1/3] perf: Remove duplicate invocation on perf_event_for_each

The @func callback was invoked twice for group leader
when perf_event_for_each() called. It seems the commit
75f937f24bd9 ("perf_counter: Fix ctx->mutex vs counter
->mutex inversion") made the mistake during the change.

Signed-off-by: Namhyung Kim <[email protected]>
---
kernel/events/core.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5b06cbbf6931..f85c0154b333 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3181,7 +3181,6 @@ static void perf_event_for_each(struct perf_event *event,
event = event->group_leader;

perf_event_for_each_child(event, func);
- func(event);
list_for_each_entry(sibling, &event->sibling_list, group_entry)
perf_event_for_each_child(sibling, func);
mutex_unlock(&ctx->mutex);
--
1.7.10.2


2012-05-31 05:54:26

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 3/3] perf evlist: Pass third argument to ioctl explicitly

The ioctl on perf event fd wants 3 arguments but we only
passed 2. As the only user of the functions is perf record
and it calls them for every events (regardless of group
setting), just pass 0 for now.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/evlist.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 4ac5f5ae4ce9..244211070264 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -263,7 +263,8 @@ void perf_evlist__disable(struct perf_evlist *evlist)
for (cpu = 0; cpu < evlist->cpus->nr; cpu++) {
list_for_each_entry(pos, &evlist->entries, node) {
for (thread = 0; thread < evlist->threads->nr; thread++)
- ioctl(FD(pos, cpu, thread), PERF_EVENT_IOC_DISABLE);
+ ioctl(FD(pos, cpu, thread),
+ PERF_EVENT_IOC_DISABLE, 0);
}
}
}
@@ -276,7 +277,8 @@ void perf_evlist__enable(struct perf_evlist *evlist)
for (cpu = 0; cpu < evlist->cpus->nr; cpu++) {
list_for_each_entry(pos, &evlist->entries, node) {
for (thread = 0; thread < evlist->threads->nr; thread++)
- ioctl(FD(pos, cpu, thread), PERF_EVENT_IOC_ENABLE);
+ ioctl(FD(pos, cpu, thread),
+ PERF_EVENT_IOC_ENABLE, 0);
}
}
}
--
1.7.10.2

2012-05-31 05:54:45

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/3] perf tools: Update ioctl documentation for PERF_IOC_FLAG_GROUP

The ioctl interface of perf event fd receives 3 arguments
to control event group behavior but it lacked documentation.

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/design.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/design.txt b/tools/perf/design.txt
index bd0bb1b1279b..67e5d0cace85 100644
--- a/tools/perf/design.txt
+++ b/tools/perf/design.txt
@@ -409,14 +409,15 @@ Counters can be enabled and disabled in two ways: via ioctl and via
prctl. When a counter is disabled, it doesn't count or generate
events but does continue to exist and maintain its count value.

-An individual counter or counter group can be enabled with
+An individual counter can be enabled with

- ioctl(fd, PERF_EVENT_IOC_ENABLE);
+ ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);

or disabled with

- ioctl(fd, PERF_EVENT_IOC_DISABLE);
+ ioctl(fd, PERF_EVENT_IOC_DISABLE, 0);

+For a counter group, pass PERF_IOC_FLAG_GROUP as the third argument.
Enabling or disabling the leader of a group enables or disables the
whole group; that is, while the group leader is disabled, none of the
counters in the group will count. Enabling or disabling a member of a
--
1.7.10.2

2012-05-31 14:36:04

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 2/3] perf tools: Update ioctl documentation for PERF_IOC_FLAG_GROUP

Em Thu, May 31, 2012 at 02:51:45PM +0900, Namhyung Kim escreveu:
> The ioctl interface of perf event fd receives 3 arguments
> to control event group behavior but it lacked documentation.

But the argument is not used for ENABLE or DISABLE, so why require it?

- Arnaldo

> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/design.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/design.txt b/tools/perf/design.txt
> index bd0bb1b1279b..67e5d0cace85 100644
> --- a/tools/perf/design.txt
> +++ b/tools/perf/design.txt
> @@ -409,14 +409,15 @@ Counters can be enabled and disabled in two ways: via ioctl and via
> prctl. When a counter is disabled, it doesn't count or generate
> events but does continue to exist and maintain its count value.
>
> -An individual counter or counter group can be enabled with
> +An individual counter can be enabled with
>
> - ioctl(fd, PERF_EVENT_IOC_ENABLE);
> + ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
>
> or disabled with
>
> - ioctl(fd, PERF_EVENT_IOC_DISABLE);
> + ioctl(fd, PERF_EVENT_IOC_DISABLE, 0);
>
> +For a counter group, pass PERF_IOC_FLAG_GROUP as the third argument.
> Enabling or disabling the leader of a group enables or disables the
> whole group; that is, while the group leader is disabled, none of the
> counters in the group will count. Enabling or disabling a member of a
> --
> 1.7.10.2

2012-05-31 14:38:07

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 2/3] perf tools: Update ioctl documentation for PERF_IOC_FLAG_GROUP

Em Thu, May 31, 2012 at 11:35:54AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, May 31, 2012 at 02:51:45PM +0900, Namhyung Kim escreveu:
> > The ioctl interface of perf event fd receives 3 arguments
> > to control event group behavior but it lacked documentation.
>
> But the argument is not used for ENABLE or DISABLE, so why require it?

Nevermind, colour me blind, applying.

- Arnaldo

2012-05-31 17:00:25

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf: Remove duplicate invocation on perf_event_for_each

On Thu, 2012-05-31 at 14:51 +0900, Namhyung Kim wrote:
> The @func callback was invoked twice for group leader
> when perf_event_for_each() called. It seems the commit
> 75f937f24bd9 ("perf_counter: Fix ctx->mutex vs counter
> ->mutex inversion") made the mistake during the change.
>
> Signed-off-by: Namhyung Kim <[email protected]>

Good spotting, thanks!

> ---
> kernel/events/core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 5b06cbbf6931..f85c0154b333 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3181,7 +3181,6 @@ static void perf_event_for_each(struct perf_event *event,
> event = event->group_leader;
>
> perf_event_for_each_child(event, func);
> - func(event);
> list_for_each_entry(sibling, &event->sibling_list, group_entry)
> perf_event_for_each_child(sibling, func);
> mutex_unlock(&ctx->mutex);

2012-06-06 07:03:54

by Namhyung Kim

[permalink] [raw]
Subject: [tip:perf/urgent] perf tools: Update ioctl documentation for PERF_IOC_FLAG_GROUP

Commit-ID: a59e64a13a927fb7530bef39e9f5e7de8268137e
Gitweb: http://git.kernel.org/tip/a59e64a13a927fb7530bef39e9f5e7de8268137e
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 31 May 2012 14:51:45 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 31 May 2012 11:38:42 -0300

perf tools: Update ioctl documentation for PERF_IOC_FLAG_GROUP

The ioctl interface of perf event fd receives 3 arguments to control
event group behavior but it lacked documentation.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/design.txt | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/design.txt b/tools/perf/design.txt
index bd0bb1b..67e5d0c 100644
--- a/tools/perf/design.txt
+++ b/tools/perf/design.txt
@@ -409,14 +409,15 @@ Counters can be enabled and disabled in two ways: via ioctl and via
prctl. When a counter is disabled, it doesn't count or generate
events but does continue to exist and maintain its count value.

-An individual counter or counter group can be enabled with
+An individual counter can be enabled with

- ioctl(fd, PERF_EVENT_IOC_ENABLE);
+ ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);

or disabled with

- ioctl(fd, PERF_EVENT_IOC_DISABLE);
+ ioctl(fd, PERF_EVENT_IOC_DISABLE, 0);

+For a counter group, pass PERF_IOC_FLAG_GROUP as the third argument.
Enabling or disabling the leader of a group enables or disables the
whole group; that is, while the group leader is disabled, none of the
counters in the group will count. Enabling or disabling a member of a

2012-06-06 07:04:42

by Namhyung Kim

[permalink] [raw]
Subject: [tip:perf/urgent] perf evlist: Pass third argument to ioctl explicitly

Commit-ID: 55da80059de6c7533724fcd95f16c5d5618ecf4d
Gitweb: http://git.kernel.org/tip/55da80059de6c7533724fcd95f16c5d5618ecf4d
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 31 May 2012 14:51:46 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 31 May 2012 11:39:16 -0300

perf evlist: Pass third argument to ioctl explicitly

The ioctl on perf event fd wants 3 arguments but we only passed 2. As
the only user of the functions is perf record and it calls them for
every event (regardless of group setting), just pass 0 for now.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/evlist.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index ed277e5..7400fb3 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -274,7 +274,8 @@ void perf_evlist__disable(struct perf_evlist *evlist)
for (cpu = 0; cpu < evlist->cpus->nr; cpu++) {
list_for_each_entry(pos, &evlist->entries, node) {
for (thread = 0; thread < evlist->threads->nr; thread++)
- ioctl(FD(pos, cpu, thread), PERF_EVENT_IOC_DISABLE);
+ ioctl(FD(pos, cpu, thread),
+ PERF_EVENT_IOC_DISABLE, 0);
}
}
}
@@ -287,7 +288,8 @@ void perf_evlist__enable(struct perf_evlist *evlist)
for (cpu = 0; cpu < evlist->cpus->nr; cpu++) {
list_for_each_entry(pos, &evlist->entries, node) {
for (thread = 0; thread < evlist->threads->nr; thread++)
- ioctl(FD(pos, cpu, thread), PERF_EVENT_IOC_ENABLE);
+ ioctl(FD(pos, cpu, thread),
+ PERF_EVENT_IOC_ENABLE, 0);
}
}
}

2012-06-06 07:09:59

by Namhyung Kim

[permalink] [raw]
Subject: [tip:perf/urgent] perf: Remove duplicate invocation on perf_event_for_each

Commit-ID: cb7225feec627e91d598198996429e9ee6804f8d
Gitweb: http://git.kernel.org/tip/cb7225feec627e91d598198996429e9ee6804f8d
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 31 May 2012 14:51:44 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 31 May 2012 14:01:00 -0300

perf: Remove duplicate invocation on perf_event_for_each

The @func callback was invoked twice for group leader when
perf_event_for_each() called. It seems the commit 75f937f24bd9
("perf_counter: Fix ctx->mutex vs counter ->mutex inversion") made the
mistake during the change.

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
kernel/events/core.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5b06cbb..f85c015 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3181,7 +3181,6 @@ static void perf_event_for_each(struct perf_event *event,
event = event->group_leader;

perf_event_for_each_child(event, func);
- func(event);
list_for_each_entry(sibling, &event->sibling_list, group_entry)
perf_event_for_each_child(sibling, func);
mutex_unlock(&ctx->mutex);