lockdep splat involving iio-hwmon and iio-rescale drivers

lockdep splat involving iio-hwmon and iio-rescale drivers

[Rasmus Villemoes]

Hi

I have an am335x board where the AINx channels are used for various
voltage/current measurement, i.e. they have a variety of current shunts
and voltage dividers in front of them. Describing those is pretty
straight-forward, and the resulting iio devices seem to work.

Now, I'd like to expose those as hwmon sensors, because there are other
i2c-attached ADCs on the board exposed that way, and it would be nice to
have them all available through "sensors" command in userspace. So
altogether, my .dts additions are

	ain {
		compatible = "iio-hwmon";
		io-channels = <&ain_12V 0>, <&ain_12V_i>, <&ain_3V3_i>, <&ain_Vin_i>, <&am335x_adc 4>;
	};

	ain_12V: ain-12V {
		compatible = "voltage-divider";
		io-channels = <&am335x_adc 0>;
		#io-channel-cells = <1>;
		output-ohms = <10000>;
		full-ohms = <110000>;
	};

	ain_12V_i: ain-12V-i {
		compatible = "current-sense-amplifier";
		io-channels = <&am335x_adc 1>;
		#io-channel-cells = <0>;
		sense-resistor-micro-ohms = <10000>;
		sense-gain-mult = <50>;
	};

	ain_3V3_i: ain-3V3-i {
		compatible = "current-sense-amplifier";
		io-channels = <&am335x_adc 2>;
		#io-channel-cells = <0>;
		sense-resistor-micro-ohms = <10000>;
		sense-gain-mult = <50>;
	};

	ain_Vin_i: ain-Vin-i {
		compatible = "current-sense-amplifier";
		io-channels = <&am335x_adc 3>;
		#io-channel-cells = <0>;
		sense-resistor-micro-ohms = <22000>;
		sense-gain-mult = <46400>;
		sense-gain-div = <1000>;
	};

[the fifth adm335x_adc channel is just to be used as-is, there's no
circuitry in front of that].

With this, running the 'sensors' command does produce sensible output,
but it also results in this splat:

  ============================================
  WARNING: possible recursive locking detected
  6.17.11 #5 Not tainted
  --------------------------------------------
  sensors/414 is trying to acquire lock:
  c1dc7b68 (&iio_dev_opaque->info_exist_lock){+.+.}-{3:3}, at: iio_read_channel_raw+0x20/0x6c
  
  but task is already holding lock:
  c1dd2b68 (&iio_dev_opaque->info_exist_lock){+.+.}-{3:3}, at: iio_read_channel_processed_scale+0x24/0xd8
  
  other info that might help us debug this:
   Possible unsafe locking scenario:
  
         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
  Exception stack(0xe00b9fa8 to 0xe00b9ff0)
  9fa0:                   00000000 00000000 00000003 004639f0 00001000 00000000
  9fc0: 00000000 00000000 b6e80a7c 00000003 00000000 ffffffff 0046b1b8 bea555bc
  9fe0: 00001000 bea54f40 00000001 b6d89e6c

Just accessing a single property under /sys/class/hwmon/hwmonX/ also
results in a similar splat.

Am I doing things wrong, or is there a genuine problem hiding somewhere?

Rasmus

Re: lockdep splat involving iio-hwmon and iio-rescale drivers

[Guenter Roeck]

On 12/10/25 06:31, Rasmus Villemoes wrote:
> Hi
> 
> I have an am335x board where the AINx channels are used for various
> voltage/current measurement, i.e. they have a variety of current shunts
> and voltage dividers in front of them. Describing those is pretty
> straight-forward, and the resulting iio devices seem to work.
> 
> Now, I'd like to expose those as hwmon sensors, because there are other
> i2c-attached ADCs on the board exposed that way, and it would be nice to
> have them all available through "sensors" command in userspace. So
> altogether, my .dts additions are
> 
> 	ain {
> 		compatible = "iio-hwmon";
> 		io-channels = <&ain_12V 0>, <&ain_12V_i>, <&ain_3V3_i>, <&ain_Vin_i>, <&am335x_adc 4>;
> 	};
> 
> 	ain_12V: ain-12V {
> 		compatible = "voltage-divider";
> 		io-channels = <&am335x_adc 0>;
> 		#io-channel-cells = <1>;
> 		output-ohms = <10000>;
> 		full-ohms = <110000>;
> 	};
> 
> 	ain_12V_i: ain-12V-i {
> 		compatible = "current-sense-amplifier";
> 		io-channels = <&am335x_adc 1>;
> 		#io-channel-cells = <0>;
> 		sense-resistor-micro-ohms = <10000>;
> 		sense-gain-mult = <50>;
> 	};
> 
> 	ain_3V3_i: ain-3V3-i {
> 		compatible = "current-sense-amplifier";
> 		io-channels = <&am335x_adc 2>;
> 		#io-channel-cells = <0>;
> 		sense-resistor-micro-ohms = <10000>;
> 		sense-gain-mult = <50>;
> 	};
> 
> 	ain_Vin_i: ain-Vin-i {
> 		compatible = "current-sense-amplifier";
> 		io-channels = <&am335x_adc 3>;
> 		#io-channel-cells = <0>;
> 		sense-resistor-micro-ohms = <22000>;
> 		sense-gain-mult = <46400>;
> 		sense-gain-div = <1000>;
> 	};
> 
> [the fifth adm335x_adc channel is just to be used as-is, there's no
> circuitry in front of that].
> 
> With this, running the 'sensors' command does produce sensible output,
> but it also results in this splat:
> 
>    ============================================
>    WARNING: possible recursive locking detected
>    6.17.11 #5 Not tainted
>    --------------------------------------------
>    sensors/414 is trying to acquire lock:
>    c1dc7b68 (&iio_dev_opaque->info_exist_lock){+.+.}-{3:3}, at: iio_read_channel_raw+0x20/0x6c
>    
>    but task is already holding lock:
>    c1dd2b68 (&iio_dev_opaque->info_exist_lock){+.+.}-{3:3}, at: iio_read_channel_processed_scale+0x24/0xd8
>    
>    other info that might help us debug this:
>     Possible unsafe locking scenario:
>    
>           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
>    Exception stack(0xe00b9fa8 to 0xe00b9ff0)
>    9fa0:                   00000000 00000000 00000003 004639f0 00001000 00000000
>    9fc0: 00000000 00000000 b6e80a7c 00000003 00000000 ffffffff 0046b1b8 bea555bc
>    9fe0: 00001000 bea54f40 00000001 b6d89e6c
> 
> Just accessing a single property under /sys/class/hwmon/hwmonX/ also
> results in a similar splat.
> 
> Am I doing things wrong, or is there a genuine problem hiding somewhere?
> 
The lock is acquired twice in the iio code, once in iio_read_channel_processed_scale()
and then again in iio_read_channel_raw(). That does look like a genuine problem to me,
but I don't know how to fix it.

Guenter

Re: lockdep splat involving iio-hwmon and iio-rescale drivers

[Peter Rosin]

Hi,

2025-12-10 at 20:12, Guenter Roeck wrote:
> On 12/10/25 06:31, Rasmus Villemoes wrote:
>> Just accessing a single property under /sys/class/hwmon/hwmonX/ also
>> results in a similar splat.
>>
>> Am I doing things wrong, or is there a genuine problem hiding somewhere?
>>
> The lock is acquired twice in the iio code, once in iio_read_channel_processed_scale()
> and then again in iio_read_channel_raw(). That does look like a genuine problem to me,
> but I don't know how to fix it.
> 
> Guenter
> 

I don't see it?

The info_exist_lock is per iio device and the
iio_read_channel_processed_scale() call operates on the rescaler
iio channel, which is a separate device from the rescaler iio
source channel of the iio_read_channel_raw() call (in this case
from the am335x adc).

So, I think everything is fine, except for the lockdep splat.

If I'm right, lockdep is not informed that recursion may happen
and thus collects the recursive iio calls in the same bin, i.e.
it keeps track of all info_exist_lock uses a single lockdep
class, even though it's not actually the same lock.

