From: "Rafael J. Wysocki" rafael.j.wysocki@intel.com
[ Upstream commit fa3fa55de0d6177fdcaf6fc254f13cc8f33c3eed ]
Marc has reported that commit 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") caused the number of wakeup interrupts to increase on an idle system [1], which was not expected to happen after merely allowing shallower idle states to be selected by the governor in some cases.
However, on the system in question, all of the idle states deeper than WFI are rejected by the driver due to a firmware issue [2]. This causes the governor to only consider the recent interval duriation data corresponding to attempts to enter WFI that are successful and the recent invervals table is filled with values lower than the scheduler tick period. Consequently, the governor predicts an idle duration below the scheduler tick period length and avoids stopping the tick more often which leads to the observed symptom.
Address it by modifying the governor to update the recent intervals table also when entering the previously selected idle state fails, so it knows that the short idle intervals might have been the minority had the selected idle states been actually entered every time.
Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") Link: https://lore.kernel.org/linux-pm/86o6sv6n94.wl-maz@kernel.org/ [1] Link: https://lore.kernel.org/linux-pm/7ffcb716-9a1b-48c2-aaa4-469d0df7c792@arm.co... [2] Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Tested-by: Christian Loehle christian.loehle@arm.com Tested-by: Marc Zyngier maz@kernel.org Reviewed-by: Christian Loehle christian.loehle@arm.com Link: https://patch.msgid.link/2793874.mvXUDI8C0e@rafael.j.wysocki Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 7337a6356dffc93194af24ee31023b3578661a5b) --- drivers/cpuidle/governors/menu.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 4edac724983a..0b3c917d505d 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -158,6 +158,14 @@ static inline int performance_multiplier(unsigned int nr_iowaiters)
static DEFINE_PER_CPU(struct menu_device, menu_devices);
+static void menu_update_intervals(struct menu_device *data, unsigned int interval_us) +{ + /* Update the repeating-pattern data. */ + data->intervals[data->interval_ptr++] = interval_us; + if (data->interval_ptr >= INTERVALS) + data->interval_ptr = 0; +} + static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev);
/* @@ -288,6 +296,14 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, if (data->needs_update) { menu_update(drv, dev); data->needs_update = 0; + } else if (!dev->last_residency_ns) { + /* + * This happens when the driver rejects the previously selected + * idle state and returns an error, so update the recent + * intervals table to prevent invalid information from being + * used going forward. + */ + menu_update_intervals(data, UINT_MAX); }
/* determine the expected residency time, round up */ @@ -542,10 +558,7 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev)
data->correction_factor[data->bucket] = new_factor;
- /* update the repeating-pattern data */ - data->intervals[data->interval_ptr++] = ktime_to_us(measured_ns); - if (data->interval_ptr >= INTERVALS) - data->interval_ptr = 0; + menu_update_intervals(data, ktime_to_us(measured_ns)); }
/**
On 10/14/25 14:03, Sergey Senozhatsky wrote:
From: "Rafael J. Wysocki" rafael.j.wysocki@intel.com
[ Upstream commit fa3fa55de0d6177fdcaf6fc254f13cc8f33c3eed ]
Marc has reported that commit 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") caused the number of wakeup interrupts to increase on an idle system [1], which was not expected to happen after merely allowing shallower idle states to be selected by the governor in some cases.
However, on the system in question, all of the idle states deeper than WFI are rejected by the driver due to a firmware issue [2]. This causes the governor to only consider the recent interval duriation data corresponding to attempts to enter WFI that are successful and the recent invervals table is filled with values lower than the scheduler tick period. Consequently, the governor predicts an idle duration below the scheduler tick period length and avoids stopping the tick more often which leads to the observed symptom.
Address it by modifying the governor to update the recent intervals table also when entering the previously selected idle state fails, so it knows that the short idle intervals might have been the minority had the selected idle states been actually entered every time.
Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") Link: https://lore.kernel.org/linux-pm/86o6sv6n94.wl-maz@kernel.org/ [1] Link: https://lore.kernel.org/linux-pm/7ffcb716-9a1b-48c2-aaa4-469d0df7c792@arm.co... [2] Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Tested-by: Christian Loehle christian.loehle@arm.com Tested-by: Marc Zyngier maz@kernel.org Reviewed-by: Christian Loehle christian.loehle@arm.com Link: https://patch.msgid.link/2793874.mvXUDI8C0e@rafael.j.wysocki Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 7337a6356dffc93194af24ee31023b3578661a5b)
drivers/cpuidle/governors/menu.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 4edac724983a..0b3c917d505d 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -158,6 +158,14 @@ static inline int performance_multiplier(unsigned int nr_iowaiters) static DEFINE_PER_CPU(struct menu_device, menu_devices); +static void menu_update_intervals(struct menu_device *data, unsigned int interval_us) +{
- /* Update the repeating-pattern data. */
- data->intervals[data->interval_ptr++] = interval_us;
- if (data->interval_ptr >= INTERVALS)
data->interval_ptr = 0;
+}
static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev); /* @@ -288,6 +296,14 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, if (data->needs_update) { menu_update(drv, dev); data->needs_update = 0;
- } else if (!dev->last_residency_ns) {
/*
* This happens when the driver rejects the previously selected
* idle state and returns an error, so update the recent
* intervals table to prevent invalid information from being
* used going forward.
*/
}menu_update_intervals(data, UINT_MAX);
/* determine the expected residency time, round up */ @@ -542,10 +558,7 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev) data->correction_factor[data->bucket] = new_factor;
- /* update the repeating-pattern data */
- data->intervals[data->interval_ptr++] = ktime_to_us(measured_ns);
- if (data->interval_ptr >= INTERVALS)
data->interval_ptr = 0;
- menu_update_intervals(data, ktime_to_us(measured_ns));
} /**
For the backport: Reviewed-by: Christian Loehle christian.loehle@arm.com Thank you Sergey!
On Tue, Oct 14, 2025 at 10:03:00PM +0900, Sergey Senozhatsky wrote:
From: "Rafael J. Wysocki" rafael.j.wysocki@intel.com
[ Upstream commit fa3fa55de0d6177fdcaf6fc254f13cc8f33c3eed ]
Marc has reported that commit 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") caused the number of wakeup interrupts to increase on an idle system [1], which was not expected to happen after merely allowing shallower idle states to be selected by the governor in some cases.
However, on the system in question, all of the idle states deeper than WFI are rejected by the driver due to a firmware issue [2]. This causes the governor to only consider the recent interval duriation data corresponding to attempts to enter WFI that are successful and the recent invervals table is filled with values lower than the scheduler tick period. Consequently, the governor predicts an idle duration below the scheduler tick period length and avoids stopping the tick more often which leads to the observed symptom.
Address it by modifying the governor to update the recent intervals table also when entering the previously selected idle state fails, so it knows that the short idle intervals might have been the minority had the selected idle states been actually entered every time.
Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") Link: https://lore.kernel.org/linux-pm/86o6sv6n94.wl-maz@kernel.org/ [1] Link: https://lore.kernel.org/linux-pm/7ffcb716-9a1b-48c2-aaa4-469d0df7c792@arm.co... [2] Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Tested-by: Christian Loehle christian.loehle@arm.com Tested-by: Marc Zyngier maz@kernel.org Reviewed-by: Christian Loehle christian.loehle@arm.com Link: https://patch.msgid.link/2793874.mvXUDI8C0e@rafael.j.wysocki Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 7337a6356dffc93194af24ee31023b3578661a5b)
You forgot to sign off on this :(
On (25/10/16 10:55), Greg KH wrote:
On Tue, Oct 14, 2025 at 10:03:00PM +0900, Sergey Senozhatsky wrote:
From: "Rafael J. Wysocki" rafael.j.wysocki@intel.com
[ Upstream commit fa3fa55de0d6177fdcaf6fc254f13cc8f33c3eed ]
Marc has reported that commit 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") caused the number of wakeup interrupts to increase on an idle system [1], which was not expected to happen after merely allowing shallower idle states to be selected by the governor in some cases.
However, on the system in question, all of the idle states deeper than WFI are rejected by the driver due to a firmware issue [2]. This causes the governor to only consider the recent interval duriation data corresponding to attempts to enter WFI that are successful and the recent invervals table is filled with values lower than the scheduler tick period. Consequently, the governor predicts an idle duration below the scheduler tick period length and avoids stopping the tick more often which leads to the observed symptom.
Address it by modifying the governor to update the recent intervals table also when entering the previously selected idle state fails, so it knows that the short idle intervals might have been the minority had the selected idle states been actually entered every time.
Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") Link: https://lore.kernel.org/linux-pm/86o6sv6n94.wl-maz@kernel.org/ [1] Link: https://lore.kernel.org/linux-pm/7ffcb716-9a1b-48c2-aaa4-469d0df7c792@arm.co... [2] Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Tested-by: Christian Loehle christian.loehle@arm.com Tested-by: Marc Zyngier maz@kernel.org Reviewed-by: Christian Loehle christian.loehle@arm.com Link: https://patch.msgid.link/2793874.mvXUDI8C0e@rafael.j.wysocki Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 7337a6356dffc93194af24ee31023b3578661a5b)
You forgot to sign off on this :(
Oh, Greg, do you want me to resend or can you just add SoB?
On Thu, Oct 16, 2025 at 05:57:36PM +0900, Sergey Senozhatsky wrote:
On (25/10/16 10:55), Greg KH wrote:
On Tue, Oct 14, 2025 at 10:03:00PM +0900, Sergey Senozhatsky wrote:
From: "Rafael J. Wysocki" rafael.j.wysocki@intel.com
[ Upstream commit fa3fa55de0d6177fdcaf6fc254f13cc8f33c3eed ]
Marc has reported that commit 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") caused the number of wakeup interrupts to increase on an idle system [1], which was not expected to happen after merely allowing shallower idle states to be selected by the governor in some cases.
However, on the system in question, all of the idle states deeper than WFI are rejected by the driver due to a firmware issue [2]. This causes the governor to only consider the recent interval duriation data corresponding to attempts to enter WFI that are successful and the recent invervals table is filled with values lower than the scheduler tick period. Consequently, the governor predicts an idle duration below the scheduler tick period length and avoids stopping the tick more often which leads to the observed symptom.
Address it by modifying the governor to update the recent intervals table also when entering the previously selected idle state fails, so it knows that the short idle intervals might have been the minority had the selected idle states been actually entered every time.
Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information") Link: https://lore.kernel.org/linux-pm/86o6sv6n94.wl-maz@kernel.org/ [1] Link: https://lore.kernel.org/linux-pm/7ffcb716-9a1b-48c2-aaa4-469d0df7c792@arm.co... [2] Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Tested-by: Christian Loehle christian.loehle@arm.com Tested-by: Marc Zyngier maz@kernel.org Reviewed-by: Christian Loehle christian.loehle@arm.com Link: https://patch.msgid.link/2793874.mvXUDI8C0e@rafael.j.wysocki Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit 7337a6356dffc93194af24ee31023b3578661a5b)
You forgot to sign off on this :(
Oh, Greg, do you want me to resend or can you just add SoB?
No one can add a signed-off-by for someone else, please go read the document for exactly what that is attesting.
I've queued up a backport I did with a cc: to you on it already, that should be identical to yours, right?
thanks,
greg k-h
On Thu, Oct 16, 2025 at 06:08:39PM +0900, Sergey Senozhatsky wrote:
On (25/10/16 11:05), Greg KH wrote:
I've queued up a backport I did with a cc: to you on it already, that should be identical to yours, right?
Looks right. Thanks.
// I wonder why doesn't git cherry-pick -x add SoB automatically.
You need "-s" to add your signed-off-by, "-x" just adds the text at the bottom about the original commit id.
thanks,
greg k-h
On (25/10/16 11:12), Greg KH wrote:
On Thu, Oct 16, 2025 at 06:08:39PM +0900, Sergey Senozhatsky wrote:
On (25/10/16 11:05), Greg KH wrote:
I've queued up a backport I did with a cc: to you on it already, that should be identical to yours, right?
Looks right. Thanks.
// I wonder why doesn't git cherry-pick -x add SoB automatically.
You need "-s" to add your signed-off-by, "-x" just adds the text at the bottom about the original commit id.
Oh, today I learned. Thank you for educating me, next time I'll make sure to use "-s". And sorry for the missing signed-off-by in the backport.
linux-stable-mirror@lists.linaro.org