Re: [PATCH v1 2/2] cpuidle: teo: Refine handling of short idle intervals

[Rafael J. Wysocki]

On Wed, Apr 16, 2025 at 5:00 PM Christian Loehle
<christian.loehle@xxxxxxx> wrote:
>
> On 4/3/25 20:18, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > Make teo take all recent wakeups (both timer and non-timer) into
> > account when looking for a new candidate idle state in the cases
> > when the majority of recent idle intervals are within the
> > LATENCY_THRESHOLD_NS range or the latency limit is within the
> > LATENCY_THRESHOLD_NS range.
> >
> > Since the tick_nohz_get_sleep_length() invocation is likely to be
> > skipped in those cases, timer wakeups should arguably be taken into
> > account somehow in case they are significant while the current code
> > mostly looks at non-timer wakeups under the assumption that frequent
> > timer wakeups are unlikely in the given idle duration range which
> > may or may not be accurate.
> >
> > The most natural way to do that is to add the "hits" metric to the
> > sums used during the new candidate idle state lookup which effectively
> > means the above.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Hi Rafael,
> I might be missing something so bare with me.
> Quoting the cover-letter too:
> "In those cases, timer wakeups are not taken into account when they are
> within the LATENCY_THRESHOLD_NS range and the idle state selection may
> be based entirely on non-timer wakeups which may be rare.  This causes
> the prediction accuracy to be low and too much energy may be used as
> a result.
>
> The first patch is preparatory and it is not expected to make any
> functional difference.
>
> The second patch causes teo to take timer wakeups into account if it
> is about to skip the tick_nohz_get_sleep_length() invocation, so they
> get a chance to influence the idle state selection."
>
> If the timer wakeups are < LATENCY_THRESHOLD_NS we will not do
>
> cpu_data->sleep_length_ns = tick_nohz_get_sleep_length(&delta_tick);
>
> but
>
> cpu_data->sleep_length_ns = KTIME_MAX;
>
> therefore
> idx_timer = drv->state_count - 1
> idx_duration = some state with residency < LATENCY_THRESHOLD_NS
>
> For any reasonable system therefore idx_timer != idx_duration
> (i.e. there's an idle state deeper than LATENCY_THRESHOLD_NS).
> So hits will never be incremented?

Why never?

First of all, you need to get into the "2 * cpu_data->short_idles >=
cpu_data->total" case somehow and this may be through timer wakeups.

> How would adding hits then help this case?

They may be dominant when this condition triggers for the first time.