drm/ttm: Remove set_need_resched from the ttm fault handler

Addresses
"[BUG] completely bonkers use of set_need_resched + VM_FAULT_NOPAGE".

In the first occurence it was used to try to be nice while releasing the
mmap_sem and retrying the fault to work around a locking inversion.
The second occurence was never used.

There has been some discussion whether we should change the locking order to
mmap_sem -> bo_reserve. This patch doesn't address that issue, and leaves
that locking order undefined. The solution that we release the mmap_sem if
tryreserve fails and wait for the buffer to become unreserved is something
we want in any case, and follows how the core vm system waits for pages
to be come unlocked while releasing the mmap_sem.

The code also outlines what needs to be changed if we want to establish the
locking order as mmap_sem -> bo::reserve.

One slight issue that remains with this code is that the fault handler might
be prone to starvation if another thread countinously reserves the buffer.
IMO that usage pattern is highly unlikely.

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

1 files changed, 20 insertions, 6 deletions

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index ac617f3ecd0c..b249ab9b1eb2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -107,13 +107,28 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
         /*
          * Work around locking order reversal in fault / nopfn
          * between mmap_sem and bo_reserve: Perform a trylock operation
-         * for reserve, and if it fails, retry the fault after scheduling.
+         * for reserve, and if it fails, retry the fault after waiting
+         * for the buffer to become unreserved.
          */
-
-        ret = ttm_bo_reserve(bo, true, true, false, 0);
+        ret = ttm_bo_reserve(bo, true, true, false, NULL);
         if (unlikely(ret != 0)) {
-                if (ret == -EBUSY)
-                        set_need_resched();
+                if (ret != -EBUSY)
+                        return VM_FAULT_NOPAGE;
+
+                if (vmf->flags & FAULT_FLAG_ALLOW_RETRY) {
+                        if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) {
+                                up_read(&vma->vm_mm->mmap_sem);
+                                (void) ttm_bo_wait_unreserved(bo);
+                        }
+
+                        return VM_FAULT_RETRY;
+                }
+
+                /*
+                 * If we'd want to change locking order to
+                 * mmap_sem -> bo::reserve, we'd use a blocking reserve here
+                 * instead of retrying the fault...
+                 */
                 return VM_FAULT_NOPAGE;
         }
 
@@ -123,7 +138,6 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
                 case 0:
                         break;
                 case -EBUSY:
-                        set_need_resched();
                 case -ERESTARTSYS:
                         retval = VM_FAULT_NOPAGE;
                         goto out_unlock;