| Message ID | 20200219120944.21200-1-janusz.krzysztofik@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm/i915/userptr: Don't activate MMU notifier if no pages can be acquired | expand |
Quoting Janusz Krzysztofik (2020-02-19 12:09:44) > 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. You mean something like https://patchwork.freedesktop.org/patch/275514/?series=54869&rev=2 to avoid lockdep cross-contamination. -Chris
Hi Chris, On Wednesday, February 19, 2020 1:17:12 PM CET Chris Wilson wrote: > Quoting Janusz Krzysztofik (2020-02-19 12:09:44) > > 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. > > You mean something like > https://patchwork.freedesktop.org/patch/275514/?series=54869&rev=2 > to avoid lockdep cross-contamination. Yes, the purpose of both seems the same. How could I help to get your preferred one merged? Thanks, Janusz > -Chris >
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c index 63ead7a2b64a..d9f3d66694a2 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c @@ -45,7 +45,9 @@ struct i915_mmu_object { static void add_object(struct i915_mmu_object *mo) { - GEM_BUG_ON(!RB_EMPTY_NODE(&mo->it.rb)); + if (!RB_EMPTY_NODE(&mo->it.rb)) + return; + interval_tree_insert(&mo->it, &mo->mn->objects); } @@ -498,6 +500,7 @@ __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; } @@ -613,7 +616,7 @@ 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); + active = pages == ERR_PTR(-EAGAIN) && pinned; } else { pages = __i915_gem_userptr_alloc_pages(obj, pvec, num_pages); active = !IS_ERR(pages);
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 some pages are successfully acquired. Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)