[PATCH] drm/ttm: Allow vm fault retries

[PATCH] drm/ttm: Allow vm fault retries

[Thomas Hellstrom]

Make use of the FAULT_FLAG_ALLOW_RETRY flag to allow dropping the
mmap_sem while waiting for bo idle.

FAULT_FLAG_ALLOW_RETRY appears to be primarily designed for disk waits
but should work just as fine for GPU waits..

Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c |   62 +++++++++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 1006c15..c03514b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -41,6 +41,51 @@
 
 #define TTM_BO_VM_NUM_PREFAULT 16
 
+static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
+				struct vm_area_struct *vma,
+				struct vm_fault *vmf)
+{
+	struct ttm_bo_device *bdev = bo->bdev;
+	int ret = 0;
+
+	spin_lock(&bdev->fence_lock);
+	if (likely(!test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)))
+		goto out_unlock;
+
+	/*
+	 * Quick non-stalling check for idle.
+	 */
+	ret = ttm_bo_wait(bo, false, false, true);
+	if (likely(ret == 0))
+		goto out_unlock;
+
+	/*
+	 * If possible, avoid waiting for GPU with mmap_sem
+	 * held.
+	 */
+	if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
+		ret = VM_FAULT_RETRY;
+		if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT)
+			goto out_unlock;
+
+		up_read(&vma->vm_mm->mmap_sem);
+		(void) ttm_bo_wait(bo, false, true, false);
+		goto out_unlock;
+	}
+
+	/*
+	 * Ordinary wait.
+	 */
+	ret = ttm_bo_wait(bo, false, true, false);
+	if (unlikely(ret != 0))
+		ret = (ret != -ERESTARTSYS) ? VM_FAULT_SIGBUS :
+			VM_FAULT_NOPAGE;
+
+out_unlock:
+	spin_unlock(&bdev->fence_lock);
+	return ret;
+}
+
 static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
@@ -91,18 +136,11 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * Wait for buffer data in transit, due to a pipelined
 	 * move.
 	 */
-
-	spin_lock(&bdev->fence_lock);
-	if (test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)) {
-		ret = ttm_bo_wait(bo, false, true, false);
-		spin_unlock(&bdev->fence_lock);
-		if (unlikely(ret != 0)) {
-			retval = (ret != -ERESTARTSYS) ?
-			    VM_FAULT_SIGBUS : VM_FAULT_NOPAGE;
-			goto out_unlock;
-		}
-	} else
-		spin_unlock(&bdev->fence_lock);
+	ret = ttm_bo_vm_fault_idle(bo, vma, vmf);
+	if (unlikely(ret != 0)) {
+		retval = ret;
+		goto out_unlock;
+	}
 
 	ret = ttm_mem_io_lock(man, true);
 	if (unlikely(ret != 0)) {
-- 
1.7.10.4

[PATCH] drm/ttm: Make NO_EVICT bos available to shrinkers pending destruction

[Thomas Hellstrom]

NO_EVICT bos that are not idle when all references are dropped are put on
the delayed destroy list. However, since they are not on LRU lists, they
are not available to shrinkers at that point, and buffers on the delayed
destroy list are not checked very often for idle.

So when these buffers are put on the delayed destroy list, clear the
NO_EVICT flag and put them on the right LRU list. This way they are
immediately available for eviction or shrinkers and will not cause false
OOMS.

Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index f1a857e..6c1a38f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -429,8 +429,20 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
 		sync_obj = driver->sync_obj_ref(bo->sync_obj);
 	spin_unlock(&bdev->fence_lock);
 
-	if (!ret)
+	if (!ret) {
+
+		/*
+		 * Make NO_EVICT bos immediately available to
+		 * shrinkers, now that they are queued for
+		 * destruction.
+		 */
+		if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
+			bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
+			ttm_bo_add_to_lru(bo);
+		}
+
 		ww_mutex_unlock(&bo->resv->lock);
+	}
 
 	kref_get(&bo->list_kref);
 	list_add_tail(&bo->ddestroy, &bdev->ddestroy);
-- 
1.7.10.4

Re: [PATCH] drm/ttm: Make NO_EVICT bos available to shrinkers pending destruction

[Jakob Bornecrantz]

