[PATCH 1/3] drm/ttm: add ttm_bo_move_to_lru_tail function

[PATCH 1/3] drm/ttm: add ttm_bo_move_to_lru_tail function

[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.

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

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 745e996..9e91cc3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -228,6 +228,26 @@ 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 (!list_empty(&bo->swap)) {
+		list_del(&bo->swap);
+		list_add_tail(&bo->swap, &bo->glob->swap_lru);
+	}
+
+	if (!list_empty(&bo->lru)) {
+		man = &bdev->man[bo->mem.mem_type];
+		list_del(&bo->lru);
+		list_add_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

[PATCH 2/3] drm/amdgpu: move VM page tables to the LRU end on CS

[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.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 10 ++++++++--
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index f7dc9ca..72504bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -987,7 +987,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
 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_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+			  struct list_head *duplicates);
 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 cb9c93c..f532f70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -419,7 +419,7 @@ static int amdgpu_cs_parser_relocs(struct amdgpu_cs_parser *p)
 	if (unlikely(r != 0))
 		goto error_reserve;
 
-	amdgpu_vm_get_pt_bos(&fpriv->vm, &duplicates);
+	amdgpu_vm_get_pt_bos(p->adev, &fpriv->vm, &duplicates);
 
 	r = amdgpu_cs_list_validate(p->adev, &fpriv->vm, &p->validated);
 	if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index c9f26df..e3bce52 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -467,7 +467,7 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 	if (r)
 		goto error_print;
 
-	amdgpu_vm_get_pt_bos(bo_va->vm, &duplicates);
+	amdgpu_vm_get_pt_bos(adev, bo_va->vm, &duplicates);
 	list_for_each_entry(entry, &list, head) {
 		domain = amdgpu_mem_type_to_domain(entry->bo->mem.mem_type);
 		/* if anything is swapped out don't swap it in here,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f3b07bf..5ff742f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -106,19 +106,25 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
  * Add the page directory to the BO duplicates list
  * for command submission.
  */
-void amdgpu_vm_get_pt_bos(struct amdgpu_vm *vm, struct list_head *duplicates)
+void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+			  struct list_head *duplicates)
 {
+	struct ttm_bo_global *glob = adev->mman.bdev.glob;
 	unsigned i;
 
-	/* add the vm page table to the list */
+	/* add the vm page table to the list and move them to the LRU tail */
+	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);
 		list_add(&entry->tv.head, duplicates);
 	}
+
+	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 3/3] 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 f532f70..0868921 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(p->adev, &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/3] drm/ttm: add ttm_bo_move_to_lru_tail function

[Alex Deucher]

On Tue, Jan 5, 2016 at 7:16 AM, Christian König <deathsimple@vodafone.de> 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.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Chunming Zhou <David1.Zhou@amd.com>

This series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Unless there are any objections to the ttm patch, I'd like to pull
this series into my drm-next tree.

Alex

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 20 ++++++++++++++++++++
>  include/drm/ttm/ttm_bo_api.h | 10 ++++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 745e996..9e91cc3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -228,6 +228,26 @@ 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 (!list_empty(&bo->swap)) {
> +               list_del(&bo->swap);
> +               list_add_tail(&bo->swap, &bo->glob->swap_lru);
> +       }
> +
> +       if (!list_empty(&bo->lru)) {
> +               man = &bdev->man[bo->mem.mem_type];
> +               list_del(&bo->lru);
> +               list_add_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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Christian König]

Am 05.01.2016 um 22:34 schrieb Alex Deucher:
> On Tue, Jan 5, 2016 at 7:16 AM, Christian König <deathsimple@vodafone.de> 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.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Reviewed-by: Chunming Zhou <David1.Zhou@amd.com>
> This series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Unless there are any objections to the ttm patch, I'd like to pull
> this series into my drm-next tree.

I have objections!

Ok seriously, David and I have found quite a bug around that in TTM. We 
need to fix that before we can push this patch.

Christian.

>
> Alex
>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c | 20 ++++++++++++++++++++
>>   include/drm/ttm/ttm_bo_api.h | 10 ++++++++++
>>   2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 745e996..9e91cc3 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -228,6 +228,26 @@ 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 (!list_empty(&bo->swap)) {
>> +               list_del(&bo->swap);
>> +               list_add_tail(&bo->swap, &bo->glob->swap_lru);
>> +       }
>> +
>> +       if (!list_empty(&bo->lru)) {
>> +               man = &bdev->man[bo->mem.mem_type];
>> +               list_del(&bo->lru);
>> +               list_add_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

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

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

[Alex Deucher]

On Fri, Jan 8, 2016 at 4:33 AM, Christian König <deathsimple@vodafone.de> wrote:
> Am 05.01.2016 um 22:34 schrieb Alex Deucher:
>>
>> On Tue, Jan 5, 2016 at 7:16 AM, Christian König <deathsimple@vodafone.de>
>> 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.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> Reviewed-by: Chunming Zhou <David1.Zhou@amd.com>
>>
>> This series is:
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> Unless there are any objections to the ttm patch, I'd like to pull
>> this series into my drm-next tree.
>
>
> I have objections!
>
> Ok seriously, David and I have found quite a bug around that in TTM. We need
> to fix that before we can push this patch.

I've already included this in my most recent fixes pull request to
Dave.  Should I ask him not to pull and drop this set or is the other
patch you just sent enough to fix it?

Thanks,

Alex


>
> Christian.
>
>
>>
>> Alex
>>
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_bo.c | 20 ++++++++++++++++++++
>>>   include/drm/ttm/ttm_bo_api.h | 10 ++++++++++
>>>   2 files changed, 30 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>>> index 745e996..9e91cc3 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -228,6 +228,26 @@ 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 (!list_empty(&bo->swap)) {
>>> +               list_del(&bo->swap);
>>> +               list_add_tail(&bo->swap, &bo->glob->swap_lru);
>>> +       }
>>> +
>>> +       if (!list_empty(&bo->lru)) {
>>> +               man = &bdev->man[bo->mem.mem_type];
>>> +               list_del(&bo->lru);
>>> +               list_add_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
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel