[PATCH 1/5] drm/ttm: fix adding foreign BOs to the LRU during init v2

[PATCH 1/5] drm/ttm: fix adding foreign BOs to the LRU during init v2

[Christian König]

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

If we import a BO with an external reservation object we don't
reserve/unreserve it. So we never add it to the LRU causing a possible
deny of service.

v2: fix typo in commit message

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

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 745e996..a98a5d5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1170,9 +1170,15 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
 	if (likely(!ret))
 		ret = ttm_bo_validate(bo, placement, interruptible, false);
 
-	if (!resv)
+	if (!resv) {
 		ttm_bo_unreserve(bo);
 
+	} else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+		spin_lock(&bo->glob->lru_lock);
+		ttm_bo_add_to_lru(bo);
+		spin_unlock(&bo->glob->lru_lock);
+	}
+
 	if (unlikely(ret))
 		ttm_bo_unref(&bo);
 
-- 
2.5.0

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

[PATCH 2/5] drm/ttm: fix adding foreign BOs to the swap LRU

[Christian König]

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

It doesn't make any sense to try to swap out imported BOs.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a98a5d5..3ea9a01 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -176,7 +176,7 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
 		list_add_tail(&bo->lru, &man->lru);
 		kref_get(&bo->list_kref);
 
-		if (bo->ttm != NULL) {
+		if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
 			list_add_tail(&bo->swap, &bo->glob->swap_lru);
 			kref_get(&bo->list_kref);
 		}
-- 
2.5.0

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

Re: [PATCH 2/5] drm/ttm: fix adding foreign BOs to the swap LRU

[Thomas Hellstrom]

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

BTW, does Radeon touch the caching mode of imported buffers through TTM
(like write-combining)?
Do we need something similar for that?

/Thomas


On 01/11/2016 03:35 PM, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> It doesn't make any sense to try to swap out imported BOs.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index a98a5d5..3ea9a01 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -176,7 +176,7 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
>  		list_add_tail(&bo->lru, &man->lru);
>  		kref_get(&bo->list_kref);
>  
> -		if (bo->ttm != NULL) {
> +		if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>  			list_add_tail(&bo->swap, &bo->glob->swap_lru);
>  			kref_get(&bo->list_kref);
>  		}



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

[PATCH 3/5] drm/ttm: add ttm_bo_move_to_lru_tail function v2

[Christian König]

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

This allows the drivers to move a BO to the end of the LRU
without removing and adding it again.

v2: Make it more robust, handle pinned and swapable BOs as well.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 21 +++++++++++++++++++++
 include/drm/ttm/ttm_bo_api.h | 10 ++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3ea9a01..4cbf265 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -228,6 +228,27 @@ void ttm_bo_del_sub_from_lru(struct ttm_buffer_object *bo)
 }
 EXPORT_SYMBOL(ttm_bo_del_sub_from_lru);
 
+void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo)
+{
+	struct ttm_bo_device *bdev = bo->bdev;
+	struct ttm_mem_type_manager *man;
+
+	lockdep_assert_held(&bo->resv->lock.base);
+
+	if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
+		list_del_init(&bo->swap);
+		list_del_init(&bo->lru);
+
+	} else {
+		if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG))
+			list_move_tail(&bo->swap, &bo->glob->swap_lru);
+
+		man = &bdev->man[bo->mem.mem_type];
+		list_move_tail(&bo->lru, &man->lru);
+	}
+}
+EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
+
 /*
  * Call bo->mutex locked.
  */
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index c768ddf..afae231 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -383,6 +383,16 @@ extern void ttm_bo_add_to_lru(struct ttm_buffer_object *bo);
  */
 extern int ttm_bo_del_from_lru(struct ttm_buffer_object *bo);
 
