From: Christian König christian.koenig@amd.com
If we unmap BOs before releasing them them the intervall tree locks up because we try to remove an entry not inside the tree.
Signed-off-by: Christian König christian.koenig@amd.com CC: stable@vger.kernel.org --- drivers/gpu/drm/radeon/radeon_vm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c index 2a5a4a9..16d8e95 100644 --- a/drivers/gpu/drm/radeon/radeon_vm.c +++ b/drivers/gpu/drm/radeon/radeon_vm.c @@ -1107,7 +1107,8 @@ void radeon_vm_bo_rmv(struct radeon_device *rdev, list_del(&bo_va->bo_list);
mutex_lock(&vm->mutex); - interval_tree_remove(&bo_va->it, &vm->va); + if (bo_va->it.start || bo_va->it.last) + interval_tree_remove(&bo_va->it, &vm->va); spin_lock(&vm->status_lock); list_del(&bo_va->vm_status);
From: Christian König christian.koenig@amd.com
Otherwise it is possible that we will have page table corruption if we change a BOs address multiple times.
Signed-off-by: Christian König christian.koenig@amd.com CC: stable@vger.kernel.org --- drivers/gpu/drm/radeon/radeon_vm.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c index 16d8e95..cabcb0a 100644 --- a/drivers/gpu/drm/radeon/radeon_vm.c +++ b/drivers/gpu/drm/radeon/radeon_vm.c @@ -490,6 +490,8 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev, spin_lock(&vm->status_lock); list_add(&tmp->vm_status, &vm->freed); spin_unlock(&vm->status_lock); + + bo_va->addr = 0; }
interval_tree_remove(&bo_va->it, &vm->va);
From: Christian König christian.koenig@amd.com
Otherwise the change isn't atomic.
Signed-off-by: Christian König christian.koenig@amd.com CC: stable@vger.kernel.org --- drivers/gpu/drm/radeon/radeon_vm.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c index cabcb0a..de42fc4 100644 --- a/drivers/gpu/drm/radeon/radeon_vm.c +++ b/drivers/gpu/drm/radeon/radeon_vm.c @@ -473,6 +473,23 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev, }
mutex_lock(&vm->mutex); + soffset /= RADEON_GPU_PAGE_SIZE; + eoffset /= RADEON_GPU_PAGE_SIZE; + if (soffset || eoffset) { + struct interval_tree_node *it; + it = interval_tree_iter_first(&vm->va, soffset, eoffset - 1); + if (it && it != &bo_va->it) { + struct radeon_bo_va *tmp; + tmp = container_of(it, struct radeon_bo_va, it); + /* bo and tmp overlap, invalid offset */ + dev_err(rdev->dev, "bo %p va 0x%010Lx conflict with " + "(bo %p 0x%010lx 0x%010lx)\n", bo_va->bo, + soffset, tmp->bo, tmp->it.start, tmp->it.last); + mutex_unlock(&vm->mutex); + return -EINVAL; + } + } + if (bo_va->it.start || bo_va->it.last) { if (bo_va->addr) { /* add a clone of the bo_va to clear the old address */ @@ -499,21 +516,7 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev, bo_va->it.last = 0; }
- soffset /= RADEON_GPU_PAGE_SIZE; - eoffset /= RADEON_GPU_PAGE_SIZE; if (soffset || eoffset) { - struct interval_tree_node *it; - it = interval_tree_iter_first(&vm->va, soffset, eoffset - 1); - if (it) { - struct radeon_bo_va *tmp; - tmp = container_of(it, struct radeon_bo_va, it); - /* bo and tmp overlap, invalid offset */ - dev_err(rdev->dev, "bo %p va 0x%010Lx conflict with " - "(bo %p 0x%010lx 0x%010lx)\n", bo_va->bo, - soffset, tmp->bo, tmp->it.start, tmp->it.last); - mutex_unlock(&vm->mutex); - return -EINVAL; - } bo_va->it.start = soffset; bo_va->it.last = eoffset - 1; interval_tree_insert(&bo_va->it, &vm->va);
From: Christian König christian.koenig@amd.com
Otherwise we print false warning from time to time.
Signed-off-by: Christian König christian.koenig@amd.com Signed-off-by: Jack Xiao Jack.Xiao@amd.com CC: stable@vger.kernel.org --- drivers/gpu/drm/radeon/radeon_mn.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c index 572b4db..30e6207 100644 --- a/drivers/gpu/drm/radeon/radeon_mn.c +++ b/drivers/gpu/drm/radeon/radeon_mn.c @@ -122,26 +122,26 @@ static void radeon_mn_invalidate_range_start(struct mmu_notifier *mn, it = interval_tree_iter_first(&rmn->objects, start, end); while (it) { struct radeon_bo *bo; - int r; + long r;
bo = container_of(it, struct radeon_bo, mn_it); it = interval_tree_iter_next(it, start, end);
r = radeon_bo_reserve(bo, true); if (r) { - DRM_ERROR("(%d) failed to reserve user bo\n", r); + DRM_ERROR("(%ld) failed to reserve user bo\n", r); continue; }
r = reservation_object_wait_timeout_rcu(bo->tbo.resv, true, false, MAX_SCHEDULE_TIMEOUT); - if (r) - DRM_ERROR("(%d) failed to wait for user bo\n", r); + if (r <= 0) + DRM_ERROR("(%ld) failed to wait for user bo\n", r);
radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU); r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false); if (r) - DRM_ERROR("(%d) failed to validate user bo\n", r); + DRM_ERROR("(%ld) failed to validate user bo\n", r);
radeon_bo_unreserve(bo); }
On Mon, Apr 27, 2015 at 11:04 AM, Christian König deathsimple@vodafone.de wrote:
From: Christian König christian.koenig@amd.com
Otherwise we print false warning from time to time.
Signed-off-by: Christian König christian.koenig@amd.com Signed-off-by: Jack Xiao Jack.Xiao@amd.com CC: stable@vger.kernel.org
Applied the series to my 4.1 fixes tree. I had to make minor changes to 4/4 so that it would apply: http://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-fixes-4.1-wip
Alex
drivers/gpu/drm/radeon/radeon_mn.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c index 572b4db..30e6207 100644 --- a/drivers/gpu/drm/radeon/radeon_mn.c +++ b/drivers/gpu/drm/radeon/radeon_mn.c @@ -122,26 +122,26 @@ static void radeon_mn_invalidate_range_start(struct mmu_notifier *mn, it = interval_tree_iter_first(&rmn->objects, start, end); while (it) { struct radeon_bo *bo;
int r;
long r; bo = container_of(it, struct radeon_bo, mn_it); it = interval_tree_iter_next(it, start, end); r = radeon_bo_reserve(bo, true); if (r) {
DRM_ERROR("(%d) failed to reserve user bo\n", r);
DRM_ERROR("(%ld) failed to reserve user bo\n", r); continue; } r = reservation_object_wait_timeout_rcu(bo->tbo.resv, true, false, MAX_SCHEDULE_TIMEOUT);
if (r)DRM_ERROR("(%d) failed to wait for user bo\n", r);
if (r <= 0)DRM_ERROR("(%ld) failed to wait for user bo\n", r); radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU); r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false); if (r)
DRM_ERROR("(%d) failed to validate user bo\n", r);
DRM_ERROR("(%ld) failed to validate user bo\n", r); radeon_bo_unreserve(bo); }-- 1.9.1
On 27.04.2015 17:04, Christian König wrote:
From: Christian König christian.koenig@amd.com
If we unmap BOs before releasing them them the intervall tree locks up because we try to remove an entry not inside the tree.
Signed-off-by: Christian König christian.koenig@amd.com CC: stable@vger.kernel.org
Somehow forgot to mention that Michel is the original author of this patch, I just fixed two additional lines which caused list corruption.
Regards, Christian.
drivers/gpu/drm/radeon/radeon_vm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c index 2a5a4a9..16d8e95 100644 --- a/drivers/gpu/drm/radeon/radeon_vm.c +++ b/drivers/gpu/drm/radeon/radeon_vm.c @@ -1107,7 +1107,8 @@ void radeon_vm_bo_rmv(struct radeon_device *rdev, list_del(&bo_va->bo_list);
mutex_lock(&vm->mutex);
- interval_tree_remove(&bo_va->it, &vm->va);
- if (bo_va->it.start || bo_va->it.last)
interval_tree_remove(&bo_va->it, &vm->va);
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König