Skip to content

Commit 5a5588f

Browse files
Thomas Hellströmintel-lab-lkp
authored andcommitted
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 locked in a sequence. This has the odd consequence that if that first mutex is unlocked and its memory then freed, the lock alloc tracking code will 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. Related lock alloc tracking warning: [ 322.660067] ========================= [ 322.660070] WARNING: held lock freed! [ 322.660074] 6.5.0-rc7+ torvalds#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+ torvalds#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] Signed-off-by: Thomas Hellström <[email protected]>
1 parent 2cdef89 commit 5a5588f

File tree

2 files changed

+32
-5
lines changed

2 files changed

+32
-5
lines changed

drivers/gpu/drm/drm_exec.c

Lines changed: 1 addition & 1 deletion
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

Lines changed: 31 additions & 4 deletions
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_object(), 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)