Commit 484d9a84 authored by Chris Wilson's avatar Chris Wilson

drm/i915/userptr: Avoid struct_mutex recursion for mmu_invalidate_range_start

Since commit 93065ac7 ("mm, oom: distinguish blockable mode for mmu
notifiers") we have been able to report failure from
mmu_invalidate_range_start which allows us to use a trylock on the
struct_mutex to avoid potential recursion and report -EBUSY instead.
Furthermore, this allows us to pull the work into the main callback and
avoid the sleight-of-hand in using a workqueue to avoid lockdep.

However, not all paths to mmu_invalidate_range_start are prepared to
handle failure, so instead of reporting the recursion, deal with it by
propagating the failure upwards, who can decide themselves to handle it
or report it.

v2: Mark up the recursive lock behaviour and comment on the various weak
points.

v3: Follow commit 3824e419 ("drm/i915: Use mutex_lock_killable() from
inside the shrinker") and also use mutex_lock_killable().
v3.1: No leak on EINTR.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108375
References: 93065ac7 ("mm, oom: distinguish blockable mode for mmu notifiers")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190115124442.3500-1-chris@chris-wilson.co.uk
parent 3f2e9ed0
...@@ -2935,7 +2935,7 @@ enum i915_mm_subclass { /* lockdep subclass for obj->mm.lock/struct_mutex */ ...@@ -2935,7 +2935,7 @@ enum i915_mm_subclass { /* lockdep subclass for obj->mm.lock/struct_mutex */
I915_MM_SHRINKER /* called "recursively" from direct-reclaim-esque */ I915_MM_SHRINKER /* called "recursively" from direct-reclaim-esque */
}; };
void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj, int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
enum i915_mm_subclass subclass); enum i915_mm_subclass subclass);
void __i915_gem_object_invalidate(struct drm_i915_gem_object *obj); void __i915_gem_object_invalidate(struct drm_i915_gem_object *obj);
......
...@@ -2303,8 +2303,8 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj) ...@@ -2303,8 +2303,8 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
struct sg_table *pages; struct sg_table *pages;
pages = fetch_and_zero(&obj->mm.pages); pages = fetch_and_zero(&obj->mm.pages);
if (!pages) if (IS_ERR_OR_NULL(pages))
return NULL; return pages;
spin_lock(&i915->mm.obj_lock); spin_lock(&i915->mm.obj_lock);
list_del(&obj->mm.link); list_del(&obj->mm.link);
...@@ -2328,22 +2328,23 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj) ...@@ -2328,22 +2328,23 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
return pages; return pages;
} }
void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj, int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
enum i915_mm_subclass subclass) enum i915_mm_subclass subclass)
{ {
struct sg_table *pages; struct sg_table *pages;
int ret;
if (i915_gem_object_has_pinned_pages(obj)) if (i915_gem_object_has_pinned_pages(obj))
return; return -EBUSY;
GEM_BUG_ON(obj->bind_count); GEM_BUG_ON(obj->bind_count);
if (!i915_gem_object_has_pages(obj))
return;
/* May be called by shrinker from within get_pages() (on another bo) */ /* May be called by shrinker from within get_pages() (on another bo) */
mutex_lock_nested(&obj->mm.lock, subclass); mutex_lock_nested(&obj->mm.lock, subclass);
if (unlikely(atomic_read(&obj->mm.pages_pin_count))) if (unlikely(atomic_read(&obj->mm.pages_pin_count))) {
ret = -EBUSY;
goto unlock; goto unlock;
}
/* /*
* ->put_pages might need to allocate memory for the bit17 swizzle * ->put_pages might need to allocate memory for the bit17 swizzle
...@@ -2351,11 +2352,24 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj, ...@@ -2351,11 +2352,24 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
* lists early. * lists early.
*/ */
pages = __i915_gem_object_unset_pages(obj); pages = __i915_gem_object_unset_pages(obj);
/*
* XXX Temporary hijinx to avoid updating all backends to handle
* NULL pages. In the future, when we have more asynchronous
* get_pages backends we should be better able to handle the
* cancellation of the async task in a more uniform manner.
*/
if (!pages && !i915_gem_object_needs_async_cancel(obj))
pages = ERR_PTR(-EINVAL);
if (!IS_ERR(pages)) if (!IS_ERR(pages))
obj->ops->put_pages(obj, pages); obj->ops->put_pages(obj, pages);
ret = 0;
unlock: unlock:
mutex_unlock(&obj->mm.lock); mutex_unlock(&obj->mm.lock);
return ret;
} }
bool i915_sg_trim(struct sg_table *orig_st) bool i915_sg_trim(struct sg_table *orig_st)
......
...@@ -57,6 +57,7 @@ struct drm_i915_gem_object_ops { ...@@ -57,6 +57,7 @@ struct drm_i915_gem_object_ops {
#define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0) #define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0)
#define I915_GEM_OBJECT_IS_SHRINKABLE BIT(1) #define I915_GEM_OBJECT_IS_SHRINKABLE BIT(1)
#define I915_GEM_OBJECT_IS_PROXY BIT(2) #define I915_GEM_OBJECT_IS_PROXY BIT(2)
#define I915_GEM_OBJECT_ASYNC_CANCEL BIT(3)
/* Interface between the GEM object and its backing storage. /* Interface between the GEM object and its backing storage.
* get_pages() is called once prior to the use of the associated set * get_pages() is called once prior to the use of the associated set
...@@ -387,6 +388,12 @@ i915_gem_object_is_proxy(const struct drm_i915_gem_object *obj) ...@@ -387,6 +388,12 @@ i915_gem_object_is_proxy(const struct drm_i915_gem_object *obj)
return obj->ops->flags & I915_GEM_OBJECT_IS_PROXY; return obj->ops->flags & I915_GEM_OBJECT_IS_PROXY;
} }
static inline bool
i915_gem_object_needs_async_cancel(const struct drm_i915_gem_object *obj)
{
return obj->ops->flags & I915_GEM_OBJECT_ASYNC_CANCEL;
}
static inline bool static inline bool
i915_gem_object_is_active(const struct drm_i915_gem_object *obj) i915_gem_object_is_active(const struct drm_i915_gem_object *obj)
{ {
......
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment