Skip to content

Commit 2518202

Browse files
Darksonnjannau
authored andcommitted
gpuvm: remove gem.gpuva.lock_dep_map
Since all users of gem.gpuva.lock_dep_map now rely on the mutex directly in gpuva, we may remove it. Whether the mutex is used is now tracked by a flag in gpuvm rather than by whether lock_dep_map is null. Note that a GEM object may not be pushed to multiple gpuvms that disagree on the value of this new flag. But that's okay because a single driver should use the same locking scheme everywhere, and a GEM object is driver specific (when a GEM is exported with prime, a new GEM object instance is created from the backing dma-buf). The flag is present even with CONFIG_LOCKDEP=n because the intent is that the flag will also cause vm_bo cleanup to become deferred. However, that will happen in a follow-up patch. Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20250827-gpuva-mutex-in-gem-v3-3-bd89f5a82c0d@google.com [ Use lockdep_is_held() instead of lock_is_held(). - Danilo ] Signed-off-by: Danilo Krummrich <dakr@kernel.org>
1 parent a158281 commit 2518202

5 files changed

Lines changed: 59 additions & 48 deletions

File tree

drivers/gpu/drm/drm_gpuvm.c

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,7 @@
432432
* DRM GPUVM also does not take care of the locking of the backing
433433
* &drm_gem_object buffers GPU VA lists and &drm_gpuvm_bo abstractions by
434434
* itself; drivers are responsible to enforce mutual exclusion using either the
435-
* GEMs dma_resv lock or alternatively a driver specific external lock. For the
436-
* latter see also drm_gem_gpuva_set_lock().
435+
* GEMs dma_resv lock or the GEMs gpuva.lock mutex.
437436
*
438437
* However, DRM GPUVM contains lockdep checks to ensure callers of its API hold
439438
* the corresponding lock whenever the &drm_gem_objects GPU VA list is accessed
@@ -1518,7 +1517,7 @@ drm_gpuvm_bo_destroy(struct kref *kref)
15181517
drm_gpuvm_bo_list_del(vm_bo, extobj, lock);
15191518
drm_gpuvm_bo_list_del(vm_bo, evict, lock);
15201519

1521-
drm_gem_gpuva_assert_lock_held(obj);
1520+
drm_gem_gpuva_assert_lock_held(gpuvm, obj);
15221521
list_del(&vm_bo->list.entry.gem);
15231522

15241523
if (ops && ops->vm_bo_free)
@@ -1539,7 +1538,8 @@ drm_gpuvm_bo_destroy(struct kref *kref)
15391538
* If the reference count drops to zero, the &gpuvm_bo is destroyed, which
15401539
* includes removing it from the GEMs gpuva list. Hence, if a call to this
15411540
* function can potentially let the reference count drop to zero the caller must
1542-
* hold the dma-resv or driver specific GEM gpuva lock.
1541+
* hold the lock that the GEM uses for its gpuva list (either the GEM's
1542+
* dma-resv or gpuva.lock mutex).
15431543
*
15441544
* This function may only be called from non-atomic context.
15451545
*
@@ -1563,7 +1563,7 @@ __drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
15631563
{
15641564
struct drm_gpuvm_bo *vm_bo;
15651565

1566-
drm_gem_gpuva_assert_lock_held(obj);
1566+
drm_gem_gpuva_assert_lock_held(gpuvm, obj);
15671567
drm_gem_for_each_gpuvm_bo(vm_bo, obj)
15681568
if (vm_bo->vm == gpuvm)
15691569
return vm_bo;
@@ -1622,7 +1622,7 @@ drm_gpuvm_bo_obtain(struct drm_gpuvm *gpuvm,
16221622
if (!vm_bo)
16231623
return ERR_PTR(-ENOMEM);
16241624

1625-
drm_gem_gpuva_assert_lock_held(obj);
1625+
drm_gem_gpuva_assert_lock_held(gpuvm, obj);
16261626
list_add_tail(&vm_bo->list.entry.gem, &obj->gpuva.list);
16271627

16281628
return vm_bo;
@@ -1658,7 +1658,7 @@ drm_gpuvm_bo_obtain_prealloc(struct drm_gpuvm_bo *__vm_bo)
16581658
return vm_bo;
16591659
}
16601660

1661-
drm_gem_gpuva_assert_lock_held(obj);
1661+
drm_gem_gpuva_assert_lock_held(gpuvm, obj);
16621662
list_add_tail(&__vm_bo->list.entry.gem, &obj->gpuva.list);
16631663

16641664
return __vm_bo;
@@ -1830,8 +1830,7 @@ EXPORT_SYMBOL_GPL(drm_gpuva_remove);
18301830
* reference of the latter is taken.
18311831
*
18321832
* This function expects the caller to protect the GEM's GPUVA list against
1833-
* concurrent access using either the GEMs dma_resv lock or a driver specific
1834-
* lock set through drm_gem_gpuva_set_lock().
1833+
* concurrent access using either the GEM's dma-resv or gpuva.lock mutex.
18351834
*/
18361835
void
18371836
drm_gpuva_link(struct drm_gpuva *va, struct drm_gpuvm_bo *vm_bo)
@@ -1846,7 +1845,7 @@ drm_gpuva_link(struct drm_gpuva *va, struct drm_gpuvm_bo *vm_bo)
18461845

18471846
va->vm_bo = drm_gpuvm_bo_get(vm_bo);
18481847

1849-
drm_gem_gpuva_assert_lock_held(obj);
1848+
drm_gem_gpuva_assert_lock_held(gpuvm, obj);
18501849
list_add_tail(&va->gem.entry, &vm_bo->list.gpuva);
18511850
}
18521851
EXPORT_SYMBOL_GPL(drm_gpuva_link);
@@ -1866,8 +1865,7 @@ EXPORT_SYMBOL_GPL(drm_gpuva_link);
18661865
* the latter is dropped.
18671866
*
18681867
* This function expects the caller to protect the GEM's GPUVA list against
1869-
* concurrent access using either the GEMs dma_resv lock or a driver specific
1870-
* lock set through drm_gem_gpuva_set_lock().
1868+
* concurrent access using either the GEM's dma-resv or gpuva.lock mutex.
18711869
*/
18721870
void
18731871
drm_gpuva_unlink(struct drm_gpuva *va)
@@ -1878,7 +1876,7 @@ drm_gpuva_unlink(struct drm_gpuva *va)
18781876
if (unlikely(!obj))
18791877
return;
18801878

1881-
drm_gem_gpuva_assert_lock_held(obj);
1879+
drm_gem_gpuva_assert_lock_held(va->vm, obj);
18821880
list_del_init(&va->gem.entry);
18831881

18841882
va->vm_bo = NULL;
@@ -2888,8 +2886,8 @@ EXPORT_SYMBOL_GPL(drm_gpuvm_bo_unmap);
28882886
* After the caller finished processing the returned &drm_gpuva_ops, they must
28892887
* be freed with &drm_gpuva_ops_free.
28902888
*
2891-
* It is the callers responsibility to protect the GEMs GPUVA list against
2892-
* concurrent access using the GEMs dma_resv lock.
2889+
* This function expects the caller to protect the GEM's GPUVA list against
2890+
* concurrent access using either the GEM's dma-resv or gpuva.lock mutex.
28932891
*
28942892
* Returns: a pointer to the &drm_gpuva_ops on success, an ERR_PTR on failure
28952893
*/
@@ -2901,7 +2899,7 @@ drm_gpuvm_bo_unmap_ops_create(struct drm_gpuvm_bo *vm_bo)
29012899
struct drm_gpuva *va;
29022900
int ret;
29032901

2904-
drm_gem_gpuva_assert_lock_held(vm_bo->obj);
2902+
drm_gem_gpuva_assert_lock_held(vm_bo->vm, vm_bo->obj);
29052903

29062904
ops = kzalloc(sizeof(*ops), GFP_KERNEL);
29072905
if (!ops)

drivers/gpu/drm/panthor/panthor_gem.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,6 @@ struct drm_gem_object *panthor_gem_create_object(struct drm_device *ddev, size_t
245245

246246
obj->base.base.funcs = &panthor_gem_funcs;
247247
obj->base.map_wc = !ptdev->coherent;
248-
drm_gem_gpuva_set_lock(&obj->base.base, &obj->base.base.gpuva.lock);
249248
mutex_init(&obj->label.lock);
250249

251250
panthor_gem_debugfs_bo_init(obj);

drivers/gpu/drm/panthor/panthor_mmu.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2385,8 +2385,9 @@ panthor_vm_create(struct panthor_device *ptdev, bool for_mcu,
23852385
* to be handled the same way user VMAs are.
23862386
*/
23872387
drm_gpuvm_init(&vm->base, for_mcu ? "panthor-MCU-VM" : "panthor-GPU-VM",
2388-
DRM_GPUVM_RESV_PROTECTED, &ptdev->base, dummy_gem,
2389-
min_va, va_range, 0, 0, &panthor_gpuvm_ops);
2388+
DRM_GPUVM_RESV_PROTECTED | DRM_GPUVM_IMMEDIATE_MODE,
2389+
&ptdev->base, dummy_gem, min_va, va_range, 0, 0,
2390+
&panthor_gpuvm_ops);
23902391
drm_gem_object_put(dummy_gem);
23912392
return vm;
23922393

include/drm/drm_gem.h

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,12 @@ struct drm_gem_object {
399399

400400
/**
401401
* @gpuva: Fields used by GPUVM to manage mappings pointing to this GEM object.
402+
*
403+
* When DRM_GPUVM_IMMEDIATE_MODE is set, this list is protected by the
404+
* mutex. Otherwise, the list is protected by the GEMs &dma_resv lock.
405+
*
406+
* Note that all entries in this list must agree on whether
407+
* DRM_GPUVM_IMMEDIATE_MODE is set.
402408
*/
403409
struct {
404410
/**
@@ -412,17 +418,14 @@ struct drm_gem_object {
412418

413419
/**
414420
* @gpuva.lock: lock protecting access to &drm_gem_object.gpuva.list
415-
* when the resv lock can't be used.
421+
* when DRM_GPUVM_IMMEDIATE_MODE is used.
416422
*
417-
* Should only be used when the VM is being modified in a fence
418-
* signalling path, otherwise you should use &drm_gem_object.resv to
419-
* protect accesses to &drm_gem_object.gpuva.list.
423+
* Only used when DRM_GPUVM_IMMEDIATE_MODE is set. It should be
424+
* safe to take this mutex during the fence signalling path, so
425+
* do not allocate memory while holding this lock. Otherwise,
426+
* the &dma_resv lock should be used.
420427
*/
421428
struct mutex lock;
422-
423-
#ifdef CONFIG_LOCKDEP
424-
struct lockdep_map *lock_dep_map;
425-
#endif
426429
} gpuva;
427430

428431
/**
@@ -607,26 +610,12 @@ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj)
607610
}
608611

609612
#ifdef CONFIG_LOCKDEP
610-
/**
611-
* drm_gem_gpuva_set_lock() - Set the lock protecting accesses to the gpuva list.
612-
* @obj: the &drm_gem_object
613-
* @lock: the lock used to protect the gpuva list. The locking primitive
614-
* must contain a dep_map field.
615-
*
616-
* Call this if you're not proctecting access to the gpuva list with the
617-
* dma-resv lock, but with a custom lock.
618-
*/
619-
#define drm_gem_gpuva_set_lock(obj, lock) \
620-
if (!WARN((obj)->gpuva.lock_dep_map, \
621-
"GEM GPUVA lock should be set only once.")) \
622-
(obj)->gpuva.lock_dep_map = &(lock)->dep_map
623-
#define drm_gem_gpuva_assert_lock_held(obj) \
624-
lockdep_assert((obj)->gpuva.lock_dep_map ? \
625-
lock_is_held((obj)->gpuva.lock_dep_map) : \
613+
#define drm_gem_gpuva_assert_lock_held(gpuvm, obj) \
614+
lockdep_assert(drm_gpuvm_immediate_mode(gpuvm) ? \
615+
lockdep_is_held(&(obj)->gpuva.lock) : \
626616
dma_resv_held((obj)->resv))
627617
#else
628-
#define drm_gem_gpuva_set_lock(obj, lock) do {} while (0)
629-
#define drm_gem_gpuva_assert_lock_held(obj) do {} while (0)
618+
#define drm_gem_gpuva_assert_lock_held(gpuvm, obj) do {} while (0)
630619
#endif
631620

632621
/**

include/drm/drm_gpuvm.h

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,20 @@ enum drm_gpuvm_flags {
214214
*/
215215
DRM_GPUVM_RESV_PROTECTED = BIT(0),
216216

217+
/**
218+
* @DRM_GPUVM_IMMEDIATE_MODE: use the locking scheme for GEMs designed
219+
* for modifying the GPUVM during the fence signalling path
220+
*
221+
* When set, gpuva.lock is used to protect gpuva.list in all GEM
222+
* objects associated with this GPUVM. Otherwise, the GEMs dma-resv is
223+
* used.
224+
*/
225+
DRM_GPUVM_IMMEDIATE_MODE = BIT(1),
226+
217227
/**
218228
* @DRM_GPUVM_USERBITS: user defined bits
219229
*/
220-
DRM_GPUVM_USERBITS = BIT(1),
230+
DRM_GPUVM_USERBITS = BIT(2),
221231
};
222232

223233
/**
@@ -387,6 +397,19 @@ drm_gpuvm_resv_protected(struct drm_gpuvm *gpuvm)
387397
return gpuvm->flags & DRM_GPUVM_RESV_PROTECTED;
388398
}
389399

400+
/**
401+
* drm_gpuvm_immediate_mode() - indicates whether &DRM_GPUVM_IMMEDIATE_MODE is
402+
* set
403+
* @gpuvm: the &drm_gpuvm
404+
*
405+
* Returns: true if &DRM_GPUVM_IMMEDIATE_MODE is set, false otherwise.
406+
*/
407+
static inline bool
408+
drm_gpuvm_immediate_mode(struct drm_gpuvm *gpuvm)
409+
{
410+
return gpuvm->flags & DRM_GPUVM_IMMEDIATE_MODE;
411+
}
412+
390413
/**
391414
* drm_gpuvm_resv() - returns the &drm_gpuvm's &dma_resv
392415
* @gpuvm__: the &drm_gpuvm
@@ -760,9 +783,10 @@ drm_gpuvm_bo_gem_evict(struct drm_gem_object *obj, bool evict)
760783
{
761784
struct drm_gpuvm_bo *vm_bo;
762785

763-
drm_gem_gpuva_assert_lock_held(obj);
764-
drm_gem_for_each_gpuvm_bo(vm_bo, obj)
786+
drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
787+
drm_gem_gpuva_assert_lock_held(vm_bo->vm, obj);
765788
drm_gpuvm_bo_evict(vm_bo, evict);
789+
}
766790
}
767791

768792
void drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo);

0 commit comments

Comments
 (0)