On 6/15/2026 4:08 PM, Christian König wrote:
On 6/12/26 15:20, Huang, Honglei wrote:
On 6/12/2026 8:02 PM, Christian König wrote:
On 6/12/26 11:09, Huang Rui wrote:
From: Honglei Huang <[email protected]>
...
+static inline void amdgpu_svm_assert_locked(struct amdgpu_svm *svm)
+{
+ lockdep_assert_held_write(&svm->svm_lock);
+}
What exactly is that lock protecting?
this lock is the driver_svm_lock required by the drm_gpusvm framework
It is registered by drm_gpusvm_driver_set_lock(), and drm_gpusvm lockdep
asserts it on every structural entry point, e.g.
drm_gpusvm_range_find_or_insert() / drm_gpusvm_range_remove().
Per amdgpu_svm it serializes the write/commit side against the fault
handler: range/notifier tree insert+remove, attribute changes, and the
garbage collector.
This is the same thing xe does, in drivers/gpu/drm/xe/xe_svm.c:
drm_gpusvm_driver_set_lock(&vm->svm.gpusvm, &vm->lock);
This is clearly incorrect in that case. Our equivalent in amdgpu is
vm->eviction_lock.
That was already completely incorrect in the old KFD implementation, please
don't use that one as blueprint.
I really want to use vm->eviction_lock as SVM primary lock,
but it seems like in Xe, Xe uses vm->lock as an outer VM lock by design
rwsem instead of mutex lock to broad VM ownership, not an eviction lock,
they are semantically different.
I tried to replace the svm lock with eviction lock locally, ABBA dead
lock encountered:
amdgpu_svm_handle_fault
amdgpu_svm_lock (A: eviction_lock)
fault_map_range
amdgpu_svm_range_update_mapping
drm_gpusvm_notifier_lock (B)
drm_gpusvm_notifier_invalidate
down_write(B - notifier_lock)
amdgpu_svm_invalidate
amdgpu_svm_range_invalidate
amdgpu_svm_range_notifier_event_begin
amdgpu_svm_range_zap_ptes
amdgpu_vm_update_range
amdgpu_vm_eviction_lock (A: eviction_lock)
the current SVM side can switch its registered driver lock at any time
easily as you wish, but doing so safely is not just an amdgpu SVM local
change.
The issue is that change lock / change lock registration is easy, while
lock semantics are not. SVM execution paths like fault handling,
invalidation, mapping updates, garbage collection, notifier callbacks...
are coupled with VM locking and update flows follow the xe svm style. If
we switch SVM to a different VM lock now, we can introduce lock
recursion, ABBA lock...
SVM can technically switch lock registration now.
But make all stack works and pass all tests maybe needs amdgpu VM
refactoring.
Regards,
Honglei
Regards,
Christian.
