[PATCH 1/4] drm/radeon: add extra check in radeon_ttm_tt_unpin_userptr

[PATCH 1/4] drm/radeon: add extra check in radeon_ttm_tt_unpin_userptr

[Christian König]

From: Christian König <christian.koenig@amd.com>

We somehow try to free the SG table twice.

Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=89734

Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: <stable@vger.kernel.org>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index d02aa1d..b292aca 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -598,6 +598,10 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
 	enum dma_data_direction direction = write ?
 		DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
 
+	/* double check that we don't free the table twice */
+	if (!ttm->sg->sgl)
+		return;
+
 	/* free the sg table and pages again */
 	dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/radeon: fix wait in radeon_mn_invalidate_range_start

[Christian König]

From: Christian König <christian.koenig@amd.com>

We need to wait for all fences, not just the exclusive one.

Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: <stable@vger.kernel.org>
---
 drivers/gpu/drm/radeon/radeon_mn.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
index a69bd44..572b4db 100644
--- a/drivers/gpu/drm/radeon/radeon_mn.c
+++ b/drivers/gpu/drm/radeon/radeon_mn.c
@@ -122,7 +122,6 @@ 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;
-		struct fence *fence;
 		int r;
 
 		bo = container_of(it, struct radeon_bo, mn_it);
@@ -134,12 +133,10 @@ static void radeon_mn_invalidate_range_start(struct mmu_notifier *mn,
 			continue;
 		}
 
-		fence = reservation_object_get_excl(bo->tbo.resv);
-		if (fence) {
-			r = radeon_fence_wait((struct radeon_fence *)fence, false);
-			if (r)
-				DRM_ERROR("(%d) failed to wait for user bo\n", r);
-		}
+		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);
 
 		radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU);
 		r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/radeon: add userptr config option

[Christian König]

From: Christian König <christian.koenig@amd.com>

This allows selecting CONFIG_MMU_NOTIFIER if it isn't already selected.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/radeon/Kconfig | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index 970f8e9..421ae13 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -1,3 +1,11 @@
+config DRM_RADEON_USERPTR
+	bool "Always enable userptr support"
+	depends on DRM_RADEON
+	select MMU_NOTIFIER
+	help
+	  This option selects CONFIG_MMU_NOTIFIER if it isn't already
+	  selected to enabled full userptr support.
+
 config DRM_RADEON_UMS
 	bool "Enable userspace modesetting on radeon (DEPRECATED)"
 	depends on DRM_RADEON
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/radeon: allow creating overlapping userptrs

[Christian König]

From: Christian König <christian.koenig@amd.com>

Similar to the Intel implementation, but instead of just falling back to a
global linear list when we have an overlapping userptr request we accumulate
all overlapping userptrs in a local list.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/radeon/radeon.h    |   2 +-
 drivers/gpu/drm/radeon/radeon_mn.c | 102 +++++++++++++++++++++++++++----------
 2 files changed, 76 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 35ab65d..a0c272e 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -507,7 +507,7 @@ struct radeon_bo {
 	pid_t				pid;
 
 	struct radeon_mn		*mn;
-	struct interval_tree_node	mn_it;
+	struct list_head		mn_list;
 };
 #define gem_to_radeon_bo(gobj) container_of((gobj), struct radeon_bo, gem_base)
 
diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
index 572b4db..0170137 100644
--- a/drivers/gpu/drm/radeon/radeon_mn.c
+++ b/drivers/gpu/drm/radeon/radeon_mn.c
@@ -53,6 +53,11 @@ struct radeon_mn {
 	struct rb_root		objects;
 };
 
+struct radeon_mn_node {
+	struct interval_tree_node	it;
+	struct list_head		bos;
+};
+
 /**
  * radeon_mn_destroy - destroy the rmn
  *
@@ -64,14 +69,21 @@ static void radeon_mn_destroy(struct work_struct *work)
 {
 	struct radeon_mn *rmn = container_of(work, struct radeon_mn, work);
 	struct radeon_device *rdev = rmn->rdev;
-	struct radeon_bo *bo, *next;
+	struct radeon_mn_node *node, *next_node;
+	struct radeon_bo *bo, *next_bo;
 
 	mutex_lock(&rdev->mn_lock);
 	mutex_lock(&rmn->lock);
 	hash_del(&rmn->node);
-	rbtree_postorder_for_each_entry_safe(bo, next, &rmn->objects, mn_it.rb) {
-		interval_tree_remove(&bo->mn_it, &rmn->objects);
-		bo->mn = NULL;
+	rbtree_postorder_for_each_entry_safe(node, next_node, &rmn->objects,
+					     it.rb) {
+
+		interval_tree_remove(&node->it, &rmn->objects);
+		list_for_each_entry_safe(bo, next_bo, &node->bos, mn_list) {
+			bo->mn = NULL;
+			list_del_init(&bo->mn_list);
+		}
+		kfree(node);
 	}
 	mutex_unlock(&rmn->lock);
 	mutex_unlock(&rdev->mn_lock);
@@ -121,29 +133,33 @@ static void radeon_mn_invalidate_range_start(struct mmu_notifier *mn,
 
 	it = interval_tree_iter_first(&rmn->objects, start, end);
 	while (it) {
+		struct radeon_mn_node *node;
 		struct radeon_bo *bo;
 		int r;
 
-		bo = container_of(it, struct radeon_bo, mn_it);
+		node = container_of(it, struct radeon_mn_node, 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);
-			continue;
-		}
+		list_for_each_entry(bo, &node->bos, mn_list) {
 
-		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);
+			r = radeon_bo_reserve(bo, true);
+			if (r) {
+				DRM_ERROR("(%d) failed to reserve user bo\n", r);
+				continue;
+			}
 
-		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);
+			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);
 
-		radeon_bo_unreserve(bo);
+			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);
+
+			radeon_bo_unreserve(bo);
+		}
 	}
 	
 	mutex_unlock(&rmn->lock);
@@ -220,24 +236,44 @@ int radeon_mn_register(struct radeon_bo *bo, unsigned long addr)
 	unsigned long end = addr + radeon_bo_size(bo) - 1;
 	struct radeon_device *rdev = bo->rdev;
 	struct radeon_mn *rmn;
+	struct radeon_mn_node *node = NULL;
+	struct list_head bos;
 	struct interval_tree_node *it;
 
 	rmn = radeon_mn_get(rdev);
 	if (IS_ERR(rmn))
 		return PTR_ERR(rmn);
 
+	INIT_LIST_HEAD(&bos);
+
 	mutex_lock(&rmn->lock);
 
-	it = interval_tree_iter_first(&rmn->objects, addr, end);
-	if (it) {
-		mutex_unlock(&rmn->lock);
-		return -EEXIST;
+	while ((it = interval_tree_iter_first(&rmn->objects, addr, end))) {
+		kfree(node);
+		node = container_of(it, struct radeon_mn_node, it);
+		interval_tree_remove(&node->it, &rmn->objects);
+		addr = min(it->start, addr);
+		end = max(it->last, end);
+		list_splice(&node->bos, &bos);
+	}
+
+	if (!node) {
+		node = kmalloc(sizeof(struct radeon_mn_node), GFP_KERNEL);
+		if (!node) {
+			mutex_unlock(&rmn->lock);
+			return -ENOMEM;
+		}
 	}
 
 	bo->mn = rmn;
-	bo->mn_it.start = addr;
-	bo->mn_it.last = end;
-	interval_tree_insert(&bo->mn_it, &rmn->objects);
+
+	node->it.start = addr;
+	node->it.last = end;
+	INIT_LIST_HEAD(&node->bos);
+	list_splice(&bos, &node->bos);
+	list_add(&bo->mn_list, &node->bos);
+
+	interval_tree_insert(&node->it, &rmn->objects);
 
 	mutex_unlock(&rmn->lock);
 
@@ -255,6 +291,7 @@ void radeon_mn_unregister(struct radeon_bo *bo)
 {
 	struct radeon_device *rdev = bo->rdev;
 	struct radeon_mn *rmn;
+	struct list_head *head;
 
 	mutex_lock(&rdev->mn_lock);
 	rmn = bo->mn;
@@ -264,8 +301,19 @@ void radeon_mn_unregister(struct radeon_bo *bo)
 	}
 
 	mutex_lock(&rmn->lock);
-	interval_tree_remove(&bo->mn_it, &rmn->objects);
+	/* save the next list entry for later */
+	head = bo->mn_list.next;
+
 	bo->mn = NULL;
+	list_del(&bo->mn_list);
+
+	if (list_empty(head)) {
+		struct radeon_mn_node *node;
+		node = container_of(head, struct radeon_mn_node, bos);
+		interval_tree_remove(&node->it, &rmn->objects);
+		kfree(node);
+	}
+
 	mutex_unlock(&rmn->lock);
 	mutex_unlock(&rdev->mn_lock);
 }
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/radeon: add extra check in radeon_ttm_tt_unpin_userptr

[Alex Deucher]

On Tue, Mar 31, 2015 at 11:36 AM, Christian König
<deathsimple@vodafone.de> wrote:
> From: Christian König <christian.koenig@amd.com>
>
> We somehow try to free the SG table twice.
>
> Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=89734
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: <stable@vger.kernel.org>

For the series:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

I've added the first two to my -fixes tree.

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index d02aa1d..b292aca 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -598,6 +598,10 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
>         enum dma_data_direction direction = write ?
>                 DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
>
> +       /* double check that we don't free the table twice */
> +       if (!ttm->sg->sgl)
> +               return;
> +
>         /* free the sg table and pages again */
>         dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
>
> --
> 1.9.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/radeon: add extra check in radeon_ttm_tt_unpin_userptr

[Christian König]

On 31.03.2015 18:13, Alex Deucher wrote:
> On Tue, Mar 31, 2015 at 11:36 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> We somehow try to free the SG table twice.
>>
>> Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=89734
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Cc: <stable@vger.kernel.org>
> For the series:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> I've added the first two to my -fixes tree.

Could you add the other two to your -next tree? You probably need to 
merge with -fixes to do so.

Regards,
Christian.

>
> Alex
>
>> ---
>>   drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>> index d02aa1d..b292aca 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>> @@ -598,6 +598,10 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm)
>>          enum dma_data_direction direction = write ?
>>                  DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
>>
>> +       /* double check that we don't free the table twice */
>> +       if (!ttm->sg->sgl)
>> +               return;
>> +
>>          /* free the sg table and pages again */
>>          dma_unmap_sg(rdev->dev, ttm->sg->sgl, ttm->sg->nents, direction);
>>
>> --
>> 1.9.1
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel