[PATCH] iio: core: add separate lockdep class for info_exist_lock

[PATCH] iio: core: add separate lockdep class for info_exist_lock

[Rasmus Villemoes]

When one iio device is a consumer of another, it is possible that
the ->info_exist_lock of both ends up being taken when reading the
value of the consumer device.

Since they currently belong to the same lockdep class (being
initialized in a single location with mutex_init()), that results in a
lockdep warning

         CPU0
         ----
    lock(&iio_dev_opaque->info_exist_lock);
    lock(&iio_dev_opaque->info_exist_lock);

   *** DEADLOCK ***

   May be due to missing lock nesting notation

  4 locks held by sensors/414:
   #0: c31fd6dc (&p->lock){+.+.}-{3:3}, at: seq_read_iter+0x44/0x4e4
   #1: c4f5a1c4 (&of->mutex){+.+.}-{3:3}, at: kernfs_seq_start+0x1c/0xac
   #2: c2827548 (kn->active#34){.+.+}-{0:0}, at: kernfs_seq_start+0x30/0xac
   #3: c1dd2b68 (&iio_dev_opaque->info_exist_lock){+.+.}-{3:3}, at: iio_read_channel_processed_scale+0x24/0xd8

  stack backtrace:
  CPU: 0 UID: 0 PID: 414 Comm: sensors Not tainted 6.17.11 #5 NONE
  Hardware name: Generic AM33XX (Flattened Device Tree)
  Call trace:
   unwind_backtrace from show_stack+0x10/0x14
   show_stack from dump_stack_lvl+0x44/0x60
   dump_stack_lvl from print_deadlock_bug+0x2b8/0x334
   print_deadlock_bug from __lock_acquire+0x13a4/0x2ab0
   __lock_acquire from lock_acquire+0xd0/0x2c0
   lock_acquire from __mutex_lock+0xa0/0xe8c
   __mutex_lock from mutex_lock_nested+0x1c/0x24
   mutex_lock_nested from iio_read_channel_raw+0x20/0x6c
   iio_read_channel_raw from rescale_read_raw+0x128/0x1c4
   rescale_read_raw from iio_channel_read+0xe4/0xf4
   iio_channel_read from iio_read_channel_processed_scale+0x6c/0xd8
   iio_read_channel_processed_scale from iio_hwmon_read_val+0x68/0xbc
   iio_hwmon_read_val from dev_attr_show+0x18/0x48
   dev_attr_show from sysfs_kf_seq_show+0x80/0x110
   sysfs_kf_seq_show from seq_read_iter+0xdc/0x4e4
   seq_read_iter from vfs_read+0x238/0x2e4
   vfs_read from ksys_read+0x6c/0xec
   ksys_read from ret_fast_syscall+0x0/0x1c

Just as the mlock_key already has its own lockdep class, add a
lock_class_key for the info_exist mutex.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 drivers/iio/industrialio-core.c | 4 +++-
 include/linux/iio/iio-opaque.h  | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f69deefcfb6f..117ffad4f376 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1657,6 +1657,7 @@ static void iio_dev_release(struct device *device)
 	mutex_destroy(&iio_dev_opaque->info_exist_lock);
 	mutex_destroy(&iio_dev_opaque->mlock);
 
+	lockdep_unregister_key(&iio_dev_opaque->info_exist_key);
 	lockdep_unregister_key(&iio_dev_opaque->mlock_key);
 
 	ida_free(&iio_ida, iio_dev_opaque->id);
@@ -1717,9 +1718,10 @@ struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv)
 	INIT_LIST_HEAD(&iio_dev_opaque->ioctl_handlers);
 
 	lockdep_register_key(&iio_dev_opaque->mlock_key);
+	lockdep_register_key(&iio_dev_opaque->info_exist_key);
 
 	mutex_init_with_key(&iio_dev_opaque->mlock, &iio_dev_opaque->mlock_key);
-	mutex_init(&iio_dev_opaque->info_exist_lock);
+	mutex_init_with_key(&iio_dev_opaque->info_exist_lock, &iio_dev_opaque->info_exist_key);
 
 	indio_dev->dev.parent = parent;
 	indio_dev->dev.type = &iio_device_type;
diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio-opaque.h
index 4247497f3f8b..b87841a355f8 100644
--- a/include/linux/iio/iio-opaque.h
+++ b/include/linux/iio/iio-opaque.h
@@ -14,6 +14,7 @@
  * @mlock:			lock used to prevent simultaneous device state changes
  * @mlock_key:			lockdep class for iio_dev lock
  * @info_exist_lock:		lock to prevent use during removal
+ * @info_exist_key:		lockdep class for info_exist lock
  * @trig_readonly:		mark the current trigger immutable
  * @event_interface:		event chrdevs associated with interrupt lines
  * @attached_buffers:		array of buffers statically attached by the driver
@@ -47,6 +48,7 @@ struct iio_dev_opaque {
 	struct mutex			mlock;
 	struct lock_class_key		mlock_key;
 	struct mutex			info_exist_lock;
+	struct lock_class_key		info_exist_key;
 	bool				trig_readonly;
 	struct iio_event_interface	*event_interface;
 	struct iio_buffer		**attached_buffers;
-- 
2.52.0

Re: [PATCH] iio: core: add separate lockdep class for info_exist_lock

[Peter Rosin]

Hi!

2025-12-15 at 14:17, Rasmus Villemoes wrote:
> When one iio device is a consumer of another, it is possible that
> the ->info_exist_lock of both ends up being taken when reading the
> value of the consumer device.
> 
> Since they currently belong to the same lockdep class (being
> initialized in a single location with mutex_init()), that results in a
> lockdep warning

...

> Just as the mlock_key already has its own lockdep class, add a
> lock_class_key for the info_exist mutex.
> 
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

Looks sane from here.

Reviewed-by: Peter Rosin <peda@axentia.se>

Cheers,
Peter

Re: [PATCH] iio: core: add separate lockdep class for info_exist_lock

[Jonathan Cameron]

On Mon, 15 Dec 2025 14:33:38 +0100
Peter Rosin <peda@axentia.se> wrote:

> Hi!
> 
> 2025-12-15 at 14:17, Rasmus Villemoes wrote:
> > When one iio device is a consumer of another, it is possible that
> > the ->info_exist_lock of both ends up being taken when reading the
> > value of the consumer device.
> > 
> > Since they currently belong to the same lockdep class (being
> > initialized in a single location with mutex_init()), that results in a
> > lockdep warning  
> 
> ...
> 
> > Just as the mlock_key already has its own lockdep class, add a
> > lock_class_key for the info_exist mutex.
> > 
> > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>  
> 
> Looks sane from here.
> 
> Reviewed-by: Peter Rosin <peda@axentia.se>
Hi Rasmus,

Thanks for doing this!
> 
We should probably merge this as a fix and get it backported.
Whilst fairly rare anyone hits this it is also safe enough wrt
to very low chance of causing any problems.

Would be good to have an appropriate Fixes tag though.
Ideally please reply to this thread with an appropriate one.
If not I'll try and figure one out, but not today!

Jonathan

> Cheers,
> Peter

Re: [PATCH] iio: core: add separate lockdep class for info_exist_lock

[Rasmus Villemoes]

On Sun, Dec 21 2025, Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 15 Dec 2025 14:33:38 +0100
> Peter Rosin <peda@axentia.se> wrote:
>
>> Hi!
>> 
>> 2025-12-15 at 14:17, Rasmus Villemoes wrote:
>> > When one iio device is a consumer of another, it is possible that
>> > the ->info_exist_lock of both ends up being taken when reading the
>> > value of the consumer device.
>> > 
>> > Since they currently belong to the same lockdep class (being
>> > initialized in a single location with mutex_init()), that results in a
>> > lockdep warning  
>> 
>> ...
>> 
>> > Just as the mlock_key already has its own lockdep class, add a
>> > lock_class_key for the info_exist mutex.
>> > 
>> > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>  
>> 
>> Looks sane from here.
>> 
>> Reviewed-by: Peter Rosin <peda@axentia.se>
> Hi Rasmus,
>
> Thanks for doing this!
>> 
> We should probably merge this as a fix and get it backported.
> Whilst fairly rare anyone hits this it is also safe enough wrt
> to very low chance of causing any problems.
>
> Would be good to have an appropriate Fixes tag though.
> Ideally please reply to this thread with an appropriate one.
> If not I'll try and figure one out, but not today!

I tried to find one, but the problem goes way back, probably all the way
to either the introduction of info_exist_lock or the ability for one iio
channel to have a dependency on another, whichever came latest. And I'm
not really very familiar with the iio subsystem, so I couldn't find one
single commit to name.

Commit 2bc9cd66eb25d ("iio: Use per-device lockdep class for mlock")
which introduced the mlock_key referred to 67e17300dc1d76 ("iio:
potentiostat: add LMP91000 support"), but that feels more like a "this
driver is what first exposed the problem" and not really "this is where
the problem was introduced in the iio framework".

Wrt. backporting, it's probably worth mentioning c76ba4b264442 ("iio:
core: Replace lockdep_set_class() + mutex_init() by combined call") as a
prerequisite, as that is needed to make it cherry-pick cleanly.

Rasmus

Re: [PATCH] iio: core: add separate lockdep class for info_exist_lock

[Jonathan Cameron]

On Mon, 22 Dec 2025 09:52:42 +0100
Rasmus Villemoes <ravi@prevas.dk> wrote:

> On Sun, Dec 21 2025, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Mon, 15 Dec 2025 14:33:38 +0100
> > Peter Rosin <peda@axentia.se> wrote:
> >  
> >> Hi!
> >> 
> >> 2025-12-15 at 14:17, Rasmus Villemoes wrote:  
> >> > When one iio device is a consumer of another, it is possible that
> >> > the ->info_exist_lock of both ends up being taken when reading the
> >> > value of the consumer device.
> >> > 
> >> > Since they currently belong to the same lockdep class (being
> >> > initialized in a single location with mutex_init()), that results in a
> >> > lockdep warning    
> >> 
> >> ...
> >>   
> >> > Just as the mlock_key already has its own lockdep class, add a
> >> > lock_class_key for the info_exist mutex.
> >> > 
> >> > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>    
> >> 
> >> Looks sane from here.
> >> 
> >> Reviewed-by: Peter Rosin <peda@axentia.se>  
> > Hi Rasmus,
> >
> > Thanks for doing this!  
> >>   
> > We should probably merge this as a fix and get it backported.
> > Whilst fairly rare anyone hits this it is also safe enough wrt
> > to very low chance of causing any problems.
> >
> > Would be good to have an appropriate Fixes tag though.
> > Ideally please reply to this thread with an appropriate one.
> > If not I'll try and figure one out, but not today!  
> 
> I tried to find one, but the problem goes way back, probably all the way
> to either the introduction of info_exist_lock or the ability for one iio
> channel to have a dependency on another, whichever came latest. And I'm
> not really very familiar with the iio subsystem, so I couldn't find one
> single commit to name.
> 
> Commit 2bc9cd66eb25d ("iio: Use per-device lockdep class for mlock")
> which introduced the mlock_key referred to 67e17300dc1d76 ("iio:
> potentiostat: add LMP91000 support"), but that feels more like a "this
> driver is what first exposed the problem" and not really "this is where
> the problem was introduced in the iio framework".
> 
> Wrt. backporting, it's probably worth mentioning c76ba4b264442 ("iio:
> core: Replace lockdep_set_class() + mutex_init() by combined call") as a
> prerequisite, as that is needed to make it cherry-pick cleanly.

Thanks for doing the archeology.  I had a dig as well and fairly sure it's.
That references the rules for consumers in the commit message so definitely
seems that all the relevant infrastructure was in there at that point.
Given it's from 2012 we can backport this to all kernels anyone still cares
about.

 ac917a81117c ("staging:iio:core set the iio_dev.info pointer to null on unregister under lock.")

So applied with that fixes tag to the fixes-togreg branch of iio.git.

thanks,

Jonathan


> Rasmus