[Intel-gfx] [RFC PATCH v2] drm/i915/userptr: Activate MMU notifier only after pages are set

[Janusz Krzysztofik]

The purpose of userptr MMU notifier is to invalidate object pages as
soon as someone unpins them from memory.  While doing that,
obj->mm.lock is acquired.  If the notifier was called with obj->mm.lock
already held, a lockdep loop would be triggered.  That scenario is
believed to be possible in several cases, one of which is when the
userptr object is created from an mmap-offset mapping of another i915
GEM object.  This patch tries to address this case.

Even if creating a userptr object on an mmap-offset mapping succeeds,
trying to pin pages of the mapping in memory always fails because of
them having a VM_PFNMAP flag set.  However, the notifier can be
activated for a userptr object even before required pages are found
already pinned in memory, as soon as a worker expected to get missing
pages is scheduled successfully.  If the worker then fails to collect
the pages, it deactivates the notifier.  However, a time window exists
when the notifier can be called for an object even with no pages set
yet.

Postpone activation of the userptr MMU notifier for an object until
pages are successfully acquired and set.

v2: Don't activate the notifier for as long as pages have not been set.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
---
Hi,

This is a slightly modified alternative to the patch "drm/i915/userptr:
Don't activate MMU notifier if no pages can be acquired" just submitted.
I think it is better than the original as it prevents the notifier from
being uselessly called before object pages are even set.

Thanks,
Janusz

 drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 63ead7a2b64a..1cb88ab61a57 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -498,14 +498,13 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
 			pages = __i915_gem_userptr_alloc_pages(obj, pvec,
 							       npages);
 			if (!IS_ERR(pages)) {
+				__i915_gem_userptr_set_active(obj, true);
 				pinned = 0;
 				pages = NULL;
 			}
 		}
 
 		obj->userptr.work = ERR_CAST(pages);
-		if (IS_ERR(pages))
-			__i915_gem_userptr_set_active(obj, false);
 	}
 	mutex_unlock(&obj->mm.lock);
 
@@ -613,7 +612,6 @@ static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
 		pinned = 0;
 	} else if (pinned < num_pages) {
 		pages = __i915_gem_userptr_get_pages_schedule(obj);
-		active = pages == ERR_PTR(-EAGAIN);
 	} else {
 		pages = __i915_gem_userptr_alloc_pages(obj, pvec, num_pages);
 		active = !IS_ERR(pages);
-- 
2.21.0