Re: [PATCH v2] cpufreq: schedutil: Fix rate limit overflow

[Rafael J. Wysocki (Intel)]

On Thu, Aug 6, 2026 at 9:28 AM Hui Su <sh_def@xxxxxxx> wrote:
>
> rate_limit_us is an unsigned int, while NSEC_PER_USEC is defined as
> 1000L. On 32-bit systems, the multiplication is therefore performed
> using 32-bit unsigned arithmetic before the result is assigned to
> freq_update_delay_ns.
>
> For example, writing 4294968 to rate_limit_us wraps the delay from
> 4294968000 ns to 704 ns. This makes schedutil update far more often
> than configured.
>
> Add sugov_update_rate_limit_us() to widen rate_limit_us to s64 before
> converting it to nanoseconds. Use the helper when updating the tunable
> through sysfs and when starting the governor, so both paths perform the
> conversion without overflow.
>
> Fixes: 9bdcb44e391d ("cpufreq: schedutil: New governor based on scheduler utilization data")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hui Su <sh_def@xxxxxxx>
> Reviewed-by: Zhongqiu Han <zhongqiu.han@xxxxxxxxxxxxxxxx>
> ---
> Changes in v2:
> - Clarify why the multiplication uses 32-bit unsigned arithmetic on
>   32-bit systems.
> - Cast rate_limit_us to s64 to match freq_update_delay_ns.
> - Add Zhongqiu's Reviewed-by tag.
>
> v1: https://lore.kernel.org/r/20260805143942.805176-1-sh_def@xxxxxxx
>
>  kernel/sched/cpufreq_schedutil.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index dff4ee04694c..51910b30c8da 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -61,6 +61,12 @@ static DEFINE_PER_CPU(struct sugov_cpu, sugov_cpu);
>
>  /************************ Governor internals ***********************/
>
> +static void sugov_update_rate_limit_us(struct sugov_policy *sg_policy)
> +{

Please add a comment here to explain why the direct type cast is
necessary or people will be sending patches to remove it.

> +       sg_policy->freq_update_delay_ns =
> +               (s64)sg_policy->tunables->rate_limit_us * NSEC_PER_USEC;
> +}
> +
>  static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time)
>  {
>         s64 delta_ns;
> @@ -606,7 +612,7 @@ rate_limit_us_store(struct gov_attr_set *attr_set, const char *buf, size_t count
>         tunables->rate_limit_us = rate_limit_us;
>
>         list_for_each_entry(sg_policy, &attr_set->policy_list, tunables_hook)
> -               sg_policy->freq_update_delay_ns = rate_limit_us * NSEC_PER_USEC;
> +               sugov_update_rate_limit_us(sg_policy);
>
>         return count;
>  }
> @@ -848,7 +854,7 @@ static int sugov_start(struct cpufreq_policy *policy)
>         void (*uu)(struct update_util_data *data, u64 time, unsigned int flags);
>         unsigned int cpu;
>
> -       sg_policy->freq_update_delay_ns = sg_policy->tunables->rate_limit_us * NSEC_PER_USEC;
> +       sugov_update_rate_limit_us(sg_policy);
>         sg_policy->last_freq_update_time        = 0;
>         sg_policy->next_freq                    = 0;
>         sg_policy->work_in_progress             = false;
> --
> 2.43.0
>