From patchwork Mon Apr 14 20:45:50 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 881171 Received: from cloudserver094114.home.pl (cloudserver094114.home.pl [79.96.170.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 16BB21EFFA2; Mon, 14 Apr 2025 20:52:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=79.96.170.134 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744663962; cv=none; b=JUdDn2UL3PgaXYojnTeI+WxcWiCn1MBwKy8s1hcpkGOn0PMvcRlCuHWXAQE9YL4HUfwmAvhqgr7Mjbli1EybV6GmCKHGnYSGOPBZ3iuyllNnmuestFhO7e8OgwEqj5OyXGDensbt5tc6Dd6EcoFJ+yNKb12P2m7clClkDEOU8z0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744663962; c=relaxed/simple; bh=eaMUlr2OVoDVhrTB1FmD26RARmqHIb0rHt9dweslJaw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=eJE1avKcneJFleNpr40PthXkeqLFtCrGt/WaFxu/25RScT8fz9uyHFR/I0Hoe0Anj3gTG14q4n/fNzQdLqFBW+1XPpjUBU/25hRQIk8xq14TU3eE3z0aoe5MWXt800cS+LhDYfvkc8vOW4Bv7MeJjcLppedBrhsHP+n7kg88hkQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rjwysocki.net; spf=pass smtp.mailfrom=rjwysocki.net; dkim=pass (2048-bit key) header.d=rjwysocki.net header.i=@rjwysocki.net header.b=Zq2V7ATl; arc=none smtp.client-ip=79.96.170.134 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rjwysocki.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rjwysocki.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rjwysocki.net header.i=@rjwysocki.net header.b="Zq2V7ATl" Received: from kreacher.localnet (unknown [195.136.19.94]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by cloudserver094114.home.pl (Postfix) with ESMTPSA id 3BD476625D3; Mon, 14 Apr 2025 22:52:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=rjwysocki.net; s=dkim; t=1744663954; bh=eaMUlr2OVoDVhrTB1FmD26RARmqHIb0rHt9dweslJaw=; h=From:Subject:Date; b=Zq2V7ATlF23mAFADjiYzJC/PqXstmuzzpv7cTnJ4xYAAhmVFjn6wN1Yd2+Odx8u8k iQxxkMVpyhu2P2bRucZbOWqqhEO4L4dZXTEQek6N82kPBFU4aWsxwJgRnd0pzIyP5y kNjMey1BE4nnm2ALRFqgZi1d5ko1BmT5JvQsKj98d94beMtNFZZJFe1hiF95piAYf4 so/ijEq5EHxRaN6/MdjN7SW2sIbTo7BDuwyiXdjHUmwqYWy4ky7SBXnI008GRfOeuj deX0jl9HJ1orJC5cWjqiDxv9hoAx4lZ6EJd+Xe2vAtc4vHkLMq/JmaWp7rf7Cmy3p6 JCkEB/jPByJIA== From: "Rafael J. Wysocki" To: Linux PM Cc: LKML , Viresh Kumar , Srinivas Pandruvada , Mario Limonciello , Vincent Guittot , Christian Loehle , Sultan Alsawaf , Peter Zijlstra , Valentin Schneider , Ingo Molnar Subject: [PATCH v1 2/5] cpufreq/sched: Explicitly synchronize limits_changed flag handling Date: Mon, 14 Apr 2025 22:45:50 +0200 Message-ID: <7811331.EvYhyI6sBW@rjwysocki.net> In-Reply-To: <3364921.aeNJFYEL58@rjwysocki.net> References: <3364921.aeNJFYEL58@rjwysocki.net> Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-CLIENT-IP: 195.136.19.94 X-CLIENT-HOSTNAME: 195.136.19.94 X-VADE-SPAMSTATE: clean X-VADE-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdduheeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecujffqoffgrffnpdggtffipffknecuuegrihhlohhuthemucduhedtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefhvfevufffkfgjfhgggfgtsehtufertddttdejnecuhfhrohhmpedftfgrfhgrvghlucflrdcuhgihshhotghkihdfuceorhhjfiesrhhjfiihshhotghkihdrnhgvtheqnecuggftrfgrthhtvghrnhepvdffueeitdfgvddtudegueejtdffteetgeefkeffvdeftddttdeuhfegfedvjefhnecukfhppeduleehrddufeeirdduledrleegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepudelhedrudefiedrudelrdelgedphhgvlhhopehkrhgvrggthhgvrhdrlhhotggrlhhnvghtpdhmrghilhhfrhhomheprhhjfiesrhhjfiihshhotghkihdrnhgvthdpnhgspghrtghpthhtohepuddupdhrtghpthhtoheplhhinhhugidqphhmsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtoheplhhinhhugidqkhgvrhhnvghlsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepvhhirhgvshhhrdhkuhhmrghrsehlihhnrghrohdrohhrghdprhgtphhtthhopehsrhhinhhivhgrshdrphgrnhgurhhuvhgruggrsehlihhnuhigrdhinhhtvghlrdgtohhmpdhrtghpthhtohepmhgrrhhiohdrlhh X-DCC--Metrics: v370.home.net.pl 1024; Body=11 Fuz1=11 Fuz2=11 From: Rafael J. Wysocki The handling of the limits_changed flag in struct sugov_policy needs to be explicitly synchronized to ensure that cpufreq policy limits updates will not be missed in some cases. Without that synchronization it is theoretically possible that the limits_changed update in sugov_should_update_freq() will be reordered with respect to the reads of the policy limits in cpufreq_driver_resolve_freq() and in that case, if the limits_changed update in sugov_limits() clobbers the one in sugov_should_update_freq(), the new policy limits may not take effect for a long time. Likewise, the limits_changed update in sugov_limits() may theoretically get reordered with respect to the updates of the policy limits in cpufreq_set_policy() and if sugov_should_update_freq() runs between them, the policy limits change may be missed. To ensure that the above situations will not take place, add memory barriers preventing the reordering in question from taking place and add READ_ONCE() and WRITE_ONCE() annotations around all of the limits_changed flag updates to prevent the compiler from messing up with that code. Fixes: 600f5badb78c ("cpufreq: schedutil: Don't skip freq update when limits change") Cc: 5.3+ # 5.3+ Signed-off-by: Rafael J. Wysocki --- kernel/sched/cpufreq_schedutil.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -81,11 +81,22 @@ if (!cpufreq_this_cpu_can_update(sg_policy->policy)) return false; - if (unlikely(sg_policy->limits_changed)) { - sg_policy->limits_changed = false; + if (unlikely(READ_ONCE(sg_policy->limits_changed))) { + WRITE_ONCE(sg_policy->limits_changed, false); sg_policy->need_freq_update = sg_policy->policy->fast_switch_enabled || cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS); + + /* + * The above limits_changed update must occur before the reads + * of policy limits in cpufreq_driver_resolve_freq() or a policy + * limits update might be missed, so use a memory barrier to + * ensure it. + * + * This pairs with the write memory barrier in sugov_limits(). + */ + smp_mb(); + return true; } @@ -367,7 +378,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) - sg_cpu->sg_policy->limits_changed = true; + WRITE_ONCE(sg_cpu->sg_policy->limits_changed, true); } static inline bool sugov_update_single_common(struct sugov_cpu *sg_cpu, @@ -873,7 +884,16 @@ mutex_unlock(&sg_policy->work_lock); } - sg_policy->limits_changed = true; + /* + * The limits_changed update below must take place before the updates + * of policy limits in cpufreq_set_policy() or a policy limits update + * might be missed, so use a memory barrier to ensure it. + * + * This pairs with the memory barrier in sugov_should_update_freq(). + */ + smp_wmb(); + + WRITE_ONCE(sg_policy->limits_changed, true); } struct cpufreq_governor schedutil_gov = {