From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id F224C34889A; Wed, 12 Nov 2025 18:00:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762970408; cv=none; b=ZRJ4Nm8NghuZFY5PHhmBQ2dLlPXLxJtffvGuvctLQXEjYqv7YwBVzleAVVKggqrmZgBtHrDGpt61AuAPpCJY9eyuuaRvS/s/kdQcaoKPHtpTYPP1uHS0xBj+Uzjztju9xGfQi6KxMHetUD3MAhoGOkR8QnYtWbgJyvywlHMBWfY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762970408; c=relaxed/simple; bh=AoCqvY0tBbLj146iF+WGYiI7uWng/r6twSXwU/G3En4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Dfic66EGgkuLmXCkEUTYpE7pXLU0rGGbiKWQVYx05+98FE3iO68i7OkqupJduRILAm5wvw4bROECPbq9woJAUIp9xR0516YXXNr2j4E8KaBcFGgiWmCc0UBvZdmOj7rROPambcoQRcVNrqJJUaL2iCDH7cizboGC/oYSYSeq7+4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6D1A81515; Wed, 12 Nov 2025 09:59:57 -0800 (PST) Received: from [10.1.28.59] (e127648.arm.com [10.1.28.59]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2F0243F63F; Wed, 12 Nov 2025 10:00:04 -0800 (PST) Message-ID: <8baa1d22-c3ad-4c62-a70e-fc64bfbfdf0e@arm.com> Date: Wed, 12 Nov 2025 18:00:01 +0000 Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 4/4] cpuidle: governors: teo: Decay metrics below DECAY_SHIFT threshold To: "Rafael J. Wysocki" Cc: Linux PM , LKML , Reka Norman References: <4701737.LvFx2qVVIh@rafael.j.wysocki> <3396811.44csPzL39Z@rafael.j.wysocki> Content-Language: en-US From: Christian Loehle In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 11/12/25 17:51, Rafael J. Wysocki wrote: > On Wed, Nov 12, 2025 at 6:29 PM Christian Loehle > wrote: >> >> On 11/12/25 16:25, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki >>> >>> If a given governor metric falls below a certain value (8 for >>> DECAY_SHIFT equal to 3), it will not decay any more due to the >>> simplistic decay implementation. This may in some cases lead to >>> subtle inconsistencies in the governor behavior, so change the >>> decay implementation to take it into account and set the metric >>> at hand to 0 in that case. >>> >>> Suggested-by: Christian Loehle >>> Signed-off-by: Rafael J. Wysocki >>> --- >>> drivers/cpuidle/governors/teo.c | 20 +++++++++++++++----- >>> 1 file changed, 15 insertions(+), 5 deletions(-) >>> >>> --- a/drivers/cpuidle/governors/teo.c >>> +++ b/drivers/cpuidle/governors/teo.c >>> @@ -148,6 +148,16 @@ struct teo_cpu { >>> >>> static DEFINE_PER_CPU(struct teo_cpu, teo_cpus); >>> >>> +static void teo_decay(unsigned int *metric) >>> +{ >>> + unsigned int delta = *metric >> DECAY_SHIFT; >>> + >>> + if (delta) >>> + *metric -= delta; >>> + else >>> + *metric = 0; >>> +} >>> + >>> /** >>> * teo_update - Update CPU metrics after wakeup. >>> * @drv: cpuidle driver containing state data. >>> @@ -159,7 +169,7 @@ static void teo_update(struct cpuidle_dr >>> int i, idx_timer = 0, idx_duration = 0; >>> s64 target_residency_ns, measured_ns; >>> >>> - cpu_data->short_idles -= cpu_data->short_idles >> DECAY_SHIFT; >>> + teo_decay(&cpu_data->short_idles); >>> >>> if (cpu_data->artificial_wakeup) { >>> /* >>> @@ -195,8 +205,8 @@ static void teo_update(struct cpuidle_dr >>> for (i = 0; i < drv->state_count; i++) { >>> struct teo_bin *bin = &cpu_data->state_bins[i]; >>> >>> - bin->hits -= bin->hits >> DECAY_SHIFT; >>> - bin->intercepts -= bin->intercepts >> DECAY_SHIFT; >>> + teo_decay(&bin->hits); >>> + teo_decay(&bin->intercepts); >>> >>> target_residency_ns = drv->states[i].target_residency_ns; >>> >>> @@ -207,7 +217,7 @@ static void teo_update(struct cpuidle_dr >>> } >>> } >>> >>> - cpu_data->tick_intercepts -= cpu_data->tick_intercepts >> DECAY_SHIFT; >>> + teo_decay(&cpu_data->tick_intercepts); >>> /* >>> * If the measured idle duration falls into the same bin as the sleep >>> * length, this is a "hit", so update the "hits" metric for that bin. >>> @@ -222,7 +232,7 @@ static void teo_update(struct cpuidle_dr >>> cpu_data->tick_intercepts += PULSE; >>> } >>> >>> - cpu_data->total -= cpu_data->total >> DECAY_SHIFT; >>> + teo_decay(&cpu_data->total); >>> cpu_data->total += PULSE; >> >> This will result in total no longer being a strict sum of the bins. > > Ah, good point. > >> Any reason not to do something like: > > Well, it would be more straightforward to just compute "total" from > scratch instead of using total_decay (it would be the same amount of > computation minus the teo_decay() changes AFAICS). Duh, of course... > > I'll send an update of this patch. Thanks!