> On Oct 15, 2018, at 1:34 AM, Peter Zijlstra <pet...@infradead.org> wrote:
>
> On Mon, Oct 15, 2018 at 10:26:06AM +0300, Alexey Budankov wrote:
>> Hi,
>>
>> On 10.10.2018 13:45, Peter Zijlstra wrote:
>>> Hi all,
>>>
>>> There have been various issues and limitations with the way perf uses
>>> (task) contexts to track events. Most notable is the single hardware PMU
>>> task context, which has resulted in a number of yucky things (both
>>> proposed and merged).
>>>
>>> Notably:
>>>
>>> - HW breakpoint PMU
>>> - ARM big.little PMU
>>> - Intel Branch Monitoring PMU
>>>
>>> Since we now track the events in RB trees, we can 'simply' add a pmu
>>> order to them and have them grouped that way, reducing to a single
>>> context. Of course, reality never quite works out that simple, and below
>>> ends up adding an intermediate data structure to bridge the context ->
>>> pmu mapping.
>>>
>>> Something a little like:
>>>
>>> ,------------------------[1:n]---------------------.
>>> V V
>>> perf_event_context <-[1:n]-> perf_event_pmu_context <--- perf_event
>>> ^ ^ | |
>>> `--------[1:n]---------' `-[n:1]-> pmu <-[1:n]-'
>>>
>>> This patch builds (provided you disable CGROUP_PERF), boots and survives
>>> perf-top without the machine catching fire.
>>>
>>> There's still a fair bit of loose ends (look for XXX), but I think this
>>> is the direction we should be going.
>>>
>>> Comments?
>>>
>>> Not-Quite-Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
>>> ---
>>> arch/powerpc/perf/core-book3s.c | 4
>>> arch/x86/events/core.c | 4
>>> arch/x86/events/intel/core.c | 6
>>> arch/x86/events/intel/ds.c | 6
>>> arch/x86/events/intel/lbr.c | 16
>>> arch/x86/events/perf_event.h | 6
>>> include/linux/perf_event.h | 80 +-
>>> include/linux/sched.h | 2
>>> kernel/events/core.c | 1412
>>> ++++++++++++++++++++--------------------
>>> 9 files changed, 815 insertions(+), 721 deletions(-)
>>
>> Rewrite is impressive however it doesn't result in code base reduction as it
>> is.
>
> Yeah.. that seems to be nature of these things ..
>
>> Nonetheless there is a clear demand for per pmu events groups tracking and
>> rotation
>> in single cpu context (HW breakpoints, ARM big.little, Intel LBRs) and there
>> is
>> a supply thru groups ordering on RB-tree.
>>
>> This might be driven into the kernel by some new Perf features that would
>> base on
>> that RB-tree groups ordering or by refactoring of existing code but in the
>> way it
>> would result in overall code base reduction thus lowering support cost.
>
> If you have a concrete suggestion on how to reduce complexity? I tried,
> but couldn't find any (without breaking something).
>
> The active lists and pmu_ctx_list could arguably be replaced with
> (slower) iteratons over the RB tree, but you'll still need the per pmu
> nr_events/nr_active counts to determine if rotation is required at all.
>
> And like you know, performance is quite important here too. I'd love to
> reduce complexity while maintaining or improve performance, but that
> rarely if ever happens :/
How about this:
1. Keep multiple perf_cpu_context per CPU, just like before this patch.
2. For perf_event_context, add PMU as an order for the RB tree.
3. (hw) pmu->perf_cpu_context->ctx only has events for this PMU (and sw
events moved to this context).
4. task->perf_event_ctxp has events for all PMUs.
With this path, we keep the existing perf_cpu_context/perf_event_context
logic as-is, which I think is simpler than the new logic (with extra
*_pmu_context). And it should also solve the problem.
Does this make sense? If this doesn't look too broken, I am happy to
draft RFC for it.
Thanks,
Song
