linux-kernel - Re: [PATCH v2 2/2] cpufreq: Pass the policy to cpufreq_driver->adjust

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6eabf604-9168-47fb-8fd8-b2f0d985db6a@amd.com>
Date: Fri, 16 Jan 2026 10:08:57 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Viresh Kumar <viresh.kumar@...aro.org>
CC: Huang Rui <ray.huang@....com>, "Gautham R. Shenoy"
	<gautham.shenoy@....com>, Mario Limonciello <mario.limonciello@....com>,
	"Rafael J. Wysocki" <rafael@...nel.org>, Srinivas Pandruvada
	<srinivas.pandruvada@...ux.intel.com>, Len Brown <lenb@...nel.org>, Sebastian
 Andrzej Siewior <bigeasy@...utronix.de>, Clark Williams
	<clrkwllms@...nel.org>, Bert Karwatzki <spasswolf@....de>,
	<linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-rt-devel@...ts.linux.dev>, Perry Yuan <perry.yuan@....com>
Subject: Re: [PATCH v2 2/2] cpufreq: Pass the policy to
 cpufreq_driver->adjust_perf()

Hello Viresh,

On 1/15/2026 9:44 AM, Viresh Kumar wrote:
> On 14-01-26, 08:51, K Prateek Nayak wrote:
>> cpufreq_cpu_get() can sleep on PREEMPT_RT in presence of concurrent
>> writer(s), however amd-pstate depends on fetching the cpudata via the
>> policy's driver data which necessitates grabbing the reference.
>>
>> Since schedutil governor can call "cpufreq_driver->update_perf()"
>> during sched_tick/enqueue/dequeue with rq_lock held and IRQs disabled,
>> fetching the policy object using the cpufreq_cpu_get() helper in the
>> scheduler fast-path leads to "BUG: scheduling while atomic" on
>> PREEMPT_RT [1].
>>
>> Pass the cached cpufreq policy object in sg_policy to the update_perf()
>> instead of just the CPU. The CPU can be inferred using "policy->cpu".
>>
>> The lifetime of cpufreq_policy object outlasts that of the governor and
>> the cpufreq driver (allocated when the CPU is onlined and only reclaimed
>> when the CPU is offlined / the CPU device is removed) which makes it
>> safe to be referenced throughout the governor's lifetime.
>>
>> Reported-by: Bert Karwatzki <spasswolf@....de>
>> Closes:https://lore.kernel.org/all/20250731092316.3191-1-spasswolf@web.de/ [1]
>> Acked-by: Viresh Kumar <viresh.kumar@...aro.org>
>> Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>
> 
> Ahh, you need to fix the Rust binding as well. Minor change there.

Guilty of not having CONFIG_RUST enabled! I've arrived at the
following after tasking a look at cpufreq.rs (specifically the
fast_switch_callback implementation):

  (Only build tested currently)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index f968fbd22890..bea109f8ff78 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -1255,18 +1255,17 @@ impl<T: Driver> Registration<T> {
     /// # Safety
     ///
     /// - This function may only be called from the cpufreq C infrastructure.
+    /// - The pointer arguments must be valid pointers.
     unsafe extern "C" fn adjust_perf_callback(
-        cpu: c_uint,
+        ptr: *mut bindings::cpufreq_policy,
         min_perf: c_ulong,
         target_perf: c_ulong,
         capacity: c_ulong,
     ) {
-        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
-        let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };
-
-        if let Ok(mut policy) = PolicyCpu::from_cpu(cpu_id) {
-            T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
-        }
+        // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+        // lifetime of `policy`.
+        let policy = unsafe { Policy::from_raw_mut(ptr) };
+        T::adjust_perf(policy, min_perf, target_perf, capacity);
     }
 
     /// Driver's `get_intermediate` callback.
---

Let me know what you think. If you think this is alright, I'll fold it
into this same patch in the next version to preserve bisectiblity with
CONFIG_RUST (or should I keep it separate?).

Since this is my first time dealing with the rust bindings, comments are
highly appreciated.

-- 
Thanks and Regards,
Prateek