Re: [RFC PATCH v3 2/5] cpuidle: Add Cpufreq Active Stats calls tracking idle entry/exit

[Lukasz Luba]

On 4/26/22 17:29, Artem Bityutskiy wrote:
> On Tue, 2022-04-26 at 16:01 +0100, Lukasz Luba wrote:
> > > I am worried about adding more stuff here.
> > >
> > > Please, consider getting the stats after interrupts are re-enabled. You may
> > > lose
> > > some "precision" because of that, but it is probably overall better that
> > > adding
> > > to idle interrupt latency.
> >
> > Definitely. I don't need such precision, so later when interrupts are
> > re-enabled is OK for me.
>
> Thanks. That is preferable in general: we do not do things with interrupts
> disabled unless there is a very good reason to.
>
> > This new call might be empty for your x86 kernels, since probably
> > you set the CONFIG_CPU_FREQ_STAT.I can add additional config
> > so platforms might still have CONFIG_CPU_FREQ_STAT but avoid this
> > new feature and additional overhead in idle exit when e.g.
> > CONFIG_CPU_FREQ_ACTIVE_STAT is not set.
> >
> > The x86 platforms won't use IPA governor, so it's reasonable to
> > do this way.
> >
> > Does this sounds good?
>
> I did not thoroughly read your patches so can't comment on the details.
>
> Just pointing that in general idle path is to be considered the critical path,
> especially the part before interrupts are re-enabled. Not only on x86,
> but on all platforms using cpuidle. This does not mean we can't read more
> statistics there, but it does mean that we should be very careful about added
> overhead, keep it under control, etc.

I totally agree with you. I didn't know that the interrupts were not
enabled at that moment. I'll address it.

Regards,
Lukasz