On 9/10/26 02:45, [email protected] wrote: > From: Vitaly Prosyak <[email protected]> > > Root cause analysis showed that the previous workaround was fixing the > symptom in the wrong layer. > > Problem: > kernel BUG at drivers/dma-buf/dma-resv.c:319! > BUG_ON(fobj->num_fences >= fobj->max_fences) > Workqueue: events amdgpu_userq_restore_worker > > - amdgpu_evf_mgr_attach_fence() had been temporarily taught to reserve/add > fence bookkeeping internally as a workaround. > - This hid the real contract violation and made reservation policy implicit > inside a helper. > > Review clarification: > - The helper-side dma_resv_add_fence-related workaround was only an intended > proposal sent for review discussion, not the final direction. > - Final fix keeps fence-slot reservation explicit in callers and leaves the > helper free of policy decisions. > > Findings from investigation: > - drm_exec_prepare_obj() both locks and reserves fence slots. > - drm_exec_lock_obj() only locks and does not reserve slots. > - Existing attach_fence callers in the GEM/userq restore flows already reserve > TTM_NUM_MOVE_FENCES + 1 before adding the bookkeeping eviction fence. > - The WPTR mapping path in mes_userq_create_wptr_mapping() used > drm_exec_lock_obj() and then called amdgpu_evf_mgr_attach_fence(), so this > specific path could add a fence without pre-reserved space. > > Fix: > - Switch the WPTR mapping path to drm_exec_prepare_obj(..., > TTM_NUM_MOVE_FENCES + 1), restoring the caller-side reservation contract. > - Remove the helper-side reservation workaround from > amdgpu_evf_mgr_attach_fence(). > > Why this is the right layer: > - Reservation policy stays explicit at call sites that define how many fences > may be added in that execution context. > - The helper only performs validate + add_fence when called with a properly > prepared reservation object, matching the existing drm_exec model. > > Fixes: 91a358fe99d86 (drm/amdgpu: dont pin wptr bo instead use eviction fence) > > Cc: Sunil Khatri <[email protected]> > Cc: Prike Liang <[email protected]> > Cc: Christian König <[email protected]> > Cc: Alex Deucher <[email protected]> > Signed-off-by: Vitaly Prosyak <[email protected]> > Change-Id: Ibb2ec1f210a8c6e5d2a48ed9b8ec77c6dabddb7e
Reviewed-by: Christian König <[email protected]> Could be that Prike is right and patch #2 is already fixed, but if it isn't feel free to add my rb to that one as well. Thanks, Christian. > --- > drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c > b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c > index 68beccee9fc0..fe49aa4dc5ff 100644 > --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c > +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c > @@ -60,7 +60,8 @@ mes_userq_create_wptr_mapping(struct amdgpu_device *adev, > } > > obj = wptr_mapping->bo_va->base.bo; > - ret = drm_exec_lock_obj(&exec, &obj->tbo.base); > + ret = drm_exec_prepare_obj(&exec, &obj->tbo.base, > + TTM_NUM_MOVE_FENCES + 1); > drm_exec_retry_on_contention(&exec); > if (unlikely(ret)) > goto fail_lock;
