I've pushed the patch to drm-misc-fixes and are now going to push it to amd-staging-drm-next as well.
Thanks, Christian. On 10/8/26 18:17, vitaly prosyak wrote: > Hi Christian, > > Thanks for the fix. Tested under the same conditions that previously > reproduced the crash at dma-resv.c:329; the crash did not reproduce > with this patch > 2026-10-03T16:00:58-04:00 Intel-system kernel: kernel BUG at > drivers/dma-buf/dma-resv.c:329! > 2026-10-03T16:00:58-04:00 Intel-system kernel: Oops: invalid opcode: 0000 > [#1] SMP NOPTI > 2026-10-03T16:00:58-04:00 Intel-system kernel: CPU: 11 UID: 0 PID: 477 Comm: > kworker/11:2 Not tainted 7.1.0+ #83 PREEMPT(full) > 2026-10-03T16:00:58-04:00 Intel-system kernel: Hardware name: ASUS System > Product Name/ROG MAXIMUS Z790 HERO, BIOS 2703 10/17/2024 > 2026-10-03T16:00:58-04:00 Intel-system kernel: Workqueue: events > amdgpu_userq_restore_worker [amdgpu] > 2026-10-03T16:00:58-04:00 Intel-system kernel: RIP: > 0010:dma_resv_add_fence+0x2d6/0x2f0 > 2026-10-03T16:00:58-04:00 Intel-system kernel: Code: 44 8b 88 30 0a 00 00 48 > 05 18 0d 00 00 50 4c 8b 45 08 48 8b 75 b8 e8 f9 01 35 ff 58 41 8b 45 14 41 39 > 45 10 0f 82 d2 fe ff ff <0f> 0b 31 d2 45 31 ff e9 ba fe ff ff e8 c9 a5 5c 00 > 66 0f 1f 84 00 > 2026-10-03T16:00:58-04:00 Intel-system kernel: RSP: 0018:ffffcb8682e5fb28 > EFLAGS: 00010246 > 2026-10-03T16:00:58-04:00 Intel-system kernel: RAX: 0000000000000000 RBX: > 0000000000000003 RCX: 0000000000000000 > 2026-10-03T16:00:58-04:00 Intel-system kernel: RDX: 0000000000000000 RSI: > 0000000000000000 RDI: 0000000000000000 > 2026-10-03T16:00:58-04:00 Intel-system kernel: RBP: ffffcb8682e5fb78 R08: > 0000000000000000 R09: 0000000000000000 > 2026-10-03T16:00:58-04:00 Intel-system kernel: R10: 0000000000000000 R11: > 0000000000000000 R12: ffff88edd40ca500 > 2026-10-03T16:00:58-04:00 Intel-system kernel: R13: ffff88ed91d83f00 R14: > ffffffffc1c3ee60 R15: 0000000000000000 > 2026-10-03T16:00:58-04:00 Intel-system kernel: FS: 0000000000000000(0000) > GS:ffff88f53e2bc000(0000) knlGS:0000000000000000 > 2026-10-03T16:00:58-04:00 Intel-system kernel: CS: 0010 DS: 0000 ES: 0000 > CR0: 0000000080050033 > 2026-10-03T16:00:58-04:00 Intel-system kernel: CR2: 0000756822dd7000 CR3: > 000000012e1d4006 CR4: 0000000000f72ef0 > 2026-10-03T16:00:58-04:00 Intel-system kernel: PKRU: 55555554 > 2026-10-03T16:00:58-04:00 Intel-system kernel: Call Trace: > 2026-10-03T16:00:58-04:00 Intel-system kernel: <TASK> > 2026-10-03T16:00:58-04:00 Intel-system kernel: > amdgpu_evf_mgr_attach_fence+0x179/0x330 [amdgpu] > 2026-10-03T16:00:58-04:00 Intel-system kernel: > amdgpu_evf_mgr_rearm+0x1ff/0x2b0 [amdgpu] > 2026-10-03T16:00:58-04:00 Intel-system kernel: > amdgpu_userq_vm_validate_and_restore_queue+0x8f0/0xc80 [amdgpu] > 2026-10-03T16:00:58-04:00 Intel-system kernel: > amdgpu_userq_restore_worker+0x64/0xb0 [amdgpu] > 2026-10-03T16:00:58-04:00 Intel-system kernel: process_one_work+0x22e/0x780 > 2026-10-03T16:00:58-04:00 Intel-system kernel: worker_thread+0x1db/0x3d0 > 2026-10-03T16:00:58-04:00 Intel-system kernel: ? > __pfx_worker_thread+0x10/0x10 > 2026-10-03T16:00:58-04:00 Intel-system kernel: kthread+0xfe/0x140 > 2026-10-03T16:00:58-04:00 Intel-system kernel: ? __pfx_kthread+0x10/0x10 > 2026-10-03T16:00:58-04:00 Intel-system kernel: ret_from_fork+0x3bd/0x470 > 2026-10-03T16:00:58-04:00 Intel-system kernel: ? __pfx_kthread+0x10/0x10 > 2026-10-03T16:00:58-04:00 Intel-system kernel: ret_from_fork_asm+0x1a/0x30 > 2026-10-03T16:00:58-04:00 Intel-system kernel: </TASK> > > > My patch preserved the fence-slot request across retries. Your approach > instead excludes the implicit prelocked BO from iteration until explicitly > requested again, avoiding the extra reservation-count bookkeeping. > > A few minor wording corrections: > > - "keeping the prelocked object of the array" should be > "keeping the prelocked object out of the array". > - "Should the prelocked object never been mentioned again" should be > "If the prelocked object is never mentioned again". > - "might never given to" should be "might never be passed to". > - "unecessarily" should be "unnecessarily". > > Tested-by: Vitaly Prosyak <[email protected]> > Reviewed-by: Vitaly Prosyak <[email protected]> > > Thanks, > Vitaly > > On 2026-10-07 08:46, Christian König wrote: >> When a contention is detected the drm_exec object grabs a reference to >> the contended object, unlocks all other objects and pre-locks the >> contended one before anything else. >> >> The problem is that it is possible that this pre-locked object was about >> to be destroyed and drm_exec is now holding the last reference to it. >> >> So we never actually see this pre-locked object again to be explicitly >> locked and so also never reserve a slot for a dma_fence object on it. >> >> Should the driver now use drm_exec_for_each_locked_object() to add a new >> fence to all locked objects we eventually see a warning or even a >> BUG_ON() to prevent random memory corruption. >> >> Fix this by keeping the prelocked object of the array of locked objs >> until it was explicitly mentioned by the driver again. >> >> Should the prelocked object never been mentioned again by the driver >> just unlock and drop the reference at the end of the locking loop. >> >> Signed-off-by: Christian König <[email protected]> >> Reported-by: Vitaly Prosyak <[email protected]> >> Fixes: 09593216bff1 ("drm: execution context for GEM buffers v7") >> CC: [email protected] # v6.6+ >> --- >> drivers/gpu/drm/drm_exec.c | 73 ++++++++++++++++++++++---------------- >> include/drm/drm_exec.h | 15 ++++---- >> 2 files changed, 52 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c >> index 2453ec41360f4..a910b0ad17ce4 100644 >> --- a/drivers/gpu/drm/drm_exec.c >> +++ b/drivers/gpu/drm/drm_exec.c >> @@ -48,6 +48,16 @@ >> * See struct dma_exec for more details. >> */ >> >> +/* Unlock and put the prelocked obj */ >> +static void drm_exec_drop_prelocked(struct drm_exec *exec) >> +{ >> + if (exec->prelocked) { >> + dma_resv_unlock(exec->prelocked->resv); >> + drm_gem_object_put(exec->prelocked); >> + exec->prelocked = NULL; >> + } >> +} >> + >> /* Unlock all objects and drop references */ >> static void drm_exec_unlock_all(struct drm_exec *exec) >> { >> @@ -58,8 +68,7 @@ static void drm_exec_unlock_all(struct drm_exec *exec) >> drm_gem_object_put(obj); >> } >> >> - drm_gem_object_put(exec->prelocked); >> - exec->prelocked = NULL; >> + drm_exec_drop_prelocked(exec); >> } >> >> /** >> @@ -118,6 +127,14 @@ EXPORT_SYMBOL(drm_exec_fini); >> bool drm_exec_cleanup(struct drm_exec *exec) >> { >> if (likely(!exec->contended)) { >> + >> + /* >> + * The contended and prelocked object might never given to >> + * drm_exec_lock_obj() again. This can happen when the object >> + * is about to be cleaned up and we now hold the last reference >> + * to it. Make sure that we don't unecessarily keep it around. >> + */ >> + drm_exec_drop_prelocked(exec); >> ww_acquire_done(&exec->ticket); >> return false; >> } >> @@ -175,16 +192,16 @@ static int drm_exec_lock_contended(struct drm_exec >> *exec) >> dma_resv_lock_slow(obj->resv, &exec->ticket); >> } >> >> - ret = drm_exec_obj_locked(exec, obj); >> - if (unlikely(ret)) >> - goto error_unlock; >> + /* >> + * It is perfectly possible that we see another contention before the >> + * first one is fully handled. >> + */ >> + drm_exec_drop_prelocked(exec); >> >> + /* We move the reference from contended to prelocked here */ >> exec->prelocked = obj; >> return 0; >> >> -error_unlock: >> - dma_resv_unlock(obj->resv); >> - >> error_dropref: >> drm_gem_object_put(obj); >> return ret; >> @@ -212,35 +229,31 @@ int drm_exec_lock_obj(struct drm_exec *exec, struct >> drm_gem_object *obj) >> if (exec->prelocked == obj) { >> drm_gem_object_put(exec->prelocked); >> exec->prelocked = NULL; >> - return 0; >> - } >> + } else { >> + if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT) >> + ret = dma_resv_lock_interruptible(obj->resv, >> + &exec->ticket); >> + else >> + ret = dma_resv_lock(obj->resv, &exec->ticket); >> + >> + if (unlikely(ret == -EDEADLK)) { >> + drm_gem_object_get(obj); >> + exec->contended = obj; >> + return -EDEADLK; >> + } >> >> - if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT) >> - ret = dma_resv_lock_interruptible(obj->resv, &exec->ticket); >> - else >> - ret = dma_resv_lock(obj->resv, &exec->ticket); >> + if (unlikely(ret == -EALREADY) && >> + exec->flags & DRM_EXEC_IGNORE_DUPLICATES) >> + return 0; >> >> - if (unlikely(ret == -EDEADLK)) { >> - drm_gem_object_get(obj); >> - exec->contended = obj; >> - return -EDEADLK; >> + if (unlikely(ret)) >> + return ret; >> } >> >> - if (unlikely(ret == -EALREADY) && >> - exec->flags & DRM_EXEC_IGNORE_DUPLICATES) >> - return 0; >> - >> - if (unlikely(ret)) >> - return ret; >> - >> ret = drm_exec_obj_locked(exec, obj); >> if (ret) >> - goto error_unlock; >> - >> - return 0; >> + dma_resv_unlock(obj->resv); >> >> -error_unlock: >> - dma_resv_unlock(obj->resv); >> return ret; >> } >> EXPORT_SYMBOL(drm_exec_lock_obj); >> diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h >> index cc2937185a9f7..c92d69a2c793d 100644 >> --- a/include/drm/drm_exec.h >> +++ b/include/drm/drm_exec.h >> @@ -73,13 +73,15 @@ drm_exec_obj(struct drm_exec *exec, unsigned long index) >> >> /* Helper for drm_exec_for_each_locked_object(). Internal use only. */ >> #define __drm_exec_for_each_locked_object(exec, obj, __index) >> \ >> - for (unsigned long __index = 0; ((obj) = drm_exec_obj(exec, __index)); >> ++__index) >> + for (unsigned long __index = 0; ((obj) = drm_exec_obj(exec, __index)); \ >> + ++__index) >> /** >> * drm_exec_for_each_locked_object - iterate over all the locked objects >> * @exec: drm_exec object >> * @obj: the current GEM object >> * >> - * Iterate over all the locked GEM objects inside the drm_exec object. >> + * Iterate over all the explicitly locked GEM objects inside the drm_exec >> + * container, except for the contended and implicit prelocked one. >> */ >> #define drm_exec_for_each_locked_object(exec, obj) \ >> __drm_exec_for_each_locked_object(exec, obj, __UNIQUE_ID(drm_exec)) >> @@ -94,12 +96,13 @@ drm_exec_obj(struct drm_exec *exec, unsigned long index) >> * @exec: drm_exec object >> * @obj: the current GEM object >> * >> - * Iterate over all the locked GEM objects inside the drm_exec object in >> - * reverse locking order. Note that the internal index may wrap around, >> - * but that will be caught by drm_exec_obj(), returning a NULL object. >> + * Iterate over all the explicitly locked GEM objects inside the drm_exec >> + * container in reverse locking order. Note that the internal index may wrap >> + * around, but that will be caught by drm_exec_obj(), returning a NULL >> object. >> */ >> #define drm_exec_for_each_locked_object_reverse(exec, obj) \ >> - __drm_exec_for_each_locked_object_reverse(exec, obj, >> __UNIQUE_ID(drm_exec)) >> + __drm_exec_for_each_locked_object_reverse(exec, obj, \ >> + __UNIQUE_ID(drm_exec)) >> >> /** >> * drm_exec_until_all_locked - loop until all GEM objects are locked