On Thu, Oct 10, 2013 at 8:22 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> NO_EVICT bos that are not idle when all references are dropped are put on
> the delayed destroy list. However, since they are not on LRU lists, they
> are not available to shrinkers at that point, and buffers on the delayed
> destroy list are not checked very often for idle.
>
> So when these buffers are put on the delayed destroy list, clear the
> NO_EVICT flag and put them on the right LRU list. This way they are
> immediately available for eviction or shrinkers and will not cause false
> OOMS.
>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>

Reviewed-by: Jakob Bornecrantz <jakob@vmware.com>

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c |   14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index f1a857e..6c1a38f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -429,8 +429,20 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
>                 sync_obj = driver->sync_obj_ref(bo->sync_obj);
>         spin_unlock(&bdev->fence_lock);
>
> -       if (!ret)
> +       if (!ret) {
> +
> +               /*
> +                * Make NO_EVICT bos immediately available to
> +                * shrinkers, now that they are queued for
> +                * destruction.
> +                */
> +               if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
> +                       bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
> +                       ttm_bo_add_to_lru(bo);
> +               }
> +
>                 ww_mutex_unlock(&bo->resv->lock);
> +       }
>
>         kref_get(&bo->list_kref);
>         list_add_tail(&bo->ddestroy, &bdev->ddestroy);
> --
> 1.7.10.4
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: Allow vm fault retries

[Jakob Bornecrantz]

On Thu, Oct 10, 2013 at 8:22 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> Make use of the FAULT_FLAG_ALLOW_RETRY flag to allow dropping the
> mmap_sem while waiting for bo idle.
>
> FAULT_FLAG_ALLOW_RETRY appears to be primarily designed for disk waits
> but should work just as fine for GPU waits..
>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>

Reviewed-by: Jakob Bornecrantz <jakob@vmware.com>

Tho somebody else should also take a look at this.


> ---
>  drivers/gpu/drm/ttm/ttm_bo_vm.c |   62 +++++++++++++++++++++++++++++++--------
>  1 file changed, 50 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 1006c15..c03514b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -41,6 +41,51 @@
>
>  #define TTM_BO_VM_NUM_PREFAULT 16
>
> +static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
> +                               struct vm_area_struct *vma,
> +                               struct vm_fault *vmf)
> +{
> +       struct ttm_bo_device *bdev = bo->bdev;
> +       int ret = 0;
> +
> +       spin_lock(&bdev->fence_lock);
> +       if (likely(!test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)))
> +               goto out_unlock;
> +
> +       /*
> +        * Quick non-stalling check for idle.
> +        */
> +       ret = ttm_bo_wait(bo, false, false, true);
> +       if (likely(ret == 0))
> +               goto out_unlock;
> +
> +       /*
> +        * If possible, avoid waiting for GPU with mmap_sem
> +        * held.
> +        */
> +       if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
> +               ret = VM_FAULT_RETRY;
> +               if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT)
> +                       goto out_unlock;
> +
> +               up_read(&vma->vm_mm->mmap_sem);
> +               (void) ttm_bo_wait(bo, false, true, false);
> +               goto out_unlock;
> +       }
> +
> +       /*
> +        * Ordinary wait.
> +        */
> +       ret = ttm_bo_wait(bo, false, true, false);
> +       if (unlikely(ret != 0))
> +               ret = (ret != -ERESTARTSYS) ? VM_FAULT_SIGBUS :
> +                       VM_FAULT_NOPAGE;
> +
> +out_unlock:
> +       spin_unlock(&bdev->fence_lock);
> +       return ret;
> +}
> +
>  static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  {
>         struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
> @@ -91,18 +136,11 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>          * Wait for buffer data in transit, due to a pipelined
>          * move.
>          */
> -
> -       spin_lock(&bdev->fence_lock);
> -       if (test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)) {
> -               ret = ttm_bo_wait(bo, false, true, false);
> -               spin_unlock(&bdev->fence_lock);
> -               if (unlikely(ret != 0)) {
> -                       retval = (ret != -ERESTARTSYS) ?
> -                           VM_FAULT_SIGBUS : VM_FAULT_NOPAGE;
> -                       goto out_unlock;
> -               }
> -       } else
> -               spin_unlock(&bdev->fence_lock);
> +       ret = ttm_bo_vm_fault_idle(bo, vma, vmf);
> +       if (unlikely(ret != 0)) {
> +               retval = ret;
> +               goto out_unlock;
> +       }
>
>         ret = ttm_mem_io_lock(man, true);
>         if (unlikely(ret != 0)) {
> --
> 1.7.10.4
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel