linux-kernel - [PATCH] cpufreq: schedutil: Don't use the limits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210208030723.781-1-zbestahu@gmail.com>
Date:   Mon,  8 Feb 2021 11:07:23 +0800
From:   Yue Hu <zbestahu@...il.com>
To:     rjw@...ysocki.net, rafael.j.wysocki@...el.com,
        viresh.kumar@...aro.org, mingo@...hat.com, peterz@...radead.org,
        juri.lelli@...hat.com, vincent.guittot@...aro.org
Cc:     dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        mgorman@...e.de, bristot@...hat.com, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, huyue2@...ong.com, zbestahu@....com,
        zhangwen@...ong.com
Subject: [PATCH] cpufreq: schedutil: Don't use the limits_changed flag any more

From: Yue Hu <huyue2@...ong.com>

The limits_changed flag was introduced by commit 600f5badb78c
("cpufreq: schedutil: Don't skip freq update when limits change") due
to race condition where need_freq_update is cleared in get_next_freq()
which causes reducing the CPU frequency is ineffective while busy.

But now, the race condition above is gone because get_next_freq()
doesn't clear the flag any more after commit 23a881852f3e ("cpufreq:
schedutil: Don't skip freq update if need_freq_update is set").

Moreover, need_freq_update currently will be set to true only in
sugov_should_update_freq() if CPUFREQ_NEED_UPDATE_LIMITS is not set
for the driver. However, limits may have changed at any time. And
subsequent frequence update is depending on need_freq_update. So, we
may skip this update.

Hence, let's remove it to avoid above issue and make code more simple.

Signed-off-by: Yue Hu <huyue2@...ong.com>
---
 kernel/sched/cpufreq_schedutil.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 41e498b..7dd85fb 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -40,7 +40,6 @@ struct sugov_policy {
 	struct task_struct	*thread;
 	bool			work_in_progress;
 
-	bool			limits_changed;
 	bool			need_freq_update;
 };
 
@@ -89,11 +88,8 @@ static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time)
 	if (!cpufreq_this_cpu_can_update(sg_policy->policy))
 		return false;
 
-	if (unlikely(sg_policy->limits_changed)) {
-		sg_policy->limits_changed = false;
-		sg_policy->need_freq_update = true;
+	if (unlikely(sg_policy->need_freq_update))
 		return true;
-	}
 
 	delta_ns = time - sg_policy->last_freq_update_time;
 
@@ -323,7 +319,7 @@ static bool sugov_cpu_is_busy(struct sugov_cpu *sg_cpu)
 static inline void ignore_dl_rate_limit(struct sugov_cpu *sg_cpu, struct sugov_policy *sg_policy)
 {
 	if (cpu_bw_dl(cpu_rq(sg_cpu->cpu)) > sg_cpu->bw_dl)
-		sg_policy->limits_changed = true;
+		sg_policy->need_freq_update = true;
 }
 
 static inline bool sugov_update_single_common(struct sugov_cpu *sg_cpu,
@@ -759,7 +755,6 @@ static int sugov_start(struct cpufreq_policy *policy)
 	sg_policy->last_freq_update_time	= 0;
 	sg_policy->next_freq			= 0;
 	sg_policy->work_in_progress		= false;
-	sg_policy->limits_changed		= false;
 	sg_policy->cached_raw_freq		= 0;
 
 	sg_policy->need_freq_update = cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS);
@@ -813,7 +808,7 @@ static void sugov_limits(struct cpufreq_policy *policy)
 		mutex_unlock(&sg_policy->work_lock);
 	}
 
-	sg_policy->limits_changed = true;
+	sg_policy->need_freq_update = true;
 }
 
 struct cpufreq_governor schedutil_gov = {
-- 
1.9.1