[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0g-=DB_W5jkxxCERy22jz9a_V1Tcj8hiVwL6_R+xSM=gQ@mail.gmail.com>
Date: Wed, 16 Apr 2025 17:28:22 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Christian Loehle <christian.loehle@....com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>, Linux PM <linux-pm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>, Daniel Lezcano <daniel.lezcano@...aro.org>,
Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>, Doug Smythies <dsmythies@...us.net>,
Aboorva Devarajan <aboorvad@...ux.ibm.com>
Subject: Re: [PATCH v1 2/2] cpuidle: teo: Refine handling of short idle intervals
On Wed, Apr 16, 2025 at 5:00 PM Christian Loehle
<christian.loehle@....com> wrote:
>
> On 4/3/25 20:18, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > 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@...el.com>
>
> 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.
Powered by blists - more mailing lists