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: <CAJZ5v0j_fFk=EX0Z9_w1twQH-FpntHJvr4d0WSMBM6PevfEqNg@mail.gmail.com>
Date: Sat, 10 May 2025 13:31:15 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Marek Szyprowski <m.szyprowski@...sung.com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>, Linux PM <linux-pm@...r.kernel.org>, 
	LKML <linux-kernel@...r.kernel.org>, Lukasz Luba <lukasz.luba@....com>, 
	Peter Zijlstra <peterz@...radead.org>, 
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, 
	Dietmar Eggemann <dietmar.eggemann@....com>, Morten Rasmussen <morten.rasmussen@....com>, 
	Vincent Guittot <vincent.guittot@...aro.org>, 
	Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>, 
	Pierre Gondois <pierre.gondois@....com>, Christian Loehle <christian.loehle@....com>
Subject: Re: [PATCH v2 2/7] cpufreq/sched: Move cpufreq-specific EAS checks to cpufreq

On Sat, May 10, 2025 at 1:49 AM Marek Szyprowski
<m.szyprowski@...sung.com> wrote:
>
> On 06.05.2025 22:37, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > Doing cpufreq-specific EAS checks that require accessing policy
> > internals directly from sched_is_eas_possible() is a bit unfortunate,
> > so introduce cpufreq_ready_for_eas() in cpufreq, move those checks
> > into that new function and make sched_is_eas_possible() call it.
> >
> > While at it, address a possible race between the EAS governor check
> > and governor change by doing the former under the policy rwsem.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > Reviewed-by: Christian Loehle <christian.loehle@....com>
> > Tested-by: Christian Loehle <christian.loehle@....com>
> > Reviewed-by: Dietmar Eggemann <dietmar.eggemann@....com>
>
> In my tests I've noticed that this patch, merged as commit 4854649b1fb4
> ("cpufreq/sched: Move cpufreq-specific EAS checks to cpufreq"), causes a
> regression on ARM64 Amlogic Meson SoC based OdroidN2 board. The board
> finally lockups. Reverting $subject on top of next-20250509 fixes this
> issue. Here is the lockdep warning observed before the lockup:

Thanks for the report!

> ======================================================
> WARNING: possible circular locking dependency detected
> 6.15.0-rc5-next-20250509-dirty #10335 Tainted: G         C
> cpufreq: cpufreq_policy_online: CPU2: Running at unlisted initial
> frequency: 999999 kHz, changing to: 1000000 kHz
> ------------------------------------------------------
> kworker/3:1/79 is trying to acquire lock:
> ffff00000494b380 (&policy->rwsem){++++}-{4:4}, at:
> cpufreq_ready_for_eas+0x60/0xbc
>
> but task is already holding lock:
> ffff8000832887a0 (sched_domains_mutex){+.+.}-{4:4}, at:
> partition_sched_domains+0x54/0x938
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #2 (sched_domains_mutex){+.+.}-{4:4}:
>         __mutex_lock+0xa8/0x598
>         mutex_lock_nested+0x24/0x30
>         partition_sched_domains+0x54/0x938
>         rebuild_sched_domains_locked+0x2d4/0x900
>         rebuild_sched_domains+0x2c/0x48
>         rebuild_sched_domains_energy+0x3c/0x58
>         rebuild_sd_workfn+0x10/0x1c
>         process_one_work+0x208/0x604
>         worker_thread+0x244/0x388
>         kthread+0x150/0x228
>         ret_from_fork+0x10/0x20
>
> -> #1 (cpuset_mutex){+.+.}-{4:4}:
>         __mutex_lock+0xa8/0x598
>         mutex_lock_nested+0x24/0x30
>         cpuset_lock+0x1c/0x28
>         __sched_setscheduler+0x31c/0x830
>         sched_setattr_nocheck+0x18/0x24
>         sugov_init+0x1b4/0x388
>         cpufreq_init_governor.part.0+0x58/0xd4
>         cpufreq_set_policy+0x2c8/0x3ec
>         cpufreq_online+0x520/0xb20
>         cpufreq_add_dev+0x80/0x98
>         subsys_interface_register+0xfc/0x118
>         cpufreq_register_driver+0x150/0x238
>         dt_cpufreq_probe+0x148/0x488
>         platform_probe+0x68/0xdc
>         really_probe+0xbc/0x298
>         __driver_probe_device+0x78/0x12c
>         driver_probe_device+0xdc/0x164
>         __device_attach_driver+0xb8/0x138
>         bus_for_each_drv+0x80/0xdc
>         __device_attach+0xa8/0x1b0
>         device_initial_probe+0x14/0x20
>         bus_probe_device+0xb0/0xb4
>         deferred_probe_work_func+0x8c/0xc8
>         process_one_work+0x208/0x604
>         worker_thread+0x244/0x388
>         kthread+0x150/0x228
>         ret_from_fork+0x10/0x20
>
> -> #0 (&policy->rwsem){++++}-{4:4}:
>         __lock_acquire+0x1408/0x2254
>         lock_acquire+0x1c8/0x354
>         down_read+0x60/0x180
>         cpufreq_ready_for_eas+0x60/0xbc
>         sched_is_eas_possible+0x144/0x170
>         partition_sched_domains+0x504/0x938
>         rebuild_sched_domains_locked+0x2d4/0x900
>         rebuild_sched_domains+0x2c/0x48
>         rebuild_sched_domains_energy+0x3c/0x58
>         rebuild_sd_workfn+0x10/0x1c
>         process_one_work+0x208/0x604
>         worker_thread+0x244/0x388
>         kthread+0x150/0x228
>         ret_from_fork+0x10/0x20
>
> other info that might help us debug this:
>
> Chain exists of:
>    &policy->rwsem --> cpuset_mutex --> sched_domains_mutex
>
>   Possible unsafe locking scenario:
>
>         CPU0                    CPU1
>         ----                    ----
>    lock(sched_domains_mutex);
>                                 lock(cpuset_mutex);
>                                 lock(sched_domains_mutex);
>    rlock(&policy->rwsem);
>
>   *** DEADLOCK ***

Well, it turns out that trying to acquire policy->rwsem under
sched_domains_mutex is a bad idea.  It was added to
cpufreq_policy_is_good_for_eas() to address a theoretical race, so it
can be dropped safely.  A theoretical race is better than a real
deadlock.

Please test the attached patch.

View attachment "cpufreq-eas-policy-locking.patch" of type "text/x-patch" (264 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