2017-06-17 07:43:47

by Zhou Chengming

Subject: [PATCH] perf/core: make sure group events are for the same cpu

The else branch are broken for taskctx: two events can on the same
taskctx, but on different cpu. This patch fix it, we don't need to
check move_group. We first make sure we're on the same task, or both
per-cpu events, and then make sure we're events for the same cpu.

Signed-off-by: Zhou Chengming <[email protected]>
---
kernel/events/core.c | 31 +++++++++++--------------------
1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 6c4e523..59270bd 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10010,28 +10010,19 @@ static int perf_event_set_clock(struct perf_event *event, clockid_t clk_id)
goto err_context;

/*
- * Do not allow to attach to a group in a different
- * task or CPU context:
+ * Make sure we're both on the same task, or both
+ * per-cpu events.
*/
- if (move_group) {
- /*
- * Make sure we're both on the same task, or both
- * per-cpu events.
- */
- if (group_leader->ctx->task != ctx->task)
- goto err_context;
+ if (group_leader->ctx->task != ctx->task)
+ goto err_context;

- /*
- * Make sure we're both events for the same CPU;
- * grouping events for different CPUs is broken; since
- * you can never concurrently schedule them anyhow.
- */
- if (group_leader->cpu != event->cpu)
- goto err_context;
- } else {
- if (group_leader->ctx != ctx)
- goto err_context;
- }
+ /*
+ * Make sure we're both events for the same CPU;
+ * grouping events for different CPUs is broken; since
+ * you can never concurrently schedule them anyhow.
+ */
+ if (group_leader->cpu != event->cpu)
+ goto err_context;

/*
* Only a group leader can be exclusive or pinned
--
1.8.3.1

2017-06-20 13:13:10

by Alexander Shishkin

Subject: Re: [PATCH] perf/core: make sure group events are for the same cpu

Zhou Chengming <[email protected]> writes:

> The else branch are broken for taskctx:

This is not a good way to open a commit message.

> two events can on the same taskctx, but on different cpu.

How?

Regards,
--
Alex

2017-06-21 06:56:02

by Zhou Chengming

Subject: Re: [PATCH] perf/core: make sure group events are for the same cpu

On 2017/6/20 21:08, Alexander Shishkin wrote:
> Zhou Chengming<[email protected]> writes:
>
>> The else branch are broken for taskctx:
> This is not a good way to open a commit message.
>
>> two events can on the same taskctx, but on different cpu.
> How?

fd1 = perf_open_event(attr, pid, 0, -1, flags);
fd2 = perf_open_event(attr, pid, 1, fd1, flags);

fd1 will be the leader event, fd2 will be the sibling event in the group.

And they are for the same task, so they will be put on the same taskctx successfully
if !move_group.

Obviously it's wrong, we can't concurrently schedule them as a group, since they are
on different cpu.

Thanks.

>
> Regards,
> --
> Alex
>
> .
>