Am 16.03.2017 um 08:46 schrieb Zhang, Jerry:
-----Original Message-----
From: Christian König [mailto:deathsim...@vodafone.de]
Sent: Thursday, March 16, 2017 15:33
To: Zhang, Jerry; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu: insert partial mappings before and after
directly
Am 16.03.2017 um 04:44 schrieb Junwei Zhang:
Currently it may miss one page before or after the target mapping
I don't think that this will work correctly. The interval tree still contains
the old
mapping at this point and as far as I know inserting overlapping mappings is not
allowed here.
Ah, I missed that, so this insert op should be after free up.
I will revise the patch
But what do you mean with miss one page before or after the target mapping?
I did a unit test to verify it and found that:
If the before mapping is 1 page size, so its start and last will be same.
Thus below condition will become false then to free the before mapping.
> if (before->it.start != before->it.last)
But in this case, we need the before mapping of 1 page size.
So does after mapping.
e.g.
Initialize a mapping: [0, 8] pages size.
A replace mapping: [1, 2] page size.
Before mapping is [0, 1], before.it.start = 0, before.it = 1 - 1 = 0.
Additionally, if we create a bo as 1 page size, the corresponding mapping also
has start==last, I think.
Ups, that's indeed not correct.
Yeah we need a better criteria if we should insert the ranges or not.
Christian.
Jerry.
Christian.
Signed-off-by: Junwei Zhang <jerry.zh...@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f7c02a9..511c6c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1767,7 +1767,11 @@ int amdgpu_vm_bo_clear_mappings(struct
amdgpu_device *adev,
before->it.last = saddr - 1;
before->offset = tmp->offset;
before->flags = tmp->flags;
+
list_add(&before->list, &tmp->list);
+ interval_tree_insert(&before->it, &vm->va);
+ if (before->flags & AMDGPU_PTE_PRT)
+ amdgpu_vm_prt_get(adev);
}
/* Remember mapping split at the end */ @@ -1777,7 +1781,11
@@ int
amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
after->offset = tmp->offset;
after->offset += after->it.start - tmp->it.start;
after->flags = tmp->flags;
+
list_add(&after->list, &tmp->list);
+ interval_tree_insert(&after->it, &vm->va);
+ if (after->flags & AMDGPU_PTE_PRT)
+ amdgpu_vm_prt_get(adev);
}
list_del(&tmp->list);
@@ -1799,24 +1807,6 @@ int amdgpu_vm_bo_clear_mappings(struct
amdgpu_device *adev,
trace_amdgpu_vm_bo_unmap(NULL, tmp);
}
- /* Insert partial mapping before the range*/
- if (before->it.start != before->it.last) {
- interval_tree_insert(&before->it, &vm->va);
- if (before->flags & AMDGPU_PTE_PRT)
- amdgpu_vm_prt_get(adev);
- } else {
- kfree(before);
- }
-
- /* Insert partial mapping after the range */
- if (after->it.start != after->it.last) {
- interval_tree_insert(&after->it, &vm->va);
- if (after->flags & AMDGPU_PTE_PRT)
- amdgpu_vm_prt_get(adev);
- } else {
- kfree(after);
- }
-
return 0;
}
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
