From patchwork Thu Oct 10 21:32:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 11184513 X-Patchwork-Delegate: rjw@sisk.pl Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 64A541864 for ; Thu, 10 Oct 2019 21:34:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4F7FA21D71 for ; Thu, 10 Oct 2019 21:34:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725867AbfJJVeT (ORCPT ); Thu, 10 Oct 2019 17:34:19 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:43958 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726983AbfJJVeO (ORCPT ); Thu, 10 Oct 2019 17:34:14 -0400 Received: from 79.184.255.36.ipv4.supernova.orange.pl (79.184.255.36) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.292) id e382dcc9448de146; Thu, 10 Oct 2019 23:34:12 +0200 From: "Rafael J. Wysocki" To: Linux PM Cc: LKML , Srinivas Pandruvada , Peter Zijlstra , Daniel Lezcano , Doug Smythies Subject: [PATCH 1/4] cpuidle: teo: Ignore disabled idle states that are too deep Date: Thu, 10 Oct 2019 23:32:17 +0200 Message-ID: <6803360.ubpITS43T2@kreacher> In-Reply-To: <60416800.X4hXmAfbqi@kreacher> References: <60416800.X4hXmAfbqi@kreacher> MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: Rafael J. Wysocki Prevent disabled CPU idle state with target residencies beyond the anticipated idle duration from being taken into account by the TEO governor. Fixes: b26bf6ab716f ("cpuidle: New timer events oriented governor for tickless systems") Signed-off-by: Rafael J. Wysocki Acked-by: Doug Smythies --- drivers/cpuidle/governors/teo.c | 7 +++++++ 1 file changed, 7 insertions(+) Index: linux-pm/drivers/cpuidle/governors/teo.c =================================================================== --- linux-pm.orig/drivers/cpuidle/governors/teo.c +++ linux-pm/drivers/cpuidle/governors/teo.c @@ -258,6 +258,13 @@ static int teo_select(struct cpuidle_dri if (s->disabled || su->disable) { /* + * Ignore disabled states with target residencies beyond + * the anticipated idle duration. + */ + if (s->target_residency > duration_us) + continue; + + /* * If the "early hits" metric of a disabled state is * greater than the current maximum, it should be taken * into account, because it would be a mistake to select From patchwork Thu Oct 10 21:32:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 11184509 X-Patchwork-Delegate: rjw@sisk.pl Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4764C1864 for ; Thu, 10 Oct 2019 21:34:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3148521A4A for ; Thu, 10 Oct 2019 21:34:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727164AbfJJVeO (ORCPT ); Thu, 10 Oct 2019 17:34:14 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:58721 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726977AbfJJVeO (ORCPT ); Thu, 10 Oct 2019 17:34:14 -0400 Received: from 79.184.255.36.ipv4.supernova.orange.pl (79.184.255.36) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.292) id 3a48057531757d0a; Thu, 10 Oct 2019 23:34:11 +0200 From: "Rafael J. Wysocki" To: Linux PM Cc: LKML , Srinivas Pandruvada , Peter Zijlstra , Daniel Lezcano , Doug Smythies Subject: [PATCH 2/4] cpuidle: teo: Rename local variable in teo_select() Date: Thu, 10 Oct 2019 23:32:59 +0200 Message-ID: <5976014.dk64zBpnPU@kreacher> In-Reply-To: <60416800.X4hXmAfbqi@kreacher> References: <60416800.X4hXmAfbqi@kreacher> MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: Rafael J. Wysocki Rename a local variable in teo_select() in preparation for subsequent code modifications, no intentional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Doug Smythies --- drivers/cpuidle/governors/teo.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) Index: linux-pm/drivers/cpuidle/governors/teo.c =================================================================== --- linux-pm.orig/drivers/cpuidle/governors/teo.c +++ linux-pm/drivers/cpuidle/governors/teo.c @@ -233,7 +233,7 @@ static int teo_select(struct cpuidle_dri { struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu); int latency_req = cpuidle_governor_latency_req(dev->cpu); - unsigned int duration_us, count; + unsigned int duration_us, early_hits; int max_early_idx, constraint_idx, idx, i; ktime_t delta_tick; @@ -247,7 +247,7 @@ static int teo_select(struct cpuidle_dri cpu_data->sleep_length_ns = tick_nohz_get_sleep_length(&delta_tick); duration_us = ktime_to_us(cpu_data->sleep_length_ns); - count = 0; + early_hits = 0; max_early_idx = -1; constraint_idx = drv->state_count; idx = -1; @@ -270,12 +270,12 @@ static int teo_select(struct cpuidle_dri * into account, because it would be a mistake to select * a deeper state with lower "early hits" metric. The * index cannot be changed to point to it, however, so - * just increase the max count alone and let the index - * still point to a shallower idle state. + * just increase the "early hits" count alone and let + * the index still point to a shallower idle state. */ if (max_early_idx >= 0 && - count < cpu_data->states[i].early_hits) - count = cpu_data->states[i].early_hits; + early_hits < cpu_data->states[i].early_hits) + early_hits = cpu_data->states[i].early_hits; continue; } @@ -291,10 +291,10 @@ static int teo_select(struct cpuidle_dri idx = i; - if (count < cpu_data->states[i].early_hits && + if (early_hits < cpu_data->states[i].early_hits && !(tick_nohz_tick_stopped() && drv->states[i].target_residency < TICK_USEC)) { - count = cpu_data->states[i].early_hits; + early_hits = cpu_data->states[i].early_hits; max_early_idx = i; } } @@ -323,10 +323,9 @@ static int teo_select(struct cpuidle_dri if (idx < 0) { idx = 0; /* No states enabled. Must use 0. */ } else if (idx > 0) { + unsigned int count = 0; u64 sum = 0; - count = 0; - /* * Count and sum the most recent idle duration values less than * the current expected idle duration value. From patchwork Thu Oct 10 21:36:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 11184515 X-Patchwork-Delegate: rjw@sisk.pl Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6327F112B for ; Thu, 10 Oct 2019 21:36:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4CFEC214E0 for ; Thu, 10 Oct 2019 21:36:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727014AbfJJVgS (ORCPT ); Thu, 10 Oct 2019 17:36:18 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:65324 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726866AbfJJVgS (ORCPT ); Thu, 10 Oct 2019 17:36:18 -0400 Received: from 79.184.255.36.ipv4.supernova.orange.pl (79.184.255.36) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.292) id 3ad197a03b599e0a; Thu, 10 Oct 2019 23:36:15 +0200 From: "Rafael J. Wysocki" To: "Rafael J. Wysocki" Cc: Linux PM , LKML , Srinivas Pandruvada , Peter Zijlstra , Daniel Lezcano , Doug Smythies Subject: [PATCH 3/4] cpuidle: teo: Consider hits and misses metrics of disabled states Date: Thu, 10 Oct 2019 23:36:15 +0200 Message-ID: <2237950.TTgtiEjdez@kreacher> In-Reply-To: <60416800.X4hXmAfbqi@kreacher> References: <60416800.X4hXmAfbqi@kreacher> MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: Rafael J. Wysocki The TEO governor uses idle duration "bins" defined in accordance with the CPU idle states table provided by the driver, so that each "bin" covers the idle duration range between the target residency of the idle state corresponding to it and the target residency of the closest deeper idle state. The governor collects statistics for each bin regardless of whether or not the idle state corresponding to it is currently enabled. In particular, the "hits" and "misses" metrics measure the likelihood of a situation in which both the time till the next timer (sleep length) and the idle duration measured after wakeup fall into the given bin. Namely, if the "hits" value is greater than the "misses" one, that situation is more likely than the one in which the sleep length falls into the given bin, but the idle duration measured after wakeup falls into a bin corresponding to one of the shallower idle states. If the idle state corresponding to the given bin is disabled, it cannot be selected and if it turns out to be the one that should be selected, a shallower idle state needs to be used instead of it. Nevertheless, the metrics collected for the bin corresponding to it are still valid and need to be taken into account as though that state had not been disabled. For this reason, make teo_select() always use the "hits" and "misses" values of the idle duration range that the sleep length falls into even if the specific idle state corresponding to it is disabled and if the "hits" values is greater than the "misses" one, select the closest enabled shallower idle state in that case. Fixes: b26bf6ab716f ("cpuidle: New timer events oriented governor for tickless systems") Signed-off-by: Rafael J. Wysocki Acked-by: Doug Smythies --- drivers/cpuidle/governors/teo.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) Index: linux-pm/drivers/cpuidle/governors/teo.c =================================================================== --- linux-pm.orig/drivers/cpuidle/governors/teo.c +++ linux-pm/drivers/cpuidle/governors/teo.c @@ -233,7 +233,7 @@ static int teo_select(struct cpuidle_dri { struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu); int latency_req = cpuidle_governor_latency_req(dev->cpu); - unsigned int duration_us, early_hits; + unsigned int duration_us, hits, misses, early_hits; int max_early_idx, constraint_idx, idx, i; ktime_t delta_tick; @@ -247,6 +247,8 @@ static int teo_select(struct cpuidle_dri cpu_data->sleep_length_ns = tick_nohz_get_sleep_length(&delta_tick); duration_us = ktime_to_us(cpu_data->sleep_length_ns); + hits = 0; + misses = 0; early_hits = 0; max_early_idx = -1; constraint_idx = drv->state_count; @@ -265,6 +267,17 @@ static int teo_select(struct cpuidle_dri continue; /* + * This state is disabled, so the range of idle duration + * values corresponding to it is covered by the current + * candidate state, but still the "hits" and "misses" + * metrics of the disabled state need to be used to + * decide whether or not the state covering the range in + * question is good enough. + */ + hits = cpu_data->states[i].hits; + misses = cpu_data->states[i].misses; + + /* * If the "early hits" metric of a disabled state is * greater than the current maximum, it should be taken * into account, because it would be a mistake to select @@ -280,8 +293,11 @@ static int teo_select(struct cpuidle_dri continue; } - if (idx < 0) + if (idx < 0) { idx = i; /* first enabled state */ + hits = cpu_data->states[i].hits; + misses = cpu_data->states[i].misses; + } if (s->target_residency > duration_us) break; @@ -290,6 +306,8 @@ static int teo_select(struct cpuidle_dri constraint_idx = i; idx = i; + hits = cpu_data->states[i].hits; + misses = cpu_data->states[i].misses; if (early_hits < cpu_data->states[i].early_hits && !(tick_nohz_tick_stopped() && @@ -307,8 +325,7 @@ static int teo_select(struct cpuidle_dri * "early hits" metric, but if that cannot be determined, just use the * state selected so far. */ - if (cpu_data->states[idx].hits <= cpu_data->states[idx].misses && - max_early_idx >= 0) { + if (hits <= misses && max_early_idx >= 0) { idx = max_early_idx; duration_us = drv->states[idx].target_residency; } From patchwork Thu Oct 10 21:37:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 11184517 X-Patchwork-Delegate: rjw@sisk.pl Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9FEA5112B for ; Thu, 10 Oct 2019 21:37:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8A4CF218AC for ; Thu, 10 Oct 2019 21:37:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726866AbfJJVhm (ORCPT ); Thu, 10 Oct 2019 17:37:42 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:58988 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725867AbfJJVhl (ORCPT ); Thu, 10 Oct 2019 17:37:41 -0400 Received: from 79.184.255.36.ipv4.supernova.orange.pl (79.184.255.36) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.292) id cd4238d3936759af; Thu, 10 Oct 2019 23:37:39 +0200 From: "Rafael J. Wysocki" To: "Rafael J. Wysocki" Cc: Linux PM , LKML , Srinivas Pandruvada , Peter Zijlstra , Daniel Lezcano , Doug Smythies Subject: [PATCH 4/4] cpuidle: teo: Fix "early hits" handling for disabled idle states Date: Thu, 10 Oct 2019 23:37:39 +0200 Message-ID: <1746940.X44tFrgt99@kreacher> In-Reply-To: <60416800.X4hXmAfbqi@kreacher> References: <60416800.X4hXmAfbqi@kreacher> MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: Rafael J. Wysocki The TEO governor uses idle duration "bins" defined in accordance with the CPU idle states table provided by the driver, so that each "bin" covers the idle duration range between the target residency of the idle state corresponding to it and the target residency of the closest deeper idle state. The governor collects statistics for each bin regardless of whether or not the idle state corresponding to it is currently enabled. In particular, the "early hits" metric measures the likelihood of a situation in which the idle duration measured after wakeup falls into to given bin, but the time till the next timer (sleep length) falls into a bin corresponding to one of the deeper idle states. It is used when the "hits" and "misses" metrics indicate that the state "matching" the sleep length should not be selected, so that the state with the maximum "early hits" value is selected instead of it. If the idle state corresponding to the given bin is disabled, it cannot be selected and if it turns out to be the one that should be selected, a shallower idle state needs to be used instead of it. Nevertheless, the metrics collected for the bin corresponding to it are still valid and need to be taken into account as though that state had not been disabled. As far as the "early hits" metric is concerned, teo_select() tries to take disabled states into account, but the state index corresponding to the maximum "early hits" value computed by it may be incorrect. Namely, it always uses the index of the previous maximum "early hits" state then, but there may be enabled idle states closer to the disabled one in question. In particular, if the current candidate state (whose index is the idx value) is closer to the disabled one and the "early hits" value of the disabled state is greater than the current maximum, the index of the current candidate state (idx) should replace the "maximum early hits state" index. Modify the code to handle that case correctly. Fixes: b26bf6ab716f ("cpuidle: New timer events oriented governor for tickless systems") Reported-by: Doug Smythies Signed-off-by: Rafael J. Wysocki Tested-by: Doug Smythies --- drivers/cpuidle/governors/teo.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) Index: linux-pm/drivers/cpuidle/governors/teo.c =================================================================== --- linux-pm.orig/drivers/cpuidle/governors/teo.c +++ linux-pm/drivers/cpuidle/governors/teo.c @@ -277,18 +277,35 @@ static int teo_select(struct cpuidle_dri hits = cpu_data->states[i].hits; misses = cpu_data->states[i].misses; + if (early_hits >= cpu_data->states[i].early_hits || + idx < 0) + continue; + + /* + * If the current candidate state has been the one with + * the maximum "early hits" metric so far, the "early + * hits" metric of the disabled state replaces the + * current "early hits" count to avoid selecting a + * deeper state with lower "early hits" metric. + */ + if (max_early_idx == idx) { + early_hits = cpu_data->states[i].early_hits; + continue; + } + /* - * If the "early hits" metric of a disabled state is - * greater than the current maximum, it should be taken - * into account, because it would be a mistake to select - * a deeper state with lower "early hits" metric. The - * index cannot be changed to point to it, however, so - * just increase the "early hits" count alone and let - * the index still point to a shallower idle state. + * The current candidate state is closer to the disabled + * one than the current maximum "early hits" state, so + * replace the latter with it, but in case the maximum + * "early hits" state index has not been set so far, + * check if the current candidate state is not too + * shallow for that role. */ - if (max_early_idx >= 0 && - early_hits < cpu_data->states[i].early_hits) + if (!(tick_nohz_tick_stopped() && + drv->states[idx].target_residency < TICK_USEC)) { early_hits = cpu_data->states[i].early_hits; + max_early_idx = idx; + } continue; }