Skip to content

Commit 0c0816d

Browse files
committed
drm/gem: fix lockdep check for dma-resv lock
When no custom lock is set to protect a GEMs GPUVA list, lockdep checks should fall back to the GEM objects dma-resv lock. With the current implementation we're setting the lock_dep_map of the GEM objects 'resv' pointer (in case no custom lock_dep_map is set yet) on drm_gem_private_object_init(). However, the GEM objects 'resv' pointer might still change after drm_gem_private_object_init() is called, e.g. through ttm_bo_init_reserved(). This can result in the wrong lock being tracked. To fix this, call dma_resv_held() directly from drm_gem_gpuva_assert_lock_held() and fall back to the GEMs lock_dep_map pointer only if an actual custom lock is set. Fixes: e6303f3 ("drm: manager to keep track of GPUs VA mappings") Reviewed-by: Dave Airlie <airlied@redhat.com> Signed-off-by: Danilo Krummrich <dakr@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230804182406.5222-2-dakr@redhat.com
1 parent a5ae331 commit 0c0816d

1 file changed

Lines changed: 9 additions & 6 deletions

File tree

include/drm/drm_gem.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -551,15 +551,17 @@ int drm_gem_evict(struct drm_gem_object *obj);
551551
* @lock: the lock used to protect the gpuva list. The locking primitive
552552
* must contain a dep_map field.
553553
*
554-
* Call this if you're not proctecting access to the gpuva list
555-
* with the dma-resv lock, otherwise, drm_gem_gpuva_init() takes care
556-
* of initializing lock_dep_map for you.
554+
* Call this if you're not proctecting access to the gpuva list with the
555+
* dma-resv lock, but with a custom lock.
557556
*/
558557
#define drm_gem_gpuva_set_lock(obj, lock) \
559-
if (!(obj)->gpuva.lock_dep_map) \
558+
if (!WARN((obj)->gpuva.lock_dep_map, \
559+
"GEM GPUVA lock should be set only once.")) \
560560
(obj)->gpuva.lock_dep_map = &(lock)->dep_map
561561
#define drm_gem_gpuva_assert_lock_held(obj) \
562-
lockdep_assert(lock_is_held((obj)->gpuva.lock_dep_map))
562+
lockdep_assert((obj)->gpuva.lock_dep_map ? \
563+
lock_is_held((obj)->gpuva.lock_dep_map) : \
564+
dma_resv_held((obj)->resv))
563565
#else
564566
#define drm_gem_gpuva_set_lock(obj, lock) do {} while (0)
565567
#define drm_gem_gpuva_assert_lock_held(obj) do {} while (0)
@@ -573,11 +575,12 @@ int drm_gem_evict(struct drm_gem_object *obj);
573575
*
574576
* Calling this function is only necessary for drivers intending to support the
575577
* &drm_driver_feature DRIVER_GEM_GPUVA.
578+
*
579+
* See also drm_gem_gpuva_set_lock().
576580
*/
577581
static inline void drm_gem_gpuva_init(struct drm_gem_object *obj)
578582
{
579583
INIT_LIST_HEAD(&obj->gpuva.list);
580-
drm_gem_gpuva_set_lock(obj, &obj->resv->lock.base);
581584
}
582585

583586
/**

0 commit comments

Comments
 (0)