Re: [PATCH] perf/core: Optimize event reschedule for a PMU

[Namhyung Kim]

Hi Peter,

On Tue, Jul 30, 2024 at 1:40 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> On Tue, Jul 30, 2024 at 12:19:25PM -0700, Namhyung Kim wrote:
> > @@ -2728,13 +2727,62 @@ static void ctx_resched(struct perf_cpu_context *cpuctx,
> >               perf_ctx_enable(task_ctx, false);
> >  }
> >
> > +static void __perf_pmu_resched(struct pmu *pmu,
> > +                            struct perf_event_context *task_ctx,
> > +                            enum event_type_t event_type)
> > +{
> > +     bool cpu_event = !!(event_type & EVENT_CPU);
> > +     struct perf_event_pmu_context *epc = NULL;
> > +     struct perf_cpu_pmu_context *cpc = this_cpu_ptr(pmu->cpu_pmu_context);
> > +
> > +     /*
> > +      * If pinned groups are involved, flexible groups also need to be
> > +      * scheduled out.
> > +      */
> > +     if (event_type & EVENT_PINNED)
> > +             event_type |= EVENT_FLEXIBLE;
> > +
> > +     event_type &= EVENT_ALL;
> > +
> > +     perf_pmu_disable(pmu);
> > +     if (task_ctx) {
> > +             if (WARN_ON_ONCE(!cpc->task_epc || cpc->task_epc->ctx != task_ctx))
> > +                     goto out;
> > +
> > +             epc = cpc->task_epc;
> > +             __pmu_ctx_sched_out(epc, event_type);
> > +     }
> > +
> > +     /*
> > +      * Decide which cpu ctx groups to schedule out based on the types
> > +      * of events that caused rescheduling:
> > +      *  - EVENT_CPU: schedule out corresponding groups;
> > +      *  - EVENT_PINNED task events: schedule out EVENT_FLEXIBLE groups;
> > +      *  - otherwise, do nothing more.
> > +      */
> > +     if (cpu_event)
> > +             __pmu_ctx_sched_out(&cpc->epc, event_type);
> > +     else if (event_type & EVENT_PINNED)
> > +             __pmu_ctx_sched_out(&cpc->epc, EVENT_FLEXIBLE);
> > +
> > +     __pmu_ctx_sched_in(&cpc->epc, EVENT_PINNED);
> > +     if (task_ctx)
> > +              __pmu_ctx_sched_in(epc, EVENT_PINNED);
> > +     __pmu_ctx_sched_in(&cpc->epc, EVENT_FLEXIBLE);
> > +     if (task_ctx)
> > +              __pmu_ctx_sched_in(epc, EVENT_FLEXIBLE);
> > +
> > +out:
> > +     perf_pmu_enable(pmu);
> > +}
>
> I so dislike duplication...
>
> So lets see, ctx_resched() has pmu_ctx iterations in:
>
>   perf_ctx_{en,dis}able()
>   ctx_sched_{in,out}()
>
> Can't we punch a 'struct pmu *pmu' argument through those callchains and
> short-circuit the iteration when !NULL?

Sure, will do that in v2.

Thanks,
Namhyung

>
> The alternative would be to lift the iteration I suppose.