Re: [PATCH v2 0/4] gpio: fix an incorrect lockdep warning

[Bartosz Golaszewski]

2016-09-20 10:48 GMT+02:00 Peter Rosin <peda@xxxxxxxxxx>:
>
> One pretty simple problematic case is:
>
>   .---.          .----.
>   |   |          |    |-- i2c2
>   |   |-- i2c0 --|mux0|          .----.
>   | l |          |    |-- i2c3 --|gpio|
>   | i |          '----'          '----'
>   | n |             .--------------'
>   | u |          .----.          .----.
>   | x |          |    |-- i2c4 --|dev0|
>   |   |-- i2c1 --|mux1|          '----'
>   |   |          |    |-- i2c5
>   '---'          '----'
>
> Accesses to dev0 will:
>
> 1. lock i2c1:mux_lock (depth 0)
> 2. switch mux1 to i2c4 using gpio
>  a lock i2c0:mux_lock (depth 0)
>  b switch mux0 to i2c3 using whatever
>  c access gpio
>  d unlock i2c0:mux_lock
> 3. access dev0
> 4. unlock i2c1:mux_lock
>
> 2a will cause a lockdep splat if i2c0:mux_lock is in the same
> lockdep class & subclass as i2c1:mux_lock. So, lockdep needs
> separate lockdep classes depending on the i2c root adapter
> (subclasses are needed to handle deeper trees, so they are off
> limits). Great fun. How do I go about creating a new lockdep
> class for every i2c root adapter instance?
>

I feel like it's just wrong to set an arbitrary limit on the number of
i2c branches - and this is what the result of this approach would be.

One solution that comes to mind is to have a separate, global set of
lock classes solely for gpio expanders. I think you mentioned earlier
that it's the only thing that can cause this kind of lockdep false
positives. We could potentially have a limited set of lock classes and
every expander that would need one would request it using some kind of
API ensuring that every instance gets a separate class. But this
sounds like a big hack too I'm afraid... And regmap would need to be
aware of that as well.

Anyways, we're past rc7 already and 4.9 will be the next LTS kernel.
We have real hardware here that runs on mainline linux and is
suffering from this issue. Are there any objections against merging
this series now and continuing the work on improving the solution for
4.10?

Best regards,
Bartosz Golaszewski