diff options
author | Andy Walls <awalls@md.metrocast.net> | 2013-03-09 03:55:11 -0500 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab@redhat.com> | 2013-03-24 10:47:09 -0400 |
commit | 6cd247ef22e493e1884e576c066661538b031981 (patch) | |
tree | c8293aafc52bc1db11ec7b52c1df31b456cd2816 | |
parent | 842059aa4796d7c59bc3801d48896ba06b1a1287 (diff) |
[media] v4l2-ctrls: eliminate lockdep false alarms for struct v4l2_ctrl_handler.lock
When calling v4l2_ctrl_add_handler(), lockdep would detect a potential
recursive locking problem on a situation that is by design intended and
not a recursive lock. This happened because all struct
v4l2_ctrl_handler.lock mutexes were created as members of the same lock
class in v4l2_ctrl_handler_init(), and v4l2_ctrl_add_handler() takes the
hdl->lock on two different v4l2_ctrl_handler instances.
This change breaks the large lockdep lock class for struct
v4l2_ctrl_handler.lock and breaks it into v4l2_ctrl_handler
instantiation specific lock classes with meaningful class names.
This will validly eliminate lockdep alarms for v4l2_ctrl_handler locking
validation, as long as the relationships between drivers adding v4l2
controls to their own handler from other v4l2 drivers' control handlers
remains straightforward.
struct v4l2_ctrl_handler.lock lock classes are created with names such
that the output of cat /proc/lockdep indicates where in the v4l2 driver
code v4l2_ctrl_handle_init() is being called on instantiations:
ffffffffa045f490 FD: 10 BD: 8 +.+...: cx2341x:1534:(hdl)->lock
ffffffffa0497d20 FD: 12 BD: 2 +.+.+.: saa7115:1581:(hdl)->lock
ffffffffa04ac660 FD: 14 BD: 2 +.+.+.: msp3400_driver:756:(hdl)->lock
ffffffffa0484b90 FD: 12 BD: 1 +.+.+.: ivtv_gpio:366:(&itv->hdl_gpio)->lock
ffffffffa04eb530 FD: 11 BD: 2 +.+.+.: cx25840_core:1982:(&state->hdl)->lock
ffffffffa04fbc80 FD: 11 BD: 3 +.+.+.: wm8775:246:(&state->hdl)->lock
Some lock chains, that were previously causing the recursion alarms, are
now visible in the output of cat /proc/lockdep_chains:
irq_context: 0
[ffffffffa0497d20] saa7115:1581:(hdl)->lock
[ffffffffa045f490] cx2341x:1534:(hdl)->lock
irq_context: 0
[ffffffffa04ac660] msp3400_driver:756:(hdl)->lock
[ffffffffa045f490] cx2341x:1534:(hdl)->lock
irq_context: 0
[ffffffffa0484b90] ivtv_gpio:366:(&itv->hdl_gpio)->lock
[ffffffffa045f490] cx2341x:1534:(hdl)->lock
irq_context: 0
[ffffffffa04eb530] cx25840_core:1982:(&state->hdl)->lock
[ffffffffa045f490] cx2341x:1534:(hdl)->lock
irq_context: 0
[ffffffffa04fbc80] wm8775:246:(&state->hdl)->lock
[ffffffffa045f490] cx2341x:1534:(hdl)->lock
Signed-off-by: Andy Walls <awalls@md.metrocast.net>
[hans.verkuil@cisco.com: keep mutex_init in v4l2_ctrl_handler_init_class]
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
-rw-r--r-- | drivers/media/v4l2-core/v4l2-ctrls.c | 8 | ||||
-rw-r--r-- | include/media/v4l2-ctrls.h | 29 |
2 files changed, 31 insertions, 6 deletions
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 6b28b5800500..b36d1ec13ee6 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c | |||
@@ -1362,11 +1362,13 @@ static inline int handler_set_err(struct v4l2_ctrl_handler *hdl, int err) | |||
1362 | } | 1362 | } |
1363 | 1363 | ||
1364 | /* Initialize the handler */ | 1364 | /* Initialize the handler */ |
1365 | int v4l2_ctrl_handler_init(struct v4l2_ctrl_handler *hdl, | 1365 | int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler *hdl, |
1366 | unsigned nr_of_controls_hint) | 1366 | unsigned nr_of_controls_hint, |
1367 | struct lock_class_key *key, const char *name) | ||
1367 | { | 1368 | { |
1368 | hdl->lock = &hdl->_lock; | 1369 | hdl->lock = &hdl->_lock; |
1369 | mutex_init(hdl->lock); | 1370 | mutex_init(hdl->lock); |
1371 | lockdep_set_class_and_name(hdl->lock, key, name); | ||
1370 | INIT_LIST_HEAD(&hdl->ctrls); | 1372 | INIT_LIST_HEAD(&hdl->ctrls); |
1371 | INIT_LIST_HEAD(&hdl->ctrl_refs); | 1373 | INIT_LIST_HEAD(&hdl->ctrl_refs); |
1372 | hdl->nr_of_buckets = 1 + nr_of_controls_hint / 8; | 1374 | hdl->nr_of_buckets = 1 + nr_of_controls_hint / 8; |
@@ -1375,7 +1377,7 @@ int v4l2_ctrl_handler_init(struct v4l2_ctrl_handler *hdl, | |||
1375 | hdl->error = hdl->buckets ? 0 : -ENOMEM; | 1377 | hdl->error = hdl->buckets ? 0 : -ENOMEM; |
1376 | return hdl->error; | 1378 | return hdl->error; |
1377 | } | 1379 | } |
1378 | EXPORT_SYMBOL(v4l2_ctrl_handler_init); | 1380 | EXPORT_SYMBOL(v4l2_ctrl_handler_init_class); |
1379 | 1381 | ||
1380 | /* Free all controls and control refs */ | 1382 | /* Free all controls and control refs */ |
1381 | void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl) | 1383 | void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl) |
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index f00d42bc01a6..7343a27fe819 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h | |||
@@ -259,7 +259,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, | |||
259 | s32 *min, s32 *max, s32 *step, s32 *def, u32 *flags); | 259 | s32 *min, s32 *max, s32 *step, s32 *def, u32 *flags); |
260 | 260 | ||
261 | 261 | ||
262 | /** v4l2_ctrl_handler_init() - Initialize the control handler. | 262 | /** v4l2_ctrl_handler_init_class() - Initialize the control handler. |
263 | * @hdl: The control handler. | 263 | * @hdl: The control handler. |
264 | * @nr_of_controls_hint: A hint of how many controls this handler is | 264 | * @nr_of_controls_hint: A hint of how many controls this handler is |
265 | * expected to refer to. This is the total number, so including | 265 | * expected to refer to. This is the total number, so including |
@@ -268,12 +268,35 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, | |||
268 | * are allocated) or the control lookup becomes slower (not enough | 268 | * are allocated) or the control lookup becomes slower (not enough |
269 | * buckets are allocated, so there are more slow list lookups). | 269 | * buckets are allocated, so there are more slow list lookups). |
270 | * It will always work, though. | 270 | * It will always work, though. |
271 | * @key: Used by the lock validator if CONFIG_LOCKDEP is set. | ||
272 | * @name: Used by the lock validator if CONFIG_LOCKDEP is set. | ||
271 | * | 273 | * |
272 | * Returns an error if the buckets could not be allocated. This error will | 274 | * Returns an error if the buckets could not be allocated. This error will |
273 | * also be stored in @hdl->error. | 275 | * also be stored in @hdl->error. |
276 | * | ||
277 | * Never use this call directly, always use the v4l2_ctrl_handler_init | ||
278 | * macro that hides the @key and @name arguments. | ||
274 | */ | 279 | */ |
275 | int v4l2_ctrl_handler_init(struct v4l2_ctrl_handler *hdl, | 280 | int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler *hdl, |
276 | unsigned nr_of_controls_hint); | 281 | unsigned nr_of_controls_hint, |
282 | struct lock_class_key *key, const char *name); | ||
283 | |||
284 | #ifdef CONFIG_LOCKDEP | ||
285 | #define v4l2_ctrl_handler_init(hdl, nr_of_controls_hint) \ | ||
286 | ( \ | ||
287 | ({ \ | ||
288 | static struct lock_class_key _key; \ | ||
289 | v4l2_ctrl_handler_init_class(hdl, nr_of_controls_hint, \ | ||
290 | &_key, \ | ||
291 | KBUILD_BASENAME ":" \ | ||
292 | __stringify(__LINE__) ":" \ | ||
293 | "(" #hdl ")->_lock"); \ | ||
294 | }) \ | ||
295 | ) | ||
296 | #else | ||
297 | #define v4l2_ctrl_handler_init(hdl, nr_of_controls_hint) \ | ||
298 | v4l2_ctrl_handler_init_class(hdl, nr_of_controls_hint, NULL, NULL) | ||
299 | #endif | ||
277 | 300 | ||
278 | /** v4l2_ctrl_handler_free() - Free all controls owned by the handler and free | 301 | /** v4l2_ctrl_handler_free() - Free all controls owned by the handler and free |
279 | * the control list. | 302 | * the control list. |