+/**
+ * ttm_bo_move_to_lru_tail
+ *
+ * @bo: The buffer object.
+ *
+ * Move this BO to the tail of all lru lists used to lookup and reserve an
+ * object. This function must be called with struct ttm_bo_global::lru_lock
+ * held, and is used to make a BO less likely to be considered for eviction.
+ */
+extern void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo);
 
 /**
  * ttm_bo_lock_delayed_workqueue
-- 
2.5.0

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

Re: [PATCH 3/5] drm/ttm: add ttm_bo_move_to_lru_tail function v2

[Thomas Hellstrom]

LGTM,

Although perhaps check that the LRU lock is held as well.

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

On 01/11/2016 03:35 PM, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> This allows the drivers to move a BO to the end of the LRU
> without removing and adding it again.
>
> v2: Make it more robust, handle pinned and swapable BOs as well.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 21 +++++++++++++++++++++
>  include/drm/ttm/ttm_bo_api.h | 10 ++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3ea9a01..4cbf265 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -228,6 +228,27 @@ void ttm_bo_del_sub_from_lru(struct ttm_buffer_object *bo)
>  }
>  EXPORT_SYMBOL(ttm_bo_del_sub_from_lru);
>  
> +void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo)
> +{
> +	struct ttm_bo_device *bdev = bo->bdev;
> +	struct ttm_mem_type_manager *man;
> +
> +	lockdep_assert_held(&bo->resv->lock.base);
> +
> +	if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
> +		list_del_init(&bo->swap);
> +		list_del_init(&bo->lru);
> +
> +	} else {
> +		if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG))
> +			list_move_tail(&bo->swap, &bo->glob->swap_lru);
> +
> +		man = &bdev->man[bo->mem.mem_type];
> +		list_move_tail(&bo->lru, &man->lru);
> +	}
> +}
> +EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
> +
>  /*
>   * Call bo->mutex locked.
>   */
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index c768ddf..afae231 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -383,6 +383,16 @@ extern void ttm_bo_add_to_lru(struct ttm_buffer_object *bo);
>   */
>  extern int ttm_bo_del_from_lru(struct ttm_buffer_object *bo);
>  
> +/**
> + * ttm_bo_move_to_lru_tail
> + *
> + * @bo: The buffer object.
> + *
> + * Move this BO to the tail of all lru lists used to lookup and reserve an
> + * object. This function must be called with struct ttm_bo_global::lru_lock
> + * held, and is used to make a BO less likely to be considered for eviction.
> + */
> +extern void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo);
>  
>  /**
>   * ttm_bo_lock_delayed_workqueue


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

[PATCH 4/5] drm/amdgpu: move VM page tables to the LRU end on CS v2

[Christian König]

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

This makes it less likely to run into an ENOMEM because
VM page tables are evicted last.

v2: move the BOs in the LRU tail after validation

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  7 ++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 27 +++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 003959f..313b0cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -987,6 +987,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
 			 struct list_head *validated,
 			 struct amdgpu_bo_list_entry *entry);
 void amdgpu_vm_get_pt_bos(struct amdgpu_vm *vm, struct list_head *duplicates);
+void amdgpu_vm_move_pt_bos_in_lru(struct amdgpu_device *adev,
+				  struct amdgpu_vm *vm);
 int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
 		      struct amdgpu_sync *sync);
 void amdgpu_vm_flush(struct amdgpu_ring *ring,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index ce0254d..1fffc33 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -428,8 +428,10 @@ static int amdgpu_cs_parser_relocs(struct amdgpu_cs_parser *p)
 	r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &duplicates);
 
 error_validate:
-	if (r)
+	if (r) {
+		amdgpu_vm_move_pt_bos_in_lru(p->adev, &fpriv->vm);
 		ttm_eu_backoff_reservation(&p->ticket, &p->validated);
+	}
 
 error_reserve:
 	if (need_mmap_lock)
@@ -473,8 +475,11 @@ static int cmp_size_smaller_first(void *priv, struct list_head *a,
  **/
 static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, bool backoff)
 {
+	struct amdgpu_fpriv *fpriv = parser->filp->driver_priv;
 	unsigned i;
 
+	amdgpu_vm_move_pt_bos_in_lru(parser->adev, &fpriv->vm);
+
 	if (!error) {
 		/* Sort the buffer list from the smallest to largest buffer,
 		 * which affects the order of buffers in the LRU list.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8f7688e..aefc668 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -119,6 +119,33 @@ void amdgpu_vm_get_pt_bos(struct amdgpu_vm *vm, struct list_head *duplicates)
 
 		list_add(&entry->tv.head, duplicates);
 	}
+
+}
+
+/**
+ * amdgpu_vm_move_pt_bos_in_lru - move the PT BOs to the LRU tail
+ *
+ * @adev: amdgpu device instance
+ * @vm: vm providing the BOs
+ *
+ * Move the PT BOs to the tail of the LRU.
+ */
+void amdgpu_vm_move_pt_bos_in_lru(struct amdgpu_device *adev,
+				  struct amdgpu_vm *vm)
+{
+	struct ttm_bo_global *glob = adev->mman.bdev.glob;
+	unsigned i;
+
+	spin_lock(&glob->lru_lock);
+	for (i = 0; i <= vm->max_pde_used; ++i) {
+		struct amdgpu_bo_list_entry *entry = &vm->page_tables[i].entry;
+
+		if (!entry->robj)
+			continue;
+
+		ttm_bo_move_to_lru_tail(&entry->robj->tbo);
+	}
+	spin_unlock(&glob->lru_lock);
 }
 
 /**
-- 
2.5.0

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

[PATCH 5/5] drm/amdgpu: validate duplicates first

[Christian König]

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

Most VM BOs end up in the duplicates list, validate it
first make -ENOMEM less likely.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 1fffc33..6f89f8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -421,11 +421,11 @@ static int amdgpu_cs_parser_relocs(struct amdgpu_cs_parser *p)
 
 	amdgpu_vm_get_pt_bos(&fpriv->vm, &duplicates);
 
-	r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &p->validated);
+	r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &duplicates);
 	if (r)
 		goto error_validate;
 
-	r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &duplicates);
+	r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &p->validated);
 
 error_validate:
 	if (r) {
-- 
2.5.0

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

Re: [PATCH 5/5] drm/amdgpu: validate duplicates first

[Alex Deucher]

On Mon, Jan 11, 2016 at 9:35 AM, Christian König
<deathsimple@vodafone.de> wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Most VM BOs end up in the duplicates list, validate it
> first make -ENOMEM less likely.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Chunming Zhou <David1.Zhou@amd.com>

Applied the series.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 1fffc33..6f89f8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -421,11 +421,11 @@ static int amdgpu_cs_parser_relocs(struct amdgpu_cs_parser *p)
>
>         amdgpu_vm_get_pt_bos(&fpriv->vm, &duplicates);
>
> -       r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &p->validated);
> +       r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &duplicates);
>         if (r)
>                 goto error_validate;
>
> -       r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &duplicates);
> +       r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &p->validated);
>
>  error_validate:
>         if (r) {
> --
> 2.5.0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/ttm: fix adding foreign BOs to the LRU during init v2

[Thomas Hellstrom]

LGTM, minor nitpick below.

Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

On 01/11/2016 03:35 PM, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> If we import a BO with an external reservation object we don't
> reserve/unreserve it. So we never add it to the LRU causing a possible
> deny of service.
s/deny/denial/

>
> v2: fix typo in commit message
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 745e996..a98a5d5 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1170,9 +1170,15 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
>  	if (likely(!ret))
>  		ret = ttm_bo_validate(bo, placement, interruptible, false);
>  
> -	if (!resv)
> +	if (!resv) {
>  		ttm_bo_unreserve(bo);
>  
> +	} else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
> +		spin_lock(&bo->glob->lru_lock);
> +		ttm_bo_add_to_lru(bo);
> +		spin_unlock(&bo->glob->lru_lock);
> +	}
> +
>  	if (unlikely(ret))
>  		ttm_bo_unref(&bo);
>  


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