Hi Felix,
so coming back to this after two weeks of distraction.
Am 14.02.20 um 22:12 schrieb Felix Kuehling:
Now you allow eviction of page tables while you allocate page tables.
Isn't the whole point of the eviction lock to prevent page table
evictions while manipulating page tables?
Or does this only apply to PTE invalidations which never allocated
memory? Is that the only case that doesn't reserve page tables?
Yes, exactly. We essentially have to distinct two cases here:
1. We are validating PTEs and so eventually need to allocate page tables.
For this the root PD is reserved and we actually know that the VM
can't be evicted.
But we still need to hold the lock while pushing the actual updates
the hardware to make sure we don't mess up the data structures with
concurrent invalidations.
2. We are invalidating PTEs.
Here we might or might not have the root PD reserved and so need to
make sure that nobody is evicting page tables while we are invalidating.
Quite a complicated dance, but of hand I also don't see much other choice.
Regards,
Christian.
Regards,
Felix
On 2020-02-12 10:14 a.m., Christian König wrote:
We need to make sure to not allocate PDs/PTs while holding
the eviction lock or otherwise we will run into lock inversion
in the MM as soon as we enable the MMU notifier.
Signed-off-by: Christian König <christian.koe...@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 31 +++++++++++++++++++++-----
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 77c400675b79..e7ab0c1e2793 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -897,27 +897,42 @@ static int amdgpu_vm_alloc_pts(struct
amdgpu_device *adev,
struct amdgpu_vm_pt *entry = cursor->entry;
struct amdgpu_bo_param bp;
struct amdgpu_bo *pt;
+ bool need_entries;
int r;
- if (cursor->level < AMDGPU_VM_PTB && !entry->entries) {
+ need_entries = cursor->level < AMDGPU_VM_PTB && !entry->entries;
+ if (!need_entries && entry->base.bo)
+ return 0;
+
+ /* We need to make sure that we don't allocate PDs/PTs while
holding the
+ * eviction lock or we run into lock recursion in the MM.
+ */
+ amdgpu_vm_eviction_unlock(vm);
+
+ if (need_entries) {
unsigned num_entries;
num_entries = amdgpu_vm_num_entries(adev, cursor->level);
entry->entries = kvmalloc_array(num_entries,
sizeof(*entry->entries),
GFP_KERNEL | __GFP_ZERO);
- if (!entry->entries)
- return -ENOMEM;
+ if (!entry->entries) {
+ r = -ENOMEM;
+ goto error_lock;
+ }
}
- if (entry->base.bo)
- return 0;
+ if (entry->base.bo) {
+ r = 0;
+ goto error_lock;
+ }
amdgpu_vm_bo_param(adev, vm, cursor->level, direct, &bp);
r = amdgpu_bo_create(adev, &bp, &pt);
+ amdgpu_vm_eviction_lock(vm);
if (r)
- return r;
+ goto error_free_pt;
/* Keep a reference to the root directory to avoid
* freeing them up in the wrong order.
@@ -936,6 +951,10 @@ static int amdgpu_vm_alloc_pts(struct
amdgpu_device *adev,
amdgpu_bo_unref(&pt);
entry->base.bo = NULL;
return r;
+
+error_lock:
+ amdgpu_vm_eviction_lock(vm);
+ return r;
}
/**
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
