Hi, On 17.10.2018 19:30, Peter Zijlstra wrote: > On Wed, Oct 17, 2018 at 11:57:49AM +0300, Alexey Budankov wrote: >> Hi, >> >> On 10.10.2018 13:45, Peter Zijlstra wrote: >> <SNIP> >>> -static bool perf_rotate_context(struct perf_cpu_context *cpuctx) >>> +/* >>> + * XXX somewhat completely buggered; this is in cpu_pmu_context, but we >>> need >>> + * event_pmu_context for rotations. We also need event_pmu_context specific >>> + * scheduling routines. ARGH >>> + * >>> + * - fixed the cpu_pmu_context vs event_pmu_context thingy >>> + * (cpu_pmu_context embeds an event_pmu_context) >>> + * >>> + * - need nr_events/nr_active in epc to do per epc rotation >>> + * (done) >>> + * >>> + * - need cpu and task pmu ctx together... >>> + * (cpc->task_epc) >>> + */ >>> +static bool perf_rotate_context(struct perf_cpu_pmu_context *cpc) >> >> Since it reduces to single cpu context (and single task context) at all >> times, >> ideally, it would probably be coded as simple as this: >> >> perf_rotate_context() >> { >> cpu = this_cpu_ptr(&cpu_context) >> for_every_pmu(pmu, cpu) > > Can't do that, because we have per PMU rotation periods..
Well, yes, the callback is already called per-cpu per-pmu, so then this simplifies a bit, like this: perf_rotate_context(pmu, cpu) { for_every_event_ctx(event_ctx, pmu) rotate(event_ctx, pmu) } event_ctx | v pmu (struct perf_cpu_pmu_context) -> ctx__0 -> ctx__1 | | v v sched_out -> fgroup00 fgroup01 -> event001 -> event101 -> event201 | ^ | ^ v | v | fgroup10 fgroup11 | | | | v | v | sched_in -> fgroup20 fgroup21 > >> for_every_event_ctx(event_ctx, pmu) >> rotate(event_ctx, pmu) >> } > > I'm also not sure I get the rest that follows... you only have to rotate > _one_ event per PMU. Yes. One group per PMU. It could end up in several HW counters reprogramming. Thanks, Alexey > > I'll try and understand the rest of you email later; brain has checked > out for the day. >