Re: [PATCH v2] cpufreq: Fix hotplug-suspend race during reboot

Re: [PATCH v2] cpufreq: Fix hotplug-suspend race during reboot

[Tianxiang Chen]

On 4/14/2026 10:44 PM, Zhongqiu Han wrote:
> May I know did you test this with lockdep enabled? Specifically, does
> the new cpus_read_lock() -> policy->rwsem ordering in cpufreq_suspend()
> trigger any lockdep warnings? Thanks

Hi Zhongqiu,

Thanks for the review.

I did test v2 with lockdep enabled and was NOT able to reproduce any
warning. 

No circular-locking splat or "possible deadlock" report was observed
in dmesg across multiple runs.

My reasoning for why the new order should be safe:

  * The patch establishes  cpus_read_lock() -> policy->rwsem.
  * The hotplug path already holds cpu_hotplug_lock (write side,
    via the hotplug core) before taking policy->rwsem inside
    cpufreq_offline()/cpufreq_online(), i.e. the same direction.
  * I grep'd cpufreq and did not find an existing path that takes
    policy->rwsem first and then acquires cpus_read_lock()
    underneath. If I missed one, please point me at it.
  * cpus_read_lock() is a percpu-rwsem read side and is re-entrant,
    so even if an outer caller already holds it (e.g. via a pm
    notifier running inside a hotplug callback) this is safe.

May I ask whether you have actually observed a lockdep splat on this
change on any downstream tree, or is this a precautionary question?
If you have a specific call chain in mind, I would like to add
targeted coverage before v3 so we can nail it down definitively.

Thanks,
Tianxiang

Re: [PATCH v2] cpufreq: Fix hotplug-suspend race during reboot

[Zhongqiu Han]

On 5/11/2026 7:14 PM, Tianxiang Chen wrote:
> On 4/14/2026 10:44 PM, Zhongqiu Han wrote:
>> May I know did you test this with lockdep enabled? Specifically, does
>> the new cpus_read_lock() -> policy->rwsem ordering in cpufreq_suspend()
>> trigger any lockdep warnings? Thanks
> 
> Hi Zhongqiu,
> 
> Thanks for the review.
> 
> I did test v2 with lockdep enabled and was NOT able to reproduce any
> warning.
> 
> No circular-locking splat or "possible deadlock" report was observed
> in dmesg across multiple runs.


Thanks for the confirmation.

> 
> My reasoning for why the new order should be safe:
> 
>    * The patch establishes  cpus_read_lock() -> policy->rwsem.
>    * The hotplug path already holds cpu_hotplug_lock (write side,
>      via the hotplug core) before taking policy->rwsem inside
>      cpufreq_offline()/cpufreq_online(), i.e. the same direction.
>    * I grep'd cpufreq and did not find an existing path that takes
>      policy->rwsem first and then acquires cpus_read_lock()
>      underneath. If I missed one, please point me at it.
>    * cpus_read_lock() is a percpu-rwsem read side and is re-entrant,
>      so even if an outer caller already holds it (e.g. via a pm
>      notifier running inside a hotplug callback) this is safe.
> 
> May I ask whether you have actually observed a lockdep splat on this
> change on any downstream tree, or is this a precautionary question?
> If you have a specific call chain in mind, I would like to add
> targeted coverage before v3 so we can nail it down definitively.


This was mainly a precautionary question on my side, to make sure there
aren't any hidden locking concerns. In my experience, running new
locking changes under lockdep can be a useful sanity check, so I wanted
to double‑check.


For context, none of the existing cpufreq driver .suspend callbacks
currently invoke cpus_read_lock() so this does not affect any existing
suspend paths.


Looks okay to me.

Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>


> 
> Thanks,
> Tianxiang


-- 
Thx and BRs,
Zhongqiu Han