diff mbox

[7/7] drm/ttm: enable swapout of per VM BOs during allocation and allows reaping of deleted BOs

Message ID 1513766101-15993-7-git-send-email-Hongbo.He@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

He, Hongbo Dec. 20, 2017, 10:35 a.m. UTC 
Change-Id: I1e87954564f38ad298bf6e4ff88c9f26f291a62d
Signed-off-by: Roger He <Hongbo.He@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c     | 15 +++++++++++----
 drivers/gpu/drm/ttm/ttm_memory.c | 12 ++++++++----
 include/drm/ttm/ttm_bo_api.h     |  3 ++-
 3 files changed, 21 insertions(+), 9 deletions(-)

Comments

Thomas Hellström (VMware) Dec. 21, 2017, 7:58 a.m. UTC | #1 
What about

"Enable recursive locking at swapout time to make it possible to swap 
out BOs that share the same reservation object."

Is "per VM BOs" an AMD specific name?  In that case, I'd avoid using it 
in the TTM code since most people have no idea what they are and why the 
need specific treatment in TTM.

/Thomas


On 12/20/2017 11:35 AM, Roger He wrote:
> Change-Id: I1e87954564f38ad298bf6e4ff88c9f26f291a62d
> Signed-off-by: Roger He <Hongbo.He@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c     | 15 +++++++++++----
>   drivers/gpu/drm/ttm/ttm_memory.c | 12 ++++++++----
>   include/drm/ttm/ttm_bo_api.h     |  3 ++-
>   3 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 313925c..ecb8916 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1686,18 +1686,20 @@ EXPORT_SYMBOL(ttm_bo_synccpu_write_release);
>    * A buffer object shrink method that tries to swap out the first
>    * buffer object on the bo_global::swap_lru list.
>    */
> -int ttm_bo_swapout(struct ttm_bo_global *glob)
> +int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
>   {
>   	struct ttm_buffer_object *bo;
>   	int ret = -EBUSY;
> +	bool locked;
>   	unsigned i;
>   
>   	spin_lock(&glob->lru_lock);
>   	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>   		list_for_each_entry(bo, &glob->swap_lru[i], swap) {
> -			ret = reservation_object_trylock(bo->resv) ? 0 : -EBUSY;
> -			if (!ret)
> +			if (ttm_bo_evict_swapout_allowable(bo, ctx, &locked)) {
> +				ret = 0;
>   				break;
> +			}
>   		}
>   		if (!ret)
>   			break;
> @@ -1773,7 +1775,12 @@ EXPORT_SYMBOL(ttm_bo_swapout);
>   
>   void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
>   {
> -	while (ttm_bo_swapout(bdev->glob) == 0)
> +	struct ttm_operation_ctx ctx = {
> +		.interruptible = false,
> +		.no_wait_gpu = false
> +	};
> +
> +	while (ttm_bo_swapout(bdev->glob, &ctx) == 0)
>   		;
>   }
>   EXPORT_SYMBOL(ttm_bo_swapout_all);
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index 8df0755..8817b86 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -211,7 +211,7 @@ static bool ttm_zones_above_swap_target(struct ttm_mem_global *glob,
>    */
>   
>   static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
> -		       uint64_t extra)
> +			uint64_t extra, struct ttm_operation_ctx *ctx)
>   {
>   	int ret;
>   
> @@ -219,7 +219,7 @@ static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
>   
>   	while (ttm_zones_above_swap_target(glob, from_wq, extra)) {
>   		spin_unlock(&glob->lock);
> -		ret = ttm_bo_swapout(glob->bo_glob);
> +		ret = ttm_bo_swapout(glob->bo_glob, ctx);
>   		spin_lock(&glob->lock);
>   		if (unlikely(ret != 0))
>   			break;
> @@ -230,10 +230,14 @@ static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
>   
>   static void ttm_shrink_work(struct work_struct *work)
>   {
> +	struct ttm_operation_ctx ctx = {
> +		.interruptible = false,
> +		.no_wait_gpu = false
> +	};
>   	struct ttm_mem_global *glob =
>   	    container_of(work, struct ttm_mem_global, work);
>   
> -	ttm_shrink(glob, true, 0ULL);
> +	ttm_shrink(glob, true, 0ULL, &ctx);
>   }
>   
>   static int ttm_mem_init_kernel_zone(struct ttm_mem_global *glob,
> @@ -520,7 +524,7 @@ static int ttm_mem_global_alloc_zone(struct ttm_mem_global *glob,
>   			return -ENOMEM;
>   		if (unlikely(count-- == 0))
>   			return -ENOMEM;
> -		ttm_shrink(glob, false, memory + (memory >> 2) + 16);
> +		ttm_shrink(glob, false, memory + (memory >> 2) + 16, ctx);
>   	}
>   
>   	return 0;
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 24a8db7..f1c74c2 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -752,7 +752,8 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
>   		  const char __user *wbuf, char __user *rbuf,
>   		  size_t count, loff_t *f_pos, bool write);
>   
> -int ttm_bo_swapout(struct ttm_bo_global *glob);
> +int ttm_bo_swapout(struct ttm_bo_global *glob,
> +			struct ttm_operation_ctx *ctx);
>   void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
>   int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo);
>   #endif
Christian König Dec. 21, 2017, 8:18 a.m. UTC | #2 
Am 21.12.2017 um 08:58 schrieb Thomas Hellstrom:
> What about
>
> "Enable recursive locking at swapout time to make it possible to swap 
> out BOs that share the same reservation object."
>
> Is "per VM BOs" an AMD specific name?

Yes, absolutely. It's even amdgpu specific, radeon uses the same 
functionality but a bit different and not for user mode allocations.

> In that case, I'd avoid using it in the TTM code since most people 
> have no idea what they are and why the need specific treatment in TTM.

Good point, going to keep that in mind when reviewing the patch set.

Christian.

>
> /Thomas
>
>
> On 12/20/2017 11:35 AM, Roger He wrote:
>> Change-Id: I1e87954564f38ad298bf6e4ff88c9f26f291a62d
>> Signed-off-by: Roger He <Hongbo.He@amd.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c     | 15 +++++++++++----
>>   drivers/gpu/drm/ttm/ttm_memory.c | 12 ++++++++----
>>   include/drm/ttm/ttm_bo_api.h     |  3 ++-
>>   3 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 313925c..ecb8916 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -1686,18 +1686,20 @@ EXPORT_SYMBOL(ttm_bo_synccpu_write_release);
>>    * A buffer object shrink method that tries to swap out the first
>>    * buffer object on the bo_global::swap_lru list.
>>    */
>> -int ttm_bo_swapout(struct ttm_bo_global *glob)
>> +int ttm_bo_swapout(struct ttm_bo_global *glob, struct 
>> ttm_operation_ctx *ctx)
>>   {
>>       struct ttm_buffer_object *bo;
>>       int ret = -EBUSY;
>> +    bool locked;
>>       unsigned i;
>>         spin_lock(&glob->lru_lock);
>>       for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
>>           list_for_each_entry(bo, &glob->swap_lru[i], swap) {
>> -            ret = reservation_object_trylock(bo->resv) ? 0 : -EBUSY;
>> -            if (!ret)
>> +            if (ttm_bo_evict_swapout_allowable(bo, ctx, &locked)) {
>> +                ret = 0;
>>                   break;
>> +            }
>>           }
>>           if (!ret)
>>               break;
>> @@ -1773,7 +1775,12 @@ EXPORT_SYMBOL(ttm_bo_swapout);
>>     void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
>>   {
>> -    while (ttm_bo_swapout(bdev->glob) == 0)
>> +    struct ttm_operation_ctx ctx = {
>> +        .interruptible = false,
>> +        .no_wait_gpu = false
>> +    };
>> +
>> +    while (ttm_bo_swapout(bdev->glob, &ctx) == 0)
>>           ;
>>   }
>>   EXPORT_SYMBOL(ttm_bo_swapout_all);
>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c 
>> b/drivers/gpu/drm/ttm/ttm_memory.c
>> index 8df0755..8817b86 100644
>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>> @@ -211,7 +211,7 @@ static bool ttm_zones_above_swap_target(struct 
>> ttm_mem_global *glob,
>>    */
>>     static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
>> -               uint64_t extra)
>> +            uint64_t extra, struct ttm_operation_ctx *ctx)
>>   {
>>       int ret;
>>   @@ -219,7 +219,7 @@ static void ttm_shrink(struct ttm_mem_global 
>> *glob, bool from_wq,
>>         while (ttm_zones_above_swap_target(glob, from_wq, extra)) {
>>           spin_unlock(&glob->lock);
>> -        ret = ttm_bo_swapout(glob->bo_glob);
>> +        ret = ttm_bo_swapout(glob->bo_glob, ctx);
>>           spin_lock(&glob->lock);
>>           if (unlikely(ret != 0))
>>               break;
>> @@ -230,10 +230,14 @@ static void ttm_shrink(struct ttm_mem_global 
>> *glob, bool from_wq,
>>     static void ttm_shrink_work(struct work_struct *work)
>>   {
>> +    struct ttm_operation_ctx ctx = {
>> +        .interruptible = false,
>> +        .no_wait_gpu = false
>> +    };
>>       struct ttm_mem_global *glob =
>>           container_of(work, struct ttm_mem_global, work);
>>   -    ttm_shrink(glob, true, 0ULL);
>> +    ttm_shrink(glob, true, 0ULL, &ctx);
>>   }
>>     static int ttm_mem_init_kernel_zone(struct ttm_mem_global *glob,
>> @@ -520,7 +524,7 @@ static int ttm_mem_global_alloc_zone(struct 
>> ttm_mem_global *glob,
>>               return -ENOMEM;
>>           if (unlikely(count-- == 0))
>>               return -ENOMEM;
>> -        ttm_shrink(glob, false, memory + (memory >> 2) + 16);
>> +        ttm_shrink(glob, false, memory + (memory >> 2) + 16, ctx);
>>       }
>>         return 0;
>> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
>> index 24a8db7..f1c74c2 100644
>> --- a/include/drm/ttm/ttm_bo_api.h
>> +++ b/include/drm/ttm/ttm_bo_api.h
>> @@ -752,7 +752,8 @@ ssize_t ttm_bo_io(struct ttm_bo_device *bdev, 
>> struct file *filp,
>>             const char __user *wbuf, char __user *rbuf,
>>             size_t count, loff_t *f_pos, bool write);
>>   -int ttm_bo_swapout(struct ttm_bo_global *glob);
>> +int ttm_bo_swapout(struct ttm_bo_global *glob,
>> +            struct ttm_operation_ctx *ctx);
>>   void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
>>   int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo);
>>   #endif
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 313925c..ecb8916 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1686,18 +1686,20 @@  EXPORT_SYMBOL(ttm_bo_synccpu_write_release);
  * A buffer object shrink method that tries to swap out the first
  * buffer object on the bo_global::swap_lru list.
  */
-int ttm_bo_swapout(struct ttm_bo_global *glob)
+int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
 {
 	struct ttm_buffer_object *bo;
 	int ret = -EBUSY;
+	bool locked;
 	unsigned i;
 
 	spin_lock(&glob->lru_lock);
 	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
 		list_for_each_entry(bo, &glob->swap_lru[i], swap) {
-			ret = reservation_object_trylock(bo->resv) ? 0 : -EBUSY;
-			if (!ret)
+			if (ttm_bo_evict_swapout_allowable(bo, ctx, &locked)) {
+				ret = 0;
 				break;
+			}
 		}
 		if (!ret)
 			break;
@@ -1773,7 +1775,12 @@  EXPORT_SYMBOL(ttm_bo_swapout);
 
 void ttm_bo_swapout_all(struct ttm_bo_device *bdev)
 {
-	while (ttm_bo_swapout(bdev->glob) == 0)
+	struct ttm_operation_ctx ctx = {
+		.interruptible = false,
+		.no_wait_gpu = false
+	};
+
+	while (ttm_bo_swapout(bdev->glob, &ctx) == 0)
 		;
 }
 EXPORT_SYMBOL(ttm_bo_swapout_all);
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 8df0755..8817b86 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -211,7 +211,7 @@  static bool ttm_zones_above_swap_target(struct ttm_mem_global *glob,
  */
 
 static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
-		       uint64_t extra)
+			uint64_t extra, struct ttm_operation_ctx *ctx)
 {
 	int ret;
 
@@ -219,7 +219,7 @@  static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
 
 	while (ttm_zones_above_swap_target(glob, from_wq, extra)) {
 		spin_unlock(&glob->lock);
-		ret = ttm_bo_swapout(glob->bo_glob);
+		ret = ttm_bo_swapout(glob->bo_glob, ctx);
 		spin_lock(&glob->lock);
 		if (unlikely(ret != 0))
 			break;
@@ -230,10 +230,14 @@  static void ttm_shrink(struct ttm_mem_global *glob, bool from_wq,
 
 static void ttm_shrink_work(struct work_struct *work)
 {
+	struct ttm_operation_ctx ctx = {
+		.interruptible = false,
+		.no_wait_gpu = false
+	};
 	struct ttm_mem_global *glob =
 	    container_of(work, struct ttm_mem_global, work);
 
-	ttm_shrink(glob, true, 0ULL);
+	ttm_shrink(glob, true, 0ULL, &ctx);
 }
 
 static int ttm_mem_init_kernel_zone(struct ttm_mem_global *glob,
@@ -520,7 +524,7 @@  static int ttm_mem_global_alloc_zone(struct ttm_mem_global *glob,
 			return -ENOMEM;
 		if (unlikely(count-- == 0))
 			return -ENOMEM;
-		ttm_shrink(glob, false, memory + (memory >> 2) + 16);
+		ttm_shrink(glob, false, memory + (memory >> 2) + 16, ctx);
 	}
 
 	return 0;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 24a8db7..f1c74c2 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -752,7 +752,8 @@  ssize_t ttm_bo_io(struct ttm_bo_device *bdev, struct file *filp,
 		  const char __user *wbuf, char __user *rbuf,
 		  size_t count, loff_t *f_pos, bool write);
 
-int ttm_bo_swapout(struct ttm_bo_global *glob);
+int ttm_bo_swapout(struct ttm_bo_global *glob,
+			struct ttm_operation_ctx *ctx);
 void ttm_bo_swapout_all(struct ttm_bo_device *bdev);
 int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo);
 #endif