[PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

[PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

[Christian König]

Move the BO on the LRU only when it is actually moved by a DMA
operation.

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

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index a1d977fbade5..e86a29a1e51f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -71,7 +71,7 @@ static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
 		ttm_bo_get(bo);
 		up_read(&vmf->vma->vm_mm->mmap_sem);
 		(void) dma_fence_wait(bo->moving, true);
-		ttm_bo_unreserve(bo);
+		reservation_object_unlock(bo->resv);
 		ttm_bo_put(bo);
 		goto out_unlock;
 	}
@@ -131,11 +131,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
 	 * for reserve, and if it fails, retry the fault after waiting
 	 * for the buffer to become unreserved.
 	 */
-	err = ttm_bo_reserve(bo, true, true, NULL);
-	if (unlikely(err != 0)) {
-		if (err != -EBUSY)
-			return VM_FAULT_NOPAGE;
-
+	if (unlikely(!reservation_object_trylock(bo->resv))) {
 		if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
 			if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
 				ttm_bo_get(bo);
@@ -165,6 +161,8 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
 	}
 
 	if (bdev->driver->fault_reserve_notify) {
+		struct dma_fence *moving = dma_fence_get(bo->moving);
+
 		err = bdev->driver->fault_reserve_notify(bo);
 		switch (err) {
 		case 0:
@@ -177,6 +175,13 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
 			ret = VM_FAULT_SIGBUS;
 			goto out_unlock;
 		}
+
+		if (bo->moving != moving) {
+			spin_lock(&bdev->glob->lru_lock);
+			ttm_bo_move_to_lru_tail(bo, NULL);
+			spin_unlock(&bdev->glob->lru_lock);
+		}
+		dma_fence_put(moving);
 	}
 
 	/*
@@ -291,7 +296,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
 out_io_unlock:
 	ttm_mem_io_unlock(man);
 out_unlock:
-	ttm_bo_unreserve(bo);
+	reservation_object_unlock(bo->resv);
 	return ret;
 }
 
-- 
2.14.1

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

Re: [PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

[Michel Dänzer]

On 2019-01-11 2:15 p.m., Christian König wrote:
> Move the BO on the LRU only when it is actually moved by a DMA
> operation.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

[Michel Dänzer]

On 2019-01-11 2:15 p.m., Christian König wrote:
> Move the BO on the LRU only when it is actually moved by a DMA
> operation.
> 
> [...]
> 
> @@ -177,6 +175,13 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>  			ret = VM_FAULT_SIGBUS;
>  			goto out_unlock;
>  		}
> +
> +		if (bo->moving != moving) {

Hmm, could a driver just update the existing fence instead of attaching
a new one?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

[Christian König]

Am 11.01.19 um 15:17 schrieb Michel Dänzer:
> On 2019-01-11 2:15 p.m., Christian König wrote:
>> Move the BO on the LRU only when it is actually moved by a DMA
>> operation.
>>
>> [...]
>>
>> @@ -177,6 +175,13 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>>   			ret = VM_FAULT_SIGBUS;
>>   			goto out_unlock;
>>   		}
>> +
>> +		if (bo->moving != moving) {
> Hmm, could a driver just update the existing fence instead of attaching
> a new one?

Mhm, not as far as I know. That would violate similar checks elsewhere.

Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

[zhoucm1]

On 2019年01月11日 21:15, Christian König wrote:
> Move the BO on the LRU only when it is actually moved by a DMA
> operation.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-And-Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

I just sent lru_notify  v2 patches, please review them. With yours and 
mine, the OOM issue is fixed without negative effect.

-David
> ---
>   drivers/gpu/drm/ttm/ttm_bo_vm.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index a1d977fbade5..e86a29a1e51f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -71,7 +71,7 @@ static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
>   		ttm_bo_get(bo);
>   		up_read(&vmf->vma->vm_mm->mmap_sem);
>   		(void) dma_fence_wait(bo->moving, true);
> -		ttm_bo_unreserve(bo);
> +		reservation_object_unlock(bo->resv);
>   		ttm_bo_put(bo);
>   		goto out_unlock;
>   	}
> @@ -131,11 +131,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>   	 * for reserve, and if it fails, retry the fault after waiting
>   	 * for the buffer to become unreserved.
>   	 */
> -	err = ttm_bo_reserve(bo, true, true, NULL);
> -	if (unlikely(err != 0)) {
> -		if (err != -EBUSY)
> -			return VM_FAULT_NOPAGE;
> -
> +	if (unlikely(!reservation_object_trylock(bo->resv))) {
>   		if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
>   			if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
>   				ttm_bo_get(bo);
> @@ -165,6 +161,8 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>   	}
>   
>   	if (bdev->driver->fault_reserve_notify) {
> +		struct dma_fence *moving = dma_fence_get(bo->moving);
> +
>   		err = bdev->driver->fault_reserve_notify(bo);
>   		switch (err) {
>   		case 0:
> @@ -177,6 +175,13 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>   			ret = VM_FAULT_SIGBUS;
>   			goto out_unlock;
>   		}
> +
> +		if (bo->moving != moving) {
> +			spin_lock(&bdev->glob->lru_lock);
> +			ttm_bo_move_to_lru_tail(bo, NULL);
> +			spin_unlock(&bdev->glob->lru_lock);
> +		}
> +		dma_fence_put(moving);
>   	}
>   
>   	/*
> @@ -291,7 +296,7 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)
>   out_io_unlock:
>   	ttm_mem_io_unlock(man);
>   out_unlock:
> -	ttm_bo_unreserve(bo);
> +	reservation_object_unlock(bo->resv);
>   	return ret;
>   }
>   

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/ttm: stop always moving BOs on the LRU on page fault

[Michel Dänzer]

On 2019-01-11 8:12 p.m., Christian König wrote:
> Am 11.01.19 um 15:17 schrieb Michel Dänzer:
>> On 2019-01-11 2:15 p.m., Christian König wrote:
>>> Move the BO on the LRU only when it is actually moved by a DMA
>>> operation.
>>>
>>> [...]
>>>
>>> @@ -177,6 +175,13 @@ static vm_fault_t ttm_bo_vm_fault(struct
>>> vm_fault *vmf)
>>>               ret = VM_FAULT_SIGBUS;
>>>               goto out_unlock;
>>>           }
>>> +
>>> +        if (bo->moving != moving) {
>> Hmm, could a driver just update the existing fence instead of attaching
>> a new one?
> 
> Mhm, not as far as I know. That would violate similar checks elsewhere.

Makes sense, my R-b stands.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx