Skip to content

Commit d20b484

Browse files
author
Thomas Hellström
committed
drm/drm_exec: Work around a WW mutex lockdep oddity
If *any* object of a certain WW mutex class is locked, lockdep will consider *all* mutexes of that class as locked. Also the lock allocation tracking code will apparently register only the address of the first mutex of a given class locked in a sequence. This has the odd consequence that if that first mutex is unlocked while other mutexes of the same class remain locked and then its memory then freed, the lock alloc tracking code will incorrectly assume that memory is freed with a held lock in there. For now, work around that for drm_exec by releasing the first grabbed object lock last. v2: - Fix a typo (Danilo Krummrich) - Reword the commit message a bit. - Add a Fixes: tag Related lock alloc tracking warning: [ 322.660067] ========================= [ 322.660070] WARNING: held lock freed! [ 322.660074] 6.5.0-rc7+ Rust-for-Linux#155 Tainted: G U N [ 322.660078] ------------------------- [ 322.660081] kunit_try_catch/4981 is freeing memory ffff888112adc000-ffff888112adc3ff, with a lock still held there! [ 322.660089] ffff888112adc1a0 (reservation_ww_class_mutex){+.+.}-{3:3}, at: drm_exec_lock_obj+0x11a/0x600 [drm_exec] [ 322.660104] 2 locks held by kunit_try_catch/4981: [ 322.660108] #0: ffffc9000343fe18 (reservation_ww_class_acquire){+.+.}-{0:0}, at: test_early_put+0x22f/0x490 [drm_exec_test] [ 322.660123] #1: ffff888112adc1a0 (reservation_ww_class_mutex){+.+.}-{3:3}, at: drm_exec_lock_obj+0x11a/0x600 [drm_exec] [ 322.660135] stack backtrace: [ 322.660139] CPU: 7 PID: 4981 Comm: kunit_try_catch Tainted: G U N 6.5.0-rc7+ Rust-for-Linux#155 [ 322.660146] Hardware name: ASUS System Product Name/PRIME B560M-A AC, BIOS 0403 01/26/2021 [ 322.660152] Call Trace: [ 322.660155] <TASK> [ 322.660158] dump_stack_lvl+0x57/0x90 [ 322.660164] debug_check_no_locks_freed+0x20b/0x2b0 [ 322.660172] slab_free_freelist_hook+0xa1/0x160 [ 322.660179] ? drm_exec_unlock_all+0x168/0x2a0 [drm_exec] [ 322.660186] __kmem_cache_free+0xb2/0x290 [ 322.660192] drm_exec_unlock_all+0x168/0x2a0 [drm_exec] [ 322.660200] drm_exec_fini+0xf/0x1c0 [drm_exec] [ 322.660206] test_early_put+0x289/0x490 [drm_exec_test] [ 322.660215] ? __pfx_test_early_put+0x10/0x10 [drm_exec_test] [ 322.660222] ? __kasan_check_byte+0xf/0x40 [ 322.660227] ? __ksize+0x63/0x140 [ 322.660233] ? drmm_add_final_kfree+0x3e/0xa0 [drm] [ 322.660289] ? _raw_spin_unlock_irqrestore+0x30/0x60 [ 322.660294] ? lockdep_hardirqs_on+0x7d/0x100 [ 322.660301] ? __pfx_kunit_try_run_case+0x10/0x10 [kunit] [ 322.660310] ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10 [kunit] [ 322.660319] kunit_generic_run_threadfn_adapter+0x4a/0x90 [kunit] [ 322.660328] kthread+0x2e7/0x3c0 [ 322.660334] ? __pfx_kthread+0x10/0x10 [ 322.660339] ret_from_fork+0x2d/0x70 [ 322.660345] ? __pfx_kthread+0x10/0x10 [ 322.660349] ret_from_fork_asm+0x1b/0x30 [ 322.660358] </TASK> [ 322.660818] ok 8 test_early_put Cc: Christian König <[email protected]> Cc: Boris Brezillon <[email protected]> Cc: Danilo Krummrich <[email protected]> Cc: [email protected] Fixes: 0959321 ("drm: execution context for GEM buffers v7") Signed-off-by: Thomas Hellström <[email protected]> Reviewed-by: Boris Brezillon <[email protected]> Reviewed-by: Danilo Krummrich <[email protected]> Reviewed-by: Christian König <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 45dc8fc commit d20b484

File tree

2 files changed

+32
-5
lines changed

2 files changed

+32
-5
lines changed

drivers/gpu/drm/drm_exec.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec)
5656
struct drm_gem_object *obj;
5757
unsigned long index;
5858

59-
drm_exec_for_each_locked_object(exec, index, obj) {
59+
drm_exec_for_each_locked_object_reverse(exec, index, obj) {
6060
dma_resv_unlock(obj->resv);
6161
drm_gem_object_put(obj);
6262
}

include/drm/drm_exec.h

+31-4
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,20 @@ struct drm_exec {
5151
struct drm_gem_object *prelocked;
5252
};
5353

54+
/**
55+
* drm_exec_obj() - Return the object for a give drm_exec index
56+
* @exec: Pointer to the drm_exec context
57+
* @index: The index.
58+
*
59+
* Return: Pointer to the locked object corresponding to @index if
60+
* index is within the number of locked objects. NULL otherwise.
61+
*/
62+
static inline struct drm_gem_object *
63+
drm_exec_obj(struct drm_exec *exec, unsigned long index)
64+
{
65+
return index < exec->num_objects ? exec->objects[index] : NULL;
66+
}
67+
5468
/**
5569
* drm_exec_for_each_locked_object - iterate over all the locked objects
5670
* @exec: drm_exec object
@@ -59,10 +73,23 @@ struct drm_exec {
5973
*
6074
* Iterate over all the locked GEM objects inside the drm_exec object.
6175
*/
62-
#define drm_exec_for_each_locked_object(exec, index, obj) \
63-
for (index = 0, obj = (exec)->objects[0]; \
64-
index < (exec)->num_objects; \
65-
++index, obj = (exec)->objects[index])
76+
#define drm_exec_for_each_locked_object(exec, index, obj) \
77+
for ((index) = 0; ((obj) = drm_exec_obj(exec, index)); ++(index))
78+
79+
/**
80+
* drm_exec_for_each_locked_object_reverse - iterate over all the locked
81+
* objects in reverse locking order
82+
* @exec: drm_exec object
83+
* @index: unsigned long index for the iteration
84+
* @obj: the current GEM object
85+
*
86+
* Iterate over all the locked GEM objects inside the drm_exec object in
87+
* reverse locking order. Note that @index may go below zero and wrap,
88+
* but that will be caught by drm_exec_obj(), returning a NULL object.
89+
*/
90+
#define drm_exec_for_each_locked_object_reverse(exec, index, obj) \
91+
for ((index) = (exec)->num_objects - 1; \
92+
((obj) = drm_exec_obj(exec, index)); --(index))
6693

6794
/**
6895
* drm_exec_until_all_locked - loop until all GEM objects are locked

0 commit comments

Comments
 (0)