Following a try_to_unmap() we may want to remove the userptr and so call
put_pages(). However, try_to_unmap() acquires the page lock and so we
must avoid recursively locking the pages ourselves -- which means that
we cannot safely acquire the lock around set_page_dirty(). Since we
can't be sure of the lock, we have to risk skip dirtying the page, or
else risk calling set_page_dirty() without a lock and so risk fs
corruption.

Reported-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
Fixes: cb6d7c7dc7ff ("drm/i915/userptr: Acquire the page lock around 
set_page_dirty()")
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwer...@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index b9d2bb15e4a6..1ad2047a6dbd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -672,7 +672,7 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
                obj->mm.dirty = false;
 
        for_each_sgt_page(page, sgt_iter, pages) {
-               if (obj->mm.dirty)
+               if (obj->mm.dirty && trylock_page(page)) {
                        /*
                         * As this may not be anonymous memory (e.g. shmem)
                         * but exist on a real mapping, we have to lock
@@ -680,8 +680,20 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
                         * the page reference is not sufficient to
                         * prevent the inode from being truncated.
                         * Play safe and take the lock.
+                        *
+                        * However...!
+                        *
+                        * The mmu-notifier can be invalidated for a
+                        * migrate_page, that is alreadying holding the lock
+                        * on the page. Such a try_to_unmap() will result
+                        * in us calling put_pages() and so recursively try
+                        * to lock the page. We avoid that deadlock with
+                        * a trylock_page() and in exchange we risk missing
+                        * some page dirtying.
                         */
-                       set_page_dirty_lock(page);
+                       set_page_dirty(page);
+                       unlock_page(page);
+               }
 
                mark_page_accessed(page);
                put_page(page);
-- 
2.22.0

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

Reply via email to