diff mbox series

[v2,3/6] cpufreq/sched: Set need_freq_update in ignore_dl_rate_limit()

Message ID 10666429.nUPlyArG6x@rjwysocki.net (mailing list archive)
State In Next
Delegated to: Rafael Wysocki
Headers show
Series cpufreq/sched: Improve synchronization of policy limits updates with schedutil | expand

Commit Message

Rafael J. Wysocki April 15, 2025, 10 a.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Notice that ignore_dl_rate_limit() need not piggy back on the
limits_changed handling to achieve its goal (which is to enforce a
frequency update before its due time).

Namely, if sugov_should_update_freq() is updated to check
sg_policy->need_freq_update and return 'true' if it is set when
sg_policy->limits_changed is not set, ignore_dl_rate_limit() may
set the former directly instead of setting the latter, so it can
avoid hitting the memory barrier in sugov_should_update_freq().

Update the code accordingly.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/sched/cpufreq_schedutil.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Christian Loehle April 16, 2025, 12:26 p.m. UTC | #1 
On 4/15/25 11:00, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Notice that ignore_dl_rate_limit() need not piggy back on the
> limits_changed handling to achieve its goal (which is to enforce a
> frequency update before its due time).
> 
> Namely, if sugov_should_update_freq() is updated to check
> sg_policy->need_freq_update and return 'true' if it is set when
> sg_policy->limits_changed is not set, ignore_dl_rate_limit() may
> set the former directly instead of setting the latter, so it can
> avoid hitting the memory barrier in sugov_should_update_freq().
> 
> Update the code accordingly.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Much cleaner now, thanks!
Reviewed-by: Christian Loehle <christian.loehle@arm.com>

> ---
>  kernel/sched/cpufreq_schedutil.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -96,6 +96,9 @@
>  		smp_mb();
>  
>  		return true;
> +	} else if (sg_policy->need_freq_update) {
> +		/* ignore_dl_rate_limit() wants a new frequency to be found. */
> +		return true;
>  	}
>  
>  	delta_ns = time - sg_policy->last_freq_update_time;
> @@ -388,7 +391,7 @@
>  static inline void ignore_dl_rate_limit(struct sugov_cpu *sg_cpu)
>  {
>  	if (cpu_bw_dl(cpu_rq(sg_cpu->cpu)) > sg_cpu->bw_min)
> -		WRITE_ONCE(sg_cpu->sg_policy->limits_changed, true);
> +		sg_cpu->sg_policy->need_freq_update = true;
>  }
>  
>  static inline bool sugov_update_single_common(struct sugov_cpu *sg_cpu,
> 
> 
>
diff mbox series

Patch

--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -96,6 +96,9 @@ 
 		smp_mb();
 
 		return true;
+	} else if (sg_policy->need_freq_update) {
+		/* ignore_dl_rate_limit() wants a new frequency to be found. */
+		return true;
 	}
 
 	delta_ns = time - sg_policy->last_freq_update_time;
@@ -388,7 +391,7 @@ 
 static inline void ignore_dl_rate_limit(struct sugov_cpu *sg_cpu)
 {
 	if (cpu_bw_dl(cpu_rq(sg_cpu->cpu)) > sg_cpu->bw_min)
-		WRITE_ONCE(sg_cpu->sg_policy->limits_changed, true);
+		sg_cpu->sg_policy->need_freq_update = true;
 }
 
 static inline bool sugov_update_single_common(struct sugov_cpu *sg_cpu,