Before commit 3092bde731ca ("iio: inkern: move to the cleanup.h
magic") I think this could have been solved with a number of:

-	mutex_lock(&iio_dev_opaque->info_exist_lock);
+	mutex_lock_nested(&iio_dev_opaque->info_exist_lock);

I don't know how to convert that change to the world of guards.

But then again, maybe I'm just wrong...

Cheers,
Peter

Re: lockdep splat involving iio-hwmon and iio-rescale drivers

[Peter Rosin]

2025-12-10 at 23:54, Peter Rosin wrote:
> Before commit 3092bde731ca ("iio: inkern: move to the cleanup.h
> magic") I think this could have been solved with a number of:
> 
> -	mutex_lock(&iio_dev_opaque->info_exist_lock);
> +	mutex_lock_nested(&iio_dev_opaque->info_exist_lock);

Oops, we need something clever for the (missing) subclass
argument to the mutex_lock_nested() calls, which I simply forgot
all about. It should have been:

+	mutex_lock_nested(&iio_dev_opaque->info_exist_lock, *subclass*);

I don't know what sane subclasses there are. One thing could be
the recursion depth, but I don't think we want to keep track of that
just for lockdep? Another is to use one lockdep class for every
info_exist_lock, but that can generate a lot of lockdep classes...

Cheers,
Peter

Re: lockdep splat involving iio-hwmon and iio-rescale drivers

[Rasmus Villemoes]

On Thu, Dec 11 2025, Peter Rosin <peda@axentia.se> wrote:

> 2025-12-10 at 23:54, Peter Rosin wrote:
>> Before commit 3092bde731ca ("iio: inkern: move to the cleanup.h
>> magic") I think this could have been solved with a number of:
>> 
>> -	mutex_lock(&iio_dev_opaque->info_exist_lock);
>> +	mutex_lock_nested(&iio_dev_opaque->info_exist_lock);
>
> Oops, we need something clever for the (missing) subclass
> argument to the mutex_lock_nested() calls, which I simply forgot
> all about. It should have been:
>
> +	mutex_lock_nested(&iio_dev_opaque->info_exist_lock, *subclass*);
>
> I don't know what sane subclasses there are. One thing could be
> the recursion depth, but I don't think we want to keep track of that
> just for lockdep? Another is to use one lockdep class for every
> info_exist_lock, but that can generate a lot of lockdep classes...

It doesn't seem to me that that info_exist_lock is the proper
mechanism for whatever it is it is protecting against.

I'm not even sure it's needed, because if the device could be
unregistered while somebody has a reference to it, why is it even
allowed to take that lock in the first place (i.e., why is the memory
containing the info_exist_lock guaranteed to still be valid)?

But, since I'm not going to propose just ripping it out, perhaps a
better approach would be something like what the gpio subsystem did in
d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device
with SRCU"), at least superficially it seems to be about a similar
problem.

Rasmus

Re: lockdep splat involving iio-hwmon and iio-rescale drivers

[Jonathan Cameron]

On Fri, 12 Dec 2025 14:12:48 +0100
Rasmus Villemoes <ravi@prevas.dk> wrote:

> On Thu, Dec 11 2025, Peter Rosin <peda@axentia.se> wrote:
> 
> > 2025-12-10 at 23:54, Peter Rosin wrote:  
> >> Before commit 3092bde731ca ("iio: inkern: move to the cleanup.h
> >> magic") I think this could have been solved with a number of:
> >> 
> >> -	mutex_lock(&iio_dev_opaque->info_exist_lock);
> >> +	mutex_lock_nested(&iio_dev_opaque->info_exist_lock);  
> >
> > Oops, we need something clever for the (missing) subclass
> > argument to the mutex_lock_nested() calls, which I simply forgot
> > all about. It should have been:
> >
> > +	mutex_lock_nested(&iio_dev_opaque->info_exist_lock, *subclass*);
> >
> > I don't know what sane subclasses there are. One thing could be
> > the recursion depth, but I don't think we want to keep track of that
> > just for lockdep? Another is to use one lockdep class for every
> > info_exist_lock, but that can generate a lot of lockdep classes...  

Either option should work but agreed tracking depth when
we otherwise don't care about it feels excessive.
We already have classes for the other major internal lock in IIO devices (mlock)

https://elixir.bootlin.com/linux/v6.18.1/source/drivers/iio/industrialio-core.c#L1722

I'm a bit curious we haven't seen many reports of this one. Whilst
there are relatively few IIO drivers that consume other IIO driver provided
channels it's also not a particularly new thing.

> 
> It doesn't seem to me that that info_exist_lock is the proper
> mechanism for whatever it is it is protecting against.
> 
> I'm not even sure it's needed, because if the device could be
> unregistered while somebody has a reference to it, why is it even
> allowed to take that lock in the first place (i.e., why is the memory
> containing the info_exist_lock guaranteed to still be valid)?

It's to protect against racing with setting of iio_dev->info to NULL.
The iio_dev itself is reference counted so it should always be safe
to do this lookup. Note that use of info == NULL isn't about accessing
info alone, but as a general gate on device has gone away (but
structures are still there until all consumers - in kernel or userspace
- are done).

In theory at least all consumers use proxy functions that check that
and error out if the thing being consumed has gone away.  Those that
are sleeping on anything are woken up and return errors.

Sure there are other approaches to providing that protection and
there was an attempt to do a generic solution a while back. I lost
track of where that got to.  Might be worth a revisit at some point.

For now though a lockdep class per instance seems the way to go to me.

Jonathan


> 
> But, since I'm not going to propose just ripping it out, perhaps a
> better approach would be something like what the gpio subsystem did in
> d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device
> with SRCU"), at least superficially it seems to be about a similar
> problem.
> 
> Rasmus

Re: lockdep splat involving iio-hwmon and iio-rescale drivers

[Jonathan Cameron]

On Sun, 14 Dec 2025 05:56:14 +0100
Peter Rosin <peda@axentia.se> wrote:

> On December 13, 2025 5:57:23 PM GMT+01:00, Jonathan Cameron <jic23@kernel.org> wrote:
> > I'm a bit curious we haven't seen many reports of this one. Whilst
> > there are relatively few IIO drivers that consume other IIO driver
> > provided channels it's also not a particularly new thing.  
> 
> I suspect the reason is that plain old iio_read_channel_raw does not
> grab the info_exist_lock, so it seems that you need two levels of
> iio_read_channel_processed[_scale] (or something like that). Which
> is rarer still.

iio_read_channel_raw() does grab the lock (you had me worried
there for a moment!)  Does it with a guard() so perhaps that threw you
off.


Jonathan

> 
> Cheers,
> Peter

Re: lockdep splat involving iio-hwmon and iio-rescale drivers

[Rasmus Villemoes]

On Sun, Dec 14 2025, Peter Rosin <peda@axentia.se> wrote:

> On December 13, 2025 5:57:23 PM GMT+01:00, Jonathan Cameron <jic23@kernel.org> wrote:
>> I'm a bit curious we haven't seen many reports of this one. Whilst
>> there are relatively few IIO drivers that consume other IIO driver
>> provided channels it's also not a particularly new thing.
>
> I suspect the reason is that plain old iio_read_channel_raw does not
> grab the info_exist_lock, so it seems that you need two levels of
> iio_read_channel_processed[_scale] (or something like that). Which
> is rarer still.

As Jonathan pointed out, that function does grab the
info_exist_lock. However, when reading the attributes via the
/sys/bus/iio/..., that seems to go through iio_read_channel_info() which
does not.

So when reading the value of a "rescale" device via /sys/bus/iio/, the
info_exist_lock of the outer device is not taken, only of the underlying
voltage channel. But the splat can be obtained without involving the
hwmon bridge, if one chains "rescale" devices (this doesn't necessarily
describe any existing hardware or even make sense, just for demonstration)

 	foo: foo {
 		compatible = "voltage-divider";
 		io-channels = <&am335x_adc 0>;
 		#io-channel-cells = <1>;
 		output-ohms = <10000>;
 		full-ohms = <110000>;
 	};
 	bar: bar {
 		compatible = "voltage-divider";
 		io-channels = <&foo 0>;
 		#io-channel-cells = <1>;
 		output-ohms = <1234>;
 		full-ohms = <2345>;
 	};

results in

        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 cat/478:
  #0: c3721b5c (&p->lock){+.+.}-{3:3}, at: seq_read_iter+0x44/0x4e4
  #1: c4d1e284 (&of->mutex){+.+.}-{3:3}, at: kernfs_seq_start+0x1c/0xac
  #2: c2805a18 (kn->active#32){.+.+}-{0:0}, at: kernfs_seq_start+0x30/0xac
  #3: c1d96b68 (&iio_dev_opaque->info_exist_lock){+.+.}-{3:3}, at: iio_read_channel_raw+0x20/0x6c
 
 stack backtrace:
 CPU: 0 UID: 0 PID: 478 Comm: cat Not tainted 6.17.11 #6 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_raw+0x44/0x6c
  iio_read_channel_raw from rescale_read_raw+0x128/0x1c4
  rescale_read_raw from iio_read_channel_info+0xfc/0x11c
  iio_read_channel_info 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 copy_splice_read+0x180/0x300
  copy_splice_read from splice_direct_to_actor+0xb4/0x2b8
  splice_direct_to_actor from do_splice_direct+0x7c/0xc8
  do_splice_direct from do_sendfile+0x314/0x3ac
  do_sendfile from sys_sendfile64+0x130/0x14c
  sys_sendfile64 from ret_fast_syscall+0x0/0x1c

I'll try to add that separate lockdep class. It should be fairly
mechanical (famous last words).

Rasmus