On Mon, 2025-06-23 at 11:55 +0200, Benjamin Berg wrote:
> Hi Hans,
>
>
> On Fri, 2025-06-20 at 17:56 +0200, Hans de Goede wrote:
> > While testing 6.16-rc2+ on a Dell XPS 9640 I got the following
> > lockdep report:
> >
> > (Note this was a build without debuginfo, so I did not run
> > decode_stacktrace.sh)
> >
> > [ 19.690210] ======================================================
> > [ 19.690212] WARNING: possible circular locking dependency detected
> > [ 19.690214] 6.16.0-rc2+ #3 Tainted: G E
> > [ 19.690217] ------------------------------------------------------
> > [ 19.690218] modprobe/906 is trying to acquire lock:
> > [ 19.690220] ffff89da8e948768 (&rdev->wiphy.mtx){+.+.}-{4:4}, at:
> > iwl_mld_tzone_get_temp+0x2f/0x1d0 [iwlmld]
> > [ 19.690269]
> > but task is already holding lock:
> > [ 19.690270] ffff89da41ac2708 (&tz->lock){+.+.}-{4:4}, at:
> > thermal_zone_device_set_mode+0x20/0xa0
> > [ 19.690284]
> > which lock already depends on the new lock.
> > [SNIP]
>
> I am thinking that this is a false-positive and the
>
> lock(cpuhp_state-up);
> lock(&tz->lock);
>
> chain is happening on a different type of thermal device.
Agreed.
#3 and #4 in the log are from x86_pkg_temp_thermal driver. So they are
poking at the "tz->lock" of different thermal_zone_device structures.
thanks,
rui
> A solution
> might be to create a separate lock class for every thermal_zone_device.
> Something like the below, I suspect.
>
> Benjamin
>
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 17ca5c082643..fcb060898733 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1556,6 +1556,7 @@ thermal_zone_device_register_with_trips(const
> char *type,
> INIT_LIST_HEAD(&tz->trips_invalid);
> ida_init(&tz->ida);
> mutex_init(&tz->lock);
> + lockdep_set_class(&tz->lock, &tz->lock_class);
> init_completion(&tz->removal);
> init_completion(&tz->resume);
> id = ida_alloc(&thermal_tz_ida, GFP_KERNEL);
> diff --git a/drivers/thermal/thermal_core.h
> b/drivers/thermal/thermal_core.h
> index bdadd141aa24..9ad5c37620f3 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -141,6 +141,7 @@ struct thermal_zone_device {
> void *governor_data;
> struct ida ida;
> struct mutex lock;
> + struct lock_class_key lock_class;
> struct list_head node;
> struct delayed_work poll_queue;
> enum thermal_notify_event notify_event;
>
