Re: [PATCH v1] cpuidle: menu: Update documentation after previous changes

[Christian Loehle <christian.loehle@arm.com>]

On 2/20/25 20:13, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The documentaion of the menu cpuidle governor needs to be updated
s/documentaion/documentation/
> to match the code bevavior after some changes made recently.

s/bevavior/behavior/

> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  Documentation/admin-guide/pm/cpuidle.rst |   27 ++++++++++++++++-----------
>  drivers/cpuidle/governors/menu.c         |   29 ++++++++++-------------------
>  2 files changed, 26 insertions(+), 30 deletions(-)
> 
> --- a/Documentation/admin-guide/pm/cpuidle.rst
> +++ b/Documentation/admin-guide/pm/cpuidle.rst
> @@ -275,20 +275,25 @@
>  and variance of them.  If the variance is small (smaller than 400 square
>  milliseconds) or it is small relative to the average (the average is greater
>  that 6 times the standard deviation), the average is regarded as the "typical
> -interval" value.  Otherwise, the longest of the saved observed idle duration
> +interval" value.  Otherwise, either the longest or the shortest (depending on
> +which one is farther from the average) of the saved observed idle duration
>  values is discarded and the computation is repeated for the remaining ones.
> +
>  Again, if the variance of them is small (in the above sense), the average is
>  taken as the "typical interval" value and so on, until either the "typical
> -interval" is determined or too many data points are disregarded, in which case
> -the "typical interval" is assumed to equal "infinity" (the maximum unsigned
> -integer value).
> +interval" is determined or too many data points are disregarded.  In the latter
> +case, if the size of the set of data points still under consideration is
> +sufficiently large, the next idle duration is not likely to be above the largest
> +idle duration value still in that set, so that value is taken as the predicted
> +next idle duration.  Finally, if the set of data points still under
> +consideration is too small, no prediction is made.
>  
> -If the "typical interval" computed this way is long enough, the governor obtains
> -the time until the closest timer event with the assumption that the scheduler
> -tick will be stopped.  That time, referred to as the *sleep length* in what follows,
> -is the upper bound on the time before the next CPU wakeup.  It is used to determine
> -the sleep length range, which in turn is needed to get the sleep length correction
> -factor.
> +If the preliminary prediction of the next idle duration computed this way is
> +long enough, the governor obtains the time until the closest timer event with
> +the assumption that the scheduler tick will be stopped.  That time, referred to
> +as the *sleep length* in what follows, is the upper bound on the time before the
> +next CPU wakeup.  It is used to determine the sleep length range, which in turn
> +is needed to get the sleep length correction factor.
>  
>  The ``menu`` governor maintains an array containing several correction factor
>  values that correspond to different sleep length ranges organized so that each
> @@ -302,7 +307,7 @@
>  The sleep length is multiplied by the correction factor for the range that it
>  falls into to obtain an approximation of the predicted idle duration that is
>  compared to the "typical interval" determined previously and the minimum of
> -the two is taken as the idle duration prediction.
> +the two is taken as the final idle duration prediction.
>  
>  If the "typical interval" value is small, which means that the CPU is likely
>  to be woken up soon enough, the sleep length computation is skipped as it may
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -41,7 +41,7 @@
>   * the  C state is required to actually break even on this cost. CPUIDLE
>   * provides us this duration in the "target_residency" field. So all that we
>   * need is a good prediction of how long we'll be idle. Like the traditional
> - * menu governor, we start with the actual known "next timer event" time.
> + * menu governor, we take the actual known "next timer event" time.
>   *
>   * Since there are other source of wakeups (interrupts for example) than
>   * the next timer event, this estimation is rather optimistic. To get a
> @@ -50,30 +50,21 @@
>   * duration always was 50% of the next timer tick, the correction factor will
>   * be 0.5.
>   *
> - * menu uses a running average for this correction factor, however it uses a
> - * set of factors, not just a single factor. This stems from the realization
> - * that the ratio is dependent on the order of magnitude of the expected
> - * duration; if we expect 500 milliseconds of idle time the likelihood of
> - * getting an interrupt very early is much higher than if we expect 50 micro
> - * seconds of idle time. A second independent factor that has big impact on
> - * the actual factor is if there is (disk) IO outstanding or not.
> - * (as a special twist, we consider every sleep longer than 50 milliseconds
> - * as perfect; there are no power gains for sleeping longer than this)
> - *
> - * For these two reasons we keep an array of 12 independent factors, that gets
> - * indexed based on the magnitude of the expected duration as well as the
> - * "is IO outstanding" property.
> + * menu uses a running average for this correction factor, but it uses a set of
> + * factors, not just a single factor. This stems from the realization that the
> + * ratio is dependent on the order of magnitude of the expected duration; if we
> + * expect 500 milliseconds of idle time the likelihood of getting an interrupt
> + * very early is much higher than if we expect 50 micro seconds of idle time.
> + * For this reason, menu keeps an array of 6 independent factors, that gets
> + * indexed based on the magnitude of the expected duration.
>   *
>   * Repeatable-interval-detector
>   * ----------------------------
>   * There are some cases where "next timer" is a completely unusable predictor:
>   * Those cases where the interval is fixed, for example due to hardware
> - * interrupt mitigation, but also due to fixed transfer rate devices such as
> - * mice.
> + * interrupt mitigation, but also due to fixed transfer rate devices like mice.
>   * For this, we use a different predictor: We track the duration of the last 8
> - * intervals and if the stand deviation of these 8 intervals is below a
> - * threshold value, we use the average of these intervals as prediction.
> - *
> + * intervals and use them to estimate the duration of the next one.
>   */

Assuming you fix up the typos in the commit message when applying:
Reviewed-by: Christian Loehle <christian.loehle@arm.com>