| From: Song Liu <songliubraving@fb.com> |
| Date: Thu, 3 May 2018 12:47:16 -0700 |
| Subject: perf/core: Fix group scheduling with mixed hw and sw events |
| |
| commit a1150c202207cc8501bebc45b63c264f91959260 upstream. |
| |
| When hw and sw events are mixed in the same group, they are all attached |
| to the hw perf_event_context. This sometimes requires moving group of |
| perf_event to a different context. |
| |
| We found a bug in how the kernel handles this, for example if we do: |
| |
| perf stat -e '{faults,ref-cycles,faults}' -I 1000 |
| |
| 1.005591180 1,297 faults |
| 1.005591180 457,476,576 ref-cycles |
| 1.005591180 <not supported> faults |
| |
| First, sw event "faults" is attached to the sw context, and becomes the |
| group leader. Then, hw event "ref-cycles" is attached, so both events |
| are moved to the hw context. Last, another sw "faults" tries to attach, |
| but it fails because of mismatch between the new target ctx (from sw |
| pmu) and the group_leader's ctx (hw context, same as ref-cycles). |
| |
| The broken condition is: |
| group_leader is sw event; |
| group_leader is on hw context; |
| add a sw event to the group. |
| |
| Fix this scenario by checking group_leader's context (instead of just |
| event type). If group_leader is on hw context, use the ->pmu of this |
| context to look up context for the new event. |
| |
| Signed-off-by: Song Liu <songliubraving@fb.com> |
| Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> |
| Cc: <kernel-team@fb.com> |
| Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> |
| Cc: Arnaldo Carvalho de Melo <acme@redhat.com> |
| Cc: Jiri Olsa <jolsa@redhat.com> |
| Cc: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Stephane Eranian <eranian@google.com> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Cc: Vince Weaver <vincent.weaver@maine.edu> |
| Fixes: b04243ef7006 ("perf: Complete software pmu grouping") |
| Link: http://lkml.kernel.org/r/20180503194716.162815-1-songliubraving@fb.com |
| Signed-off-by: Ingo Molnar <mingo@kernel.org> |
| [bwh: Backported to 3.16: adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| include/linux/perf_event.h | 8 ++++++++ |
| kernel/events/core.c | 21 +++++++++++---------- |
| 2 files changed, 19 insertions(+), 10 deletions(-) |
| |
| --- a/include/linux/perf_event.h |
| +++ b/include/linux/perf_event.h |
| @@ -640,6 +640,14 @@ static inline int is_software_event(stru |
| return event->pmu->task_ctx_nr == perf_sw_context; |
| } |
| |
| +/* |
| + * Return 1 for event in sw context, 0 for event in hw context |
| + */ |
| +static inline int in_software_context(struct perf_event *event) |
| +{ |
| + return event->ctx->pmu->task_ctx_nr == perf_sw_context; |
| +} |
| + |
| extern struct static_key perf_swevent_enabled[PERF_COUNT_SW_MAX]; |
| |
| extern void ___perf_sw_event(u32, u64, struct pt_regs *, u64); |
| --- a/kernel/events/core.c |
| +++ b/kernel/events/core.c |
| @@ -7502,19 +7502,20 @@ SYSCALL_DEFINE5(perf_event_open, |
| */ |
| pmu = event->pmu; |
| |
| - if (group_leader && |
| - (is_software_event(event) != is_software_event(group_leader))) { |
| - if (is_software_event(event)) { |
| + if (group_leader) { |
| + if (is_software_event(event) && |
| + !in_software_context(group_leader)) { |
| /* |
| - * If event and group_leader are not both a software |
| - * event, and event is, then group leader is not. |
| + * If the event is a sw event, but the group_leader |
| + * is on hw context. |
| * |
| - * Allow the addition of software events to !software |
| - * groups, this is safe because software events never |
| - * fail to schedule. |
| + * Allow the addition of software events to hw |
| + * groups, this is safe because software events |
| + * never fail to schedule. |
| */ |
| - pmu = group_leader->pmu; |
| - } else if (is_software_event(group_leader) && |
| + pmu = group_leader->ctx->pmu; |
| + } else if (!is_software_event(event) && |
| + is_software_event(group_leader) && |
| (group_leader->group_flags & PERF_GROUP_SOFTWARE)) { |
| /* |
| * In case the group is a pure software group, and we |