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
