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

[Jonathan Cameron <jic23@kernel.org>]

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