Re: [PATCH 1/2] regmap: pass map name to lockdep
On 12/18/2014 11:34 PM, Lars-Peter Clausen wrote: On 12/18/2014 10:05 PM, Antti Palosaari wrote: lockdep complains recursive locking and deadlock when two different regmap instances are called in a nested order. That happen easily for example when both I2C client and muxed/repeater I2C adapter are using regmap. As a solution, pass regmap name for lockdep in order to force lockdep validate regmap mutex per driver - not as all regmap instances grouped together. That's not how it works. Locks are grouped by lock class, the name is just for pretty printing. The only reason you do not get a warning anymore is because you have now different lock classes one for configs with a name and one for configs without a name. You really need a way to specify a custom lock class per regmap instance in order to solve this problem. I looked example for that solution from v4l controls. So it is also wrong? https://patchwork.linuxtv.org/patch/17262/ Do you think I should change to mutex_lock_nested() as documented in Documentation/locking/lockdep-design.txt ? Should these macros used at all: include/linux/lockdep.h There is not much documentation, especially how these recursive lock warnings should be silenced. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] regmap: pass map name to lockdep
On 12/19/2014 11:58 AM, Antti Palosaari wrote: On 12/18/2014 11:34 PM, Lars-Peter Clausen wrote: On 12/18/2014 10:05 PM, Antti Palosaari wrote: lockdep complains recursive locking and deadlock when two different regmap instances are called in a nested order. That happen easily for example when both I2C client and muxed/repeater I2C adapter are using regmap. As a solution, pass regmap name for lockdep in order to force lockdep validate regmap mutex per driver - not as all regmap instances grouped together. That's not how it works. Locks are grouped by lock class, the name is just for pretty printing. The only reason you do not get a warning anymore is because you have now different lock classes one for configs with a name and one for configs without a name. You really need a way to specify a custom lock class per regmap instance in order to solve this problem. I looked example for that solution from v4l controls. So it is also wrong? https://patchwork.linuxtv.org/patch/17262/ No, that's correct. It creates one lock class per v4l2_ctrl_handler_init() invocation site. Do you think I should change to mutex_lock_nested() as documented in Documentation/locking/lockdep-design.txt ? No, mutex_lock_nested() only works if you can identify lock subclasses. Should these macros used at all: include/linux/lockdep.h There is not much documentation, especially how these recursive lock warnings should be silenced. You have a couple of options, either do what v4l2_ctrl_handler_init() and create a lock class key per regmap_init_*() invocation site. Or just add a lock class key per regmap instance. Or add a helper function which allows to change the lock class of a regmap instance that can be used by drivers where we expect that there will be nested locking. E.g. like in a bus master. - Lars -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] regmap: pass map name to lockdep
On 12/19/2014 03:45 PM, Lars-Peter Clausen wrote: On 12/19/2014 11:58 AM, Antti Palosaari wrote: On 12/18/2014 11:34 PM, Lars-Peter Clausen wrote: On 12/18/2014 10:05 PM, Antti Palosaari wrote: lockdep complains recursive locking and deadlock when two different regmap instances are called in a nested order. That happen easily for example when both I2C client and muxed/repeater I2C adapter are using regmap. As a solution, pass regmap name for lockdep in order to force lockdep validate regmap mutex per driver - not as all regmap instances grouped together. That's not how it works. Locks are grouped by lock class, the name is just for pretty printing. The only reason you do not get a warning anymore is because you have now different lock classes one for configs with a name and one for configs without a name. You really need a way to specify a custom lock class per regmap instance in order to solve this problem. I looked example for that solution from v4l controls. So it is also wrong? https://patchwork.linuxtv.org/patch/17262/ No, that's correct. It creates one lock class per v4l2_ctrl_handler_init() invocation site. ah, yes, it it includes that piece of code, which declares key for each caller module. Do you think I should change to mutex_lock_nested() as documented in Documentation/locking/lockdep-design.txt ? No, mutex_lock_nested() only works if you can identify lock subclasses. Should these macros used at all: include/linux/lockdep.h There is not much documentation, especially how these recursive lock warnings should be silenced. You have a couple of options, either do what v4l2_ctrl_handler_init() and create a lock class key per regmap_init_*() invocation site. Or just add a lock class key per regmap instance. Or add a helper function which allows to change the lock class of a regmap instance that can be used by drivers where we expect that there will be nested locking. E.g. like in a bus master. I think I will add key to regmap config. Then driver which uses regmap, could declare key and pass it to regmap. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] regmap: pass map name to lockdep
lockdep complains recursive locking and deadlock when two different
regmap instances are called in a nested order. That happen easily
for example when both I2C client and muxed/repeater I2C adapter are
using regmap. As a solution, pass regmap name for lockdep in order
to force lockdep validate regmap mutex per driver - not as all regmap
instances grouped together.
Here is example connection, where nested regmap is used to control
I2C mux.
__ ___ ___
| USB IF | | demod | | tuner |
|--| |---| |---|
| |--I2C--|-/ |--I2C--| |
|I2C master| | I2C mux | | I2C slave |
|__| |___| |___|
Cc: Mark Brown broo...@kernel.org
Signed-off-by: Antti Palosaari cr...@iki.fi
---
drivers/base/regmap/regmap.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index d2f8a81..8d8ad37 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -559,6 +559,12 @@ struct regmap *regmap_init(struct device *dev,
mutex_init(map-mutex);
map-lock = regmap_lock_mutex;
map-unlock = regmap_unlock_mutex;
+ if (config-name) {
+ static struct lock_class_key key;
+
+ lockdep_set_class_and_name(map-mutex, key,
+ config-name);
+ }
}
map-lock_arg = map;
}
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] regmap: pass map name to lockdep
On 12/18/2014 10:05 PM, Antti Palosaari wrote: lockdep complains recursive locking and deadlock when two different regmap instances are called in a nested order. That happen easily for example when both I2C client and muxed/repeater I2C adapter are using regmap. As a solution, pass regmap name for lockdep in order to force lockdep validate regmap mutex per driver - not as all regmap instances grouped together. That's not how it works. Locks are grouped by lock class, the name is just for pretty printing. The only reason you do not get a warning anymore is because you have now different lock classes one for configs with a name and one for configs without a name. You really need a way to specify a custom lock class per regmap instance in order to solve this problem. - Lars -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
