Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Locking mismatch and missing error handling in `xe_ttm_bo_release_notify` lead to deadlocks, state corruption, and kernel panics for imported dma-buf objects. --
commit 654aea229ee4882cf8f04774d507ece22e8921fd Author: Natalie Vock <[email protected]> drm/xe: remove workaround for TTM internals This commit removes the xe_ttm_bo_lock_in_destructor() workaround and replaces it with a WARN_ON() check in xe_ttm_bo_release_notify(). > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c > index 5843f850339c7..34eae56716076 100644 > --- a/drivers/gpu/drm/xe/xe_bo.c > +++ b/drivers/gpu/drm/xe/xe_bo.c [ ... ] > @@ -1680,8 +1655,11 @@ static void xe_ttm_bo_release_notify(struct > ttm_buffer_object *ttm_bo) > bo = ttm_to_xe_bo(ttm_bo); > xe_assert(xe_bo_device(bo), !(bo->created && > kref_read(&ttm_bo->base.refcount))); > > - if (!xe_ttm_bo_lock_in_destructor(ttm_bo)) > - return; > + /* > + * This should never fail since there are no other references to the BO > + * any more. > + */ > + WARN_ON(!dma_resv_trylock(ttm_bo->base.resv)); [Severity: Critical] Since WARN_ON() does not return on failure, if the trylock fails, won't execution fall through and modify fences without holding the lock? Also, there appears to be a mismatch in the resv pointer being locked and unlocked. We are locking ttm_bo->base.resv here, but later in this same function it acts on ttm_bo->base._resv: dma_resv_for_each_fence(&cursor, &ttm_bo->base._resv, DMA_RESV_USAGE_BOOKKEEP, fence) { ... } dma_resv_unlock(&ttm_bo->base._resv); For imported dma-buf objects, base.resv might not equal &base._resv. Could this lock the exporter's shared resv but attempt to unlock the local embedded _resv? This might cause a deadlock on the shared resv and an attempt to unlock an unheld mutex. > > /* > * Scrub the preempt fences if any. The unbind fence is already -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
