Hi, Maarten, On 3/11/21 2:41 PM, Maarten Lankhorst wrote: > Instead of doing what we do currently, which will never work with > PROVE_LOCKING, do the same as AMD does, and something similar to > relocation slowpath. When all locks are dropped, we acquire the > pages for pinning. When the locks are taken, we transfer those > pages in .get_pages() to the bo. As a final check before installing > the fences, we ensure that the mmu notifier was not called; if it is, > we return -EAGAIN to userspace to signal it has to start over. > > Changes since v1: > - Unbinding is done in submit_init only. submit_begin() removed. > - MMU_NOTFIER -> MMU_NOTIFIER > Changes since v2: > - Make i915->mm.notifier a spinlock. > Changes since v3: > - Add WARN_ON if there are any page references left, should have been 0. > - Return 0 on success in submit_init(), bug from spinlock conversion. > - Release pvec outside of notifier_lock (Thomas). > Changes since v4: > - Mention why we're clearing eb->[i + 1].vma in the code. (Thomas) > - Actually check all invalidations in eb_move_to_gpu. (Thomas) > - Do not wait when process is exiting to fix gem_ctx_persistence.userptr. > Changes since v5: > - Clarify why check on PF_EXITING is (temporarily) required. > Changes since v6: > - Ensure userptr validity is checked in set_domain through a special path. > > Signed-off-by: Maarten Lankhorst > Acked-by: Dave Airlie Mostly LGTM. Comments / suggestions below. > --- > drivers/gpu/drm/i915/gem/i915_gem_domain.c | 18 +- > .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 101 ++- > drivers/gpu/drm/i915/gem/i915_gem_object.h | 38 +- > .../gpu/drm/i915/gem/i915_gem_object_types.h | 10 +- > drivers/gpu/drm/i915/gem/i915_gem_pages.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 796 ++++++------------ > drivers/gpu/drm/i915/i915_drv.h | 9 +- > drivers/gpu/drm/i915/i915_gem.c | 5 +- > 8 files changed, 395 insertions(+), 584 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c > index 2f4980bf742e..76cb9f5c66aa 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c > @@ -468,14 +468,28 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, > if (!obj) > return -ENOENT; > > + if (i915_gem_object_is_userptr(obj)) { > + /* > + * Try to grab userptr pages, iris uses set_domain to check > + * userptr validity > + */ > + err = i915_gem_object_userptr_validate(obj); > + if (!err) > + err = i915_gem_object_wait(obj, > + I915_WAIT_INTERRUPTIBLE | > + I915_WAIT_PRIORITY | > + (write_domain ? I915_WAIT_ALL : 0), > + MAX_SCHEDULE_TIMEOUT); > + goto out; > + } > + > /* > * Proxy objects do not control access to the backing storage, ergo > * they cannot be used as a means to manipulate the cache domain > * tracking for that backing storage. The proxy object is always > * considered to be outside of any cache domain. > */ > - if (i915_gem_object_is_proxy(obj) && > - !i915_gem_object_is_userptr(obj)) { > + if (i915_gem_object_is_proxy(obj)) { > err = -ENXIO; > goto out; > } > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index c72440c10876..64d0e5fccece 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -53,14 +53,16 @@ enum { > /* __EXEC_OBJECT_NO_RESERVE is BIT(31), defined in i915_vma.h */ > #define __EXEC_OBJECT_HAS_PIN BIT(30) > #define __EXEC_OBJECT_HAS_FENCE BIT(29) > -#define __EXEC_OBJECT_NEEDS_MAP BIT(28) > -#define __EXEC_OBJECT_NEEDS_BIAS BIT(27) > -#define __EXEC_OBJECT_INTERNAL_FLAGS (~0u << 27) /* all of the above + */ > +#define __EXEC_OBJECT_USERPTR_INIT BIT(28) > +#define __EXEC_OBJECT_NEEDS_MAP BIT(27) > +#define __EXEC_OBJECT_NEEDS_BIAS BIT(26) > +#define __EXEC_OBJECT_INTERNAL_FLAGS (~0u << 26) /* all of the above + */ > #define __EXEC_OBJECT_RESERVED (__EXEC_OBJECT_HAS_PIN | __EXEC_OBJECT_HAS_FENCE) > > #define __EXEC_HAS_RELOC BIT(31) > #define __EXEC_ENGINE_PINNED BIT(30) > -#define __EXEC_INTERNAL_FLAGS (~0u << 30) > +#define __EXEC_USERPTR_USED BIT(29) > +#define __EXEC_INTERNAL_FLAGS (~0u << 29) > #define UPDATE PIN_OFFSET_FIXED > > #define BATCH_OFFSET_BIAS (256*1024) > @@ -864,6 +866,26 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) > } > > eb_add_vma(eb, i, batch, vma); > + > + if (i915_gem_object_is_userptr(vma->obj)) { > + err = i915_gem_object_userptr_submit_init(vma->obj); > + if (err) { > + if (i + 1 < eb->buffer_count) { > + /* > + * Execbuffer code expects last vma entry to be NULL, > + * since we already initialized this entry, > + * set the next value to NULL or we mess up > + * cleanup handling. > + */ > + eb->vma[i + 1].vma = NULL; > + } > + > + return err; > + } > + > + eb->vma[i].flags |= __EXEC_OBJECT_USERPTR_INIT; > + eb->args->flags |= __EXEC_USERPTR_USED; > + } > } > > if (unlikely(eb->batch->flags & EXEC_OBJECT_WRITE)) { > @@ -965,7 +987,7 @@ eb_get_vma(const struct i915_execbuffer *eb, unsigned long handle) > } > } > > -static void eb_release_vmas(struct i915_execbuffer *eb, bool final) > +static void eb_release_vmas(struct i915_execbuffer *eb, bool final, bool release_userptr) > { > const unsigned int count = eb->buffer_count; > unsigned int i; > @@ -979,6 +1001,11 @@ static void eb_release_vmas(struct i915_execbuffer *eb, bool final) > > eb_unreserve_vma(ev); > > + if (release_userptr && ev->flags & __EXEC_OBJECT_USERPTR_INIT) { > + ev->flags &= ~__EXEC_OBJECT_USERPTR_INIT; > + i915_gem_object_userptr_submit_fini(vma->obj); > + } > + > if (final) > i915_vma_put(vma); > } > @@ -1909,6 +1936,31 @@ static int eb_prefault_relocations(const struct i915_execbuffer *eb) > return 0; > } > > +static int eb_reinit_userptr(struct i915_execbuffer *eb) > +{ > + const unsigned int count = eb->buffer_count; > + unsigned int i; > + int ret; > + > + if (likely(!(eb->args->flags & __EXEC_USERPTR_USED))) > + return 0; > + > + for (i = 0; i < count; i++) { > + struct eb_vma *ev = &eb->vma[i]; > + > + if (!i915_gem_object_is_userptr(ev->vma->obj)) > + continue; > + > + ret = i915_gem_object_userptr_submit_init(ev->vma->obj); > + if (ret) > + return ret; > + > + ev->flags |= __EXEC_OBJECT_USERPTR_INIT; > + } > + > + return 0; > +} > + > static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb, > struct i915_request *rq) > { > @@ -1923,7 +1975,7 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb, > } > > /* We may process another execbuffer during the unlock... */ > - eb_release_vmas(eb, false); > + eb_release_vmas(eb, false, true); > i915_gem_ww_ctx_fini(&eb->ww); > > if (rq) { > @@ -1964,10 +2016,8 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb, > err = 0; > } > > -#ifdef CONFIG_MMU_NOTIFIER > if (!err) > - flush_workqueue(eb->i915->mm.userptr_wq); > -#endif > + err = eb_reinit_userptr(eb); > > err_relock: > i915_gem_ww_ctx_init(&eb->ww, true); > @@ -2029,7 +2079,7 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb, > > err: > if (err == -EDEADLK) { > - eb_release_vmas(eb, false); > + eb_release_vmas(eb, false, false); > err = i915_gem_ww_ctx_backoff(&eb->ww); > if (!err) > goto repeat_validate; > @@ -2126,7 +2176,7 @@ static int eb_relocate_parse(struct i915_execbuffer *eb) > > err: > if (err == -EDEADLK) { > - eb_release_vmas(eb, false); > + eb_release_vmas(eb, false, false); > err = i915_gem_ww_ctx_backoff(&eb->ww); > if (!err) > goto retry; > @@ -2201,6 +2251,30 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb) > flags | __EXEC_OBJECT_NO_RESERVE); > } > > +#ifdef CONFIG_MMU_NOTIFIER > + if (!err && (eb->args->flags & __EXEC_USERPTR_USED)) { > + spin_lock(&eb->i915->mm.notifier_lock); > + > + /* > + * count is always at least 1, otherwise __EXEC_USERPTR_USED > + * could not have been set > + */ > + for (i = 0; i < count; i++) { > + struct eb_vma *ev = &eb->vma[i]; > + struct drm_i915_gem_object *obj = ev->vma->obj; > + > + if (!i915_gem_object_is_userptr(obj)) > + continue; > + > + err = i915_gem_object_userptr_submit_done(obj); > + if (err) > + break; > + } > + > + spin_unlock(&eb->i915->mm.notifier_lock); > + } > +#endif > + > if (unlikely(err)) > goto err_skip; > > @@ -3345,7 +3419,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > err = eb_lookup_vmas(&eb); > if (err) { > - eb_release_vmas(&eb, true); > + eb_release_vmas(&eb, true, true); > goto err_engine; > } > > @@ -3417,6 +3491,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > trace_i915_request_queue(eb.request, eb.batch_flags); > err = eb_submit(&eb, batch); > + > err_request: > i915_request_get(eb.request); > err = eb_request_add(&eb, err); > @@ -3437,7 +3512,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > i915_request_put(eb.request); > > err_vma: > - eb_release_vmas(&eb, true); > + eb_release_vmas(&eb, true, true); > if (eb.trampoline) > i915_vma_unpin(eb.trampoline); > WARN_ON(err == -EDEADLK); > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h > index 69509dbd01c7..b5af9c440ac5 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > @@ -59,6 +59,7 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *i915, > const void *data, resource_size_t size); > > extern const struct drm_i915_gem_object_ops i915_gem_shmem_ops; > + > void __i915_gem_object_release_shmem(struct drm_i915_gem_object *obj, > struct sg_table *pages, > bool needs_clflush); > @@ -278,12 +279,6 @@ i915_gem_object_never_mmap(const struct drm_i915_gem_object *obj) > return i915_gem_object_type_has(obj, I915_GEM_OBJECT_NO_MMAP); > } > > -static inline bool > -i915_gem_object_needs_async_cancel(const struct drm_i915_gem_object *obj) > -{ > - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_ASYNC_CANCEL); > -} > - > static inline bool > i915_gem_object_is_framebuffer(const struct drm_i915_gem_object *obj) > { > @@ -573,16 +568,6 @@ void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, > void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj, > enum fb_op_origin origin); > > -static inline bool > -i915_gem_object_is_userptr(struct drm_i915_gem_object *obj) > -{ > -#ifdef CONFIG_MMU_NOTIFIER > - return obj->userptr.mm; > -#else > - return false; > -#endif > -} > - > static inline void > i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj, > enum fb_op_origin origin) > @@ -603,4 +588,25 @@ int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64 offset, > > bool i915_gem_object_is_shmem(const struct drm_i915_gem_object *obj); > > +#ifdef CONFIG_MMU_NOTIFIER > +static inline bool > +i915_gem_object_is_userptr(struct drm_i915_gem_object *obj) > +{ > + return obj->userptr.notifier.mm; > +} > + > +int i915_gem_object_userptr_submit_init(struct drm_i915_gem_object *obj); > +int i915_gem_object_userptr_submit_done(struct drm_i915_gem_object *obj); > +void i915_gem_object_userptr_submit_fini(struct drm_i915_gem_object *obj); > +int i915_gem_object_userptr_validate(struct drm_i915_gem_object *obj); > +#else > +static inline bool i915_gem_object_is_userptr(struct drm_i915_gem_object *obj) { return false; } > + > +static inline int i915_gem_object_userptr_submit_init(struct drm_i915_gem_object *obj) { GEM_BUG_ON(1); return -ENODEV; } > +static inline int i915_gem_object_userptr_submit_done(struct drm_i915_gem_object *obj) { GEM_BUG_ON(1); return -ENODEV; } > +static inline void i915_gem_object_userptr_submit_fini(struct drm_i915_gem_object *obj) { GEM_BUG_ON(1); } > +static inline int i915_gem_object_userptr_validate(struct drm_i915_gem_object *obj) { GEM_BUG_ON(1); return -ENODEV; } > + > +#endif > + > #endif > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > index 414322619781..4c0a34231623 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > @@ -7,6 +7,8 @@ > #ifndef __I915_GEM_OBJECT_TYPES_H__ > #define __I915_GEM_OBJECT_TYPES_H__ > > +#include > + > #include > #include > > @@ -34,7 +36,6 @@ struct drm_i915_gem_object_ops { > #define I915_GEM_OBJECT_IS_SHRINKABLE BIT(2) > #define I915_GEM_OBJECT_IS_PROXY BIT(3) > #define I915_GEM_OBJECT_NO_MMAP BIT(4) > -#define I915_GEM_OBJECT_ASYNC_CANCEL BIT(5) > > /* Interface between the GEM object and its backing storage. > * get_pages() is called once prior to the use of the associated set > @@ -299,10 +300,11 @@ struct drm_i915_gem_object { > #ifdef CONFIG_MMU_NOTIFIER > struct i915_gem_userptr { > uintptr_t ptr; > + unsigned long notifier_seq; > > - struct i915_mm_struct *mm; > - struct i915_mmu_object *mmu_object; > - struct work_struct *work; > + struct mmu_interval_notifier notifier; > + struct page **pvec; > + int page_ref; > } userptr; > #endif > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c > index bf61b88a2113..e7d7650072c5 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c > @@ -226,7 +226,7 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj) > * 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)) > + if (!pages) > pages = ERR_PTR(-EINVAL); > > if (!IS_ERR(pages)) > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > index b466ab2def4d..1e42fbc68697 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > @@ -2,10 +2,39 @@ > * SPDX-License-Identifier: MIT > * > * Copyright © 2012-2014 Intel Corporation > + * > + * Based on amdgpu_mn, which bears the following notice: > + * > + * Copyright 2014 Advanced Micro Devices, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE > + * USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portions > + * of the Software. > + * > + */ > +/* > + * Authors: > + * Christian König > */ > > #include > -#include > #include > #include > #include > @@ -15,373 +44,121 @@ > #include "i915_gem_object.h" > #include "i915_scatterlist.h" > > -#if defined(CONFIG_MMU_NOTIFIER) > - > -struct i915_mm_struct { > - struct mm_struct *mm; > - struct drm_i915_private *i915; > - struct i915_mmu_notifier *mn; > - struct hlist_node node; > - struct kref kref; > - struct rcu_work work; > -}; > - > -#include > - > -struct i915_mmu_notifier { > - spinlock_t lock; > - struct hlist_node node; > - struct mmu_notifier mn; > - struct rb_root_cached objects; > - struct i915_mm_struct *mm; > -}; > - > -struct i915_mmu_object { > - struct i915_mmu_notifier *mn; > - struct drm_i915_gem_object *obj; > - struct interval_tree_node it; > -}; > +#ifdef CONFIG_MMU_NOTIFIER > > -static void add_object(struct i915_mmu_object *mo) > +/** > + * i915_gem_userptr_invalidate - callback to notify about mm change > + * > + * @mni: the range (mm) is about to update > + * @range: details on the invalidation > + * @cur_seq: Value to pass to mmu_interval_set_seq() > + * > + * Block for operations on BOs to finish and mark pages as accessed and > + * potentially dirty. > + */ > +static bool i915_gem_userptr_invalidate(struct mmu_interval_notifier *mni, > + const struct mmu_notifier_range *range, > + unsigned long cur_seq) > { > - GEM_BUG_ON(!RB_EMPTY_NODE(&mo->it.rb)); > - interval_tree_insert(&mo->it, &mo->mn->objects); > -} > + struct drm_i915_gem_object *obj = container_of(mni, struct drm_i915_gem_object, userptr.notifier); > + struct drm_i915_private *i915 = to_i915(obj->base.dev); > + long r; > > -static void del_object(struct i915_mmu_object *mo) > -{ > - if (RB_EMPTY_NODE(&mo->it.rb)) > - return; > + if (!mmu_notifier_range_blockable(range)) > + return false; > > - interval_tree_remove(&mo->it, &mo->mn->objects); > - RB_CLEAR_NODE(&mo->it.rb); > -} > + spin_lock(&i915->mm.notifier_lock); > > -static void > -__i915_gem_userptr_set_active(struct drm_i915_gem_object *obj, bool value) > -{ > - struct i915_mmu_object *mo = obj->userptr.mmu_object; > + mmu_interval_set_seq(mni, cur_seq); > + > + spin_unlock(&i915->mm.notifier_lock); > > /* > - * During mm_invalidate_range we need to cancel any userptr that > - * overlaps the range being invalidated. Doing so requires the > - * struct_mutex, and that risks recursion. In order to cause > - * recursion, the user must alias the userptr address space with > - * a GTT mmapping (possible with a MAP_FIXED) - then when we have > - * to invalidate that mmaping, mm_invalidate_range is called with > - * the userptr address *and* the struct_mutex held. To prevent that > - * we set a flag under the i915_mmu_notifier spinlock to indicate > - * whether this object is valid. > + * We don't wait when the process is exiting. This is valid > + * because the object will be cleaned up anyway. > + * > + * This is also temporarily required as a hack, because we > + * cannot currently force non-consistent batch buffers to preempt > + * and reschedule by waiting on it, hanging processes on exit. > */ > - if (!mo) > - return; > - > - spin_lock(&mo->mn->lock); > - if (value) > - add_object(mo); > - else > - del_object(mo); > - spin_unlock(&mo->mn->lock); > -} > - > -static int > -userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, > - const struct mmu_notifier_range *range) > -{ > - struct i915_mmu_notifier *mn = > - container_of(_mn, struct i915_mmu_notifier, mn); > - struct interval_tree_node *it; > - unsigned long end; > - int ret = 0; > - > - if (RB_EMPTY_ROOT(&mn->objects.rb_root)) > - return 0; > - > - /* interval ranges are inclusive, but invalidate range is exclusive */ > - end = range->end - 1; > - > - spin_lock(&mn->lock); > - it = interval_tree_iter_first(&mn->objects, range->start, end); > - while (it) { > - struct drm_i915_gem_object *obj; > - > - if (!mmu_notifier_range_blockable(range)) { > - ret = -EAGAIN; > - break; > - } > - > - /* > - * The mmu_object is released late when destroying the > - * GEM object so it is entirely possible to gain a > - * reference on an object in the process of being freed > - * since our serialisation is via the spinlock and not > - * the struct_mutex - and consequently use it after it > - * is freed and then double free it. To prevent that > - * use-after-free we only acquire a reference on the > - * object if it is not in the process of being destroyed. > - */ > - obj = container_of(it, struct i915_mmu_object, it)->obj; > - if (!kref_get_unless_zero(&obj->base.refcount)) { > - it = interval_tree_iter_next(it, range->start, end); > - continue; > - } > - spin_unlock(&mn->lock); > - > - ret = i915_gem_object_unbind(obj, > - I915_GEM_OBJECT_UNBIND_ACTIVE | > - I915_GEM_OBJECT_UNBIND_BARRIER); > - if (ret == 0) > - ret = __i915_gem_object_put_pages(obj); > - i915_gem_object_put(obj); > - if (ret) > - return ret; > + if (current->flags & PF_EXITING) > + return true; > > - spin_lock(&mn->lock); > - > - /* > - * As we do not (yet) protect the mmu from concurrent insertion > - * over this range, there is no guarantee that this search will > - * terminate given a pathologic workload. > - */ > - it = interval_tree_iter_first(&mn->objects, range->start, end); > - } > - spin_unlock(&mn->lock); > - > - return ret; > + /* we will unbind on next submission, still have userptr pins */ > + r = dma_resv_wait_timeout_rcu(obj->base.resv, true, false, > + MAX_SCHEDULE_TIMEOUT); > + if (r <= 0) > + drm_err(&i915->drm, "(%ld) failed to wait for idle\n", r); I think, since linux 5.9 where a fork is no longer setting up COW on pinned pages, and we do in fact still pin pages, I think this fence wait should be removed, together with the PF_EXIT special case, as it does not improve on anything but creates hangs that only hangcheck / watchdog can resolve. If we, in future work no longer pin the pages, which is the direction we're moving towards, let's re-add it when needed. > > + return true; > } > > -static const struct mmu_notifier_ops i915_gem_userptr_notifier = { > - .invalidate_range_start = userptr_mn_invalidate_range_start, > +static const struct mmu_interval_notifier_ops i915_gem_userptr_notifier_ops = { > + .invalidate = i915_gem_userptr_invalidate, > }; > > -static struct i915_mmu_notifier * > -i915_mmu_notifier_create(struct i915_mm_struct *mm) > -{ > - struct i915_mmu_notifier *mn; > - > - mn = kmalloc(sizeof(*mn), GFP_KERNEL); > - if (mn == NULL) > - return ERR_PTR(-ENOMEM); > - > - spin_lock_init(&mn->lock); > - mn->mn.ops = &i915_gem_userptr_notifier; > - mn->objects = RB_ROOT_CACHED; > - mn->mm = mm; > - > - return mn; > -} > - > -static void > -i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) > -{ > - struct i915_mmu_object *mo; > - > - mo = fetch_and_zero(&obj->userptr.mmu_object); > - if (!mo) > - return; > - > - spin_lock(&mo->mn->lock); > - del_object(mo); > - spin_unlock(&mo->mn->lock); > - kfree(mo); > -} > - > -static struct i915_mmu_notifier * > -i915_mmu_notifier_find(struct i915_mm_struct *mm) > -{ > - struct i915_mmu_notifier *mn, *old; > - int err; > - > - mn = READ_ONCE(mm->mn); > - if (likely(mn)) > - return mn; > - > - mn = i915_mmu_notifier_create(mm); > - if (IS_ERR(mn)) > - return mn; > - > - err = mmu_notifier_register(&mn->mn, mm->mm); > - if (err) { > - kfree(mn); > - return ERR_PTR(err); > - } > - > - old = cmpxchg(&mm->mn, NULL, mn); > - if (old) { > - mmu_notifier_unregister(&mn->mn, mm->mm); > - kfree(mn); > - mn = old; > - } > - > - return mn; > -} > - > static int > i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj) > { > - struct i915_mmu_notifier *mn; > - struct i915_mmu_object *mo; > - > - if (GEM_WARN_ON(!obj->userptr.mm)) > - return -EINVAL; > - > - mn = i915_mmu_notifier_find(obj->userptr.mm); > - if (IS_ERR(mn)) > - return PTR_ERR(mn); > - > - mo = kzalloc(sizeof(*mo), GFP_KERNEL); > - if (!mo) > - return -ENOMEM; > - > - mo->mn = mn; > - mo->obj = obj; > - mo->it.start = obj->userptr.ptr; > - mo->it.last = obj->userptr.ptr + obj->base.size - 1; > - RB_CLEAR_NODE(&mo->it.rb); > - > - obj->userptr.mmu_object = mo; > - return 0; > -} > - > -static void > -i915_mmu_notifier_free(struct i915_mmu_notifier *mn, > - struct mm_struct *mm) > -{ > - if (mn == NULL) > - return; > - > - mmu_notifier_unregister(&mn->mn, mm); > - kfree(mn); > -} > - > - > -static struct i915_mm_struct * > -__i915_mm_struct_find(struct drm_i915_private *i915, struct mm_struct *real) > -{ > - struct i915_mm_struct *it, *mm = NULL; > - > - rcu_read_lock(); > - hash_for_each_possible_rcu(i915->mm_structs, > - it, node, > - (unsigned long)real) > - if (it->mm == real && kref_get_unless_zero(&it->kref)) { > - mm = it; > - break; > - } > - rcu_read_unlock(); > - > - return mm; > + return mmu_interval_notifier_insert(&obj->userptr.notifier, current->mm, > + obj->userptr.ptr, obj->base.size, > + &i915_gem_userptr_notifier_ops); > } > > -static int > -i915_gem_userptr_init__mm_struct(struct drm_i915_gem_object *obj) > +static void i915_gem_object_userptr_drop_ref(struct drm_i915_gem_object *obj) > { > struct drm_i915_private *i915 = to_i915(obj->base.dev); > - struct i915_mm_struct *mm, *new; > - int ret = 0; > - > - /* During release of the GEM object we hold the struct_mutex. This > - * precludes us from calling mmput() at that time as that may be > - * the last reference and so call exit_mmap(). exit_mmap() will > - * attempt to reap the vma, and if we were holding a GTT mmap > - * would then call drm_gem_vm_close() and attempt to reacquire > - * the struct mutex. So in order to avoid that recursion, we have > - * to defer releasing the mm reference until after we drop the > - * struct_mutex, i.e. we need to schedule a worker to do the clean > - * up. > - */ > - mm = __i915_mm_struct_find(i915, current->mm); > - if (mm) > - goto out; > + struct page **pvec = NULL; > > - new = kmalloc(sizeof(*mm), GFP_KERNEL); > - if (!new) > - return -ENOMEM; > - > - kref_init(&new->kref); > - new->i915 = to_i915(obj->base.dev); > - new->mm = current->mm; > - new->mn = NULL; > - > - spin_lock(&i915->mm_lock); > - mm = __i915_mm_struct_find(i915, current->mm); > - if (!mm) { > - hash_add_rcu(i915->mm_structs, > - &new->node, > - (unsigned long)new->mm); > - mmgrab(current->mm); > - mm = new; > + spin_lock(&i915->mm.notifier_lock); > + if (!--obj->userptr.page_ref) { > + pvec = obj->userptr.pvec; > + obj->userptr.pvec = NULL; > } > - spin_unlock(&i915->mm_lock); > - if (mm != new) > - kfree(new); > + GEM_BUG_ON(obj->userptr.page_ref < 0); > + spin_unlock(&i915->mm.notifier_lock); > > -out: > - obj->userptr.mm = mm; > - return ret; > -} > - > -static void > -__i915_mm_struct_free__worker(struct work_struct *work) > -{ > - struct i915_mm_struct *mm = container_of(work, typeof(*mm), work.work); > - > - i915_mmu_notifier_free(mm->mn, mm->mm); > - mmdrop(mm->mm); > - kfree(mm); > -} > - > -static void > -__i915_mm_struct_free(struct kref *kref) > -{ > - struct i915_mm_struct *mm = container_of(kref, typeof(*mm), kref); > - > - spin_lock(&mm->i915->mm_lock); > - hash_del_rcu(&mm->node); > - spin_unlock(&mm->i915->mm_lock); > - > - INIT_RCU_WORK(&mm->work, __i915_mm_struct_free__worker); > - queue_rcu_work(system_wq, &mm->work); > -} > - > -static void > -i915_gem_userptr_release__mm_struct(struct drm_i915_gem_object *obj) > -{ > - if (obj->userptr.mm == NULL) > - return; > + if (pvec) { > + const unsigned long num_pages = obj->base.size >> PAGE_SHIFT; > > - kref_put(&obj->userptr.mm->kref, __i915_mm_struct_free); > - obj->userptr.mm = NULL; > + unpin_user_pages(pvec, num_pages); > + kfree(pvec); IIRC, CQ spotted that we should have a kvfree here right? > + } > } > > -struct get_pages_work { > - struct work_struct work; > - struct drm_i915_gem_object *obj; > - struct task_struct *task; > -}; > - > -static struct sg_table * > -__i915_gem_userptr_alloc_pages(struct drm_i915_gem_object *obj, > - struct page **pvec, unsigned long num_pages) > +static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) > { > + struct drm_i915_private *i915 = to_i915(obj->base.dev); > + const unsigned long num_pages = obj->base.size >> PAGE_SHIFT; > unsigned int max_segment = i915_sg_segment_size(); > struct sg_table *st; > unsigned int sg_page_sizes; > struct scatterlist *sg; > + struct page **pvec; > int ret; > > st = kmalloc(sizeof(*st), GFP_KERNEL); > if (!st) > - return ERR_PTR(-ENOMEM); > + return -ENOMEM; > + > + spin_lock(&i915->mm.notifier_lock); > + if (GEM_WARN_ON(!obj->userptr.page_ref)) { > + spin_unlock(&i915->mm.notifier_lock); > + ret = -EFAULT; > + goto err_free; > + } > + > + obj->userptr.page_ref++; > + pvec = obj->userptr.pvec; > + spin_unlock(&i915->mm.notifier_lock); > > alloc_table: > sg = __sg_alloc_table_from_pages(st, pvec, num_pages, 0, > num_pages << PAGE_SHIFT, max_segment, > NULL, 0, GFP_KERNEL); > if (IS_ERR(sg)) { > - kfree(st); > - return ERR_CAST(sg); > + ret = PTR_ERR(sg); > + goto err; > } > > ret = i915_gem_gtt_prepare_pages(obj, st); > @@ -393,203 +170,20 @@ __i915_gem_userptr_alloc_pages(struct drm_i915_gem_object *obj, > goto alloc_table; > } > > - kfree(st); > - return ERR_PTR(ret); > + goto err; > } > > sg_page_sizes = i915_sg_page_sizes(st->sgl); > > __i915_gem_object_set_pages(obj, st, sg_page_sizes); > > - return st; > -} > - > -static void > -__i915_gem_userptr_get_pages_worker(struct work_struct *_work) > -{ > - struct get_pages_work *work = container_of(_work, typeof(*work), work); > - struct drm_i915_gem_object *obj = work->obj; > - const unsigned long npages = obj->base.size >> PAGE_SHIFT; > - unsigned long pinned; > - struct page **pvec; > - int ret; > - > - ret = -ENOMEM; > - pinned = 0; > - > - pvec = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL); > - if (pvec != NULL) { > - struct mm_struct *mm = obj->userptr.mm->mm; > - unsigned int flags = 0; > - int locked = 0; > - > - if (!i915_gem_object_is_readonly(obj)) > - flags |= FOLL_WRITE; > - > - ret = -EFAULT; > - if (mmget_not_zero(mm)) { > - while (pinned < npages) { > - if (!locked) { > - mmap_read_lock(mm); > - locked = 1; > - } > - ret = pin_user_pages_remote > - (mm, > - obj->userptr.ptr + pinned * PAGE_SIZE, > - npages - pinned, > - flags, > - pvec + pinned, NULL, &locked); > - if (ret < 0) > - break; > - > - pinned += ret; > - } > - if (locked) > - mmap_read_unlock(mm); > - mmput(mm); > - } > - } > - > - mutex_lock_nested(&obj->mm.lock, I915_MM_GET_PAGES); > - if (obj->userptr.work == &work->work) { > - struct sg_table *pages = ERR_PTR(ret); > - > - if (pinned == npages) { > - pages = __i915_gem_userptr_alloc_pages(obj, pvec, > - npages); > - if (!IS_ERR(pages)) { > - 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); > - > - unpin_user_pages(pvec, pinned); > - kvfree(pvec); > - > - i915_gem_object_put(obj); > - put_task_struct(work->task); > - kfree(work); > -} > - > -static struct sg_table * > -__i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj) > -{ > - struct get_pages_work *work; > - > - /* Spawn a worker so that we can acquire the > - * user pages without holding our mutex. Access > - * to the user pages requires mmap_lock, and we have > - * a strict lock ordering of mmap_lock, struct_mutex - > - * we already hold struct_mutex here and so cannot > - * call gup without encountering a lock inversion. > - * > - * Userspace will keep on repeating the operation > - * (thanks to EAGAIN) until either we hit the fast > - * path or the worker completes. If the worker is > - * cancelled or superseded, the task is still run > - * but the results ignored. (This leads to > - * complications that we may have a stray object > - * refcount that we need to be wary of when > - * checking for existing objects during creation.) > - * If the worker encounters an error, it reports > - * that error back to this function through > - * obj->userptr.work = ERR_PTR. > - */ > - work = kmalloc(sizeof(*work), GFP_KERNEL); > - if (work == NULL) > - return ERR_PTR(-ENOMEM); > - > - obj->userptr.work = &work->work; > - > - work->obj = i915_gem_object_get(obj); > - > - work->task = current; > - get_task_struct(work->task); > - > - INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); > - queue_work(to_i915(obj->base.dev)->mm.userptr_wq, &work->work); > - > - return ERR_PTR(-EAGAIN); > -} > - > -static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) > -{ > - const unsigned long num_pages = obj->base.size >> PAGE_SHIFT; > - struct mm_struct *mm = obj->userptr.mm->mm; > - struct page **pvec; > - struct sg_table *pages; > - bool active; > - int pinned; > - unsigned int gup_flags = 0; > - > - /* If userspace should engineer that these pages are replaced in > - * the vma between us binding this page into the GTT and completion > - * of rendering... Their loss. If they change the mapping of their > - * pages they need to create a new bo to point to the new vma. > - * > - * However, that still leaves open the possibility of the vma > - * being copied upon fork. Which falls under the same userspace > - * synchronisation issue as a regular bo, except that this time > - * the process may not be expecting that a particular piece of > - * memory is tied to the GPU. > - * > - * Fortunately, we can hook into the mmu_notifier in order to > - * discard the page references prior to anything nasty happening > - * to the vma (discard or cloning) which should prevent the more > - * egregious cases from causing harm. > - */ > - > - if (obj->userptr.work) { > - /* active flag should still be held for the pending work */ > - if (IS_ERR(obj->userptr.work)) > - return PTR_ERR(obj->userptr.work); > - else > - return -EAGAIN; > - } > - > - pvec = NULL; > - pinned = 0; > - > - if (mm == current->mm) { > - pvec = kvmalloc_array(num_pages, sizeof(struct page *), > - GFP_KERNEL | > - __GFP_NORETRY | > - __GFP_NOWARN); > - if (pvec) { > - /* defer to worker if malloc fails */ > - if (!i915_gem_object_is_readonly(obj)) > - gup_flags |= FOLL_WRITE; > - pinned = pin_user_pages_fast_only(obj->userptr.ptr, > - num_pages, gup_flags, > - pvec); > - } > - } > - > - active = false; > - if (pinned < 0) { > - pages = ERR_PTR(pinned); > - 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); > - } > - if (active) > - __i915_gem_userptr_set_active(obj, true); > - > - if (IS_ERR(pages)) > - unpin_user_pages(pvec, pinned); > - kvfree(pvec); > + return 0; > > - return PTR_ERR_OR_ZERO(pages); > +err: > + i915_gem_object_userptr_drop_ref(obj); > +err_free: > + kfree(st); > + return ret; > } > > static void > @@ -599,9 +193,6 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj, > struct sgt_iter sgt_iter; > struct page *page; > > - /* Cancel any inflight work and force them to restart their gup */ > - obj->userptr.work = NULL; > - __i915_gem_userptr_set_active(obj, false); > if (!pages) > return; > > @@ -641,19 +232,161 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj, > } > > mark_page_accessed(page); > - unpin_user_page(page); > } > obj->mm.dirty = false; > > sg_free_table(pages); > kfree(pages); > + > + i915_gem_object_userptr_drop_ref(obj); > +} > + > +static int i915_gem_object_userptr_unbind(struct drm_i915_gem_object *obj, bool get_pages) > +{ > + struct sg_table *pages; > + int err; > + > + err = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE); > + if (err) > + return err; > + > + if (GEM_WARN_ON(i915_gem_object_has_pinned_pages(obj))) > + return -EBUSY; > + > + mutex_lock_nested(&obj->mm.lock, I915_MM_GET_PAGES); > + > + pages = __i915_gem_object_unset_pages(obj); > + if (!IS_ERR_OR_NULL(pages)) > + i915_gem_userptr_put_pages(obj, pages); > + > + if (get_pages) > + err = ____i915_gem_object_get_pages(obj); > + mutex_unlock(&obj->mm.lock); > + > + return err; > +} > + > +int i915_gem_object_userptr_submit_init(struct drm_i915_gem_object *obj) > +{ > + struct drm_i915_private *i915 = to_i915(obj->base.dev); > + const unsigned long num_pages = obj->base.size >> PAGE_SHIFT; > + struct page **pvec; > + unsigned int gup_flags = 0; > + unsigned long notifier_seq; > + int pinned, ret; > + > + if (obj->userptr.notifier.mm != current->mm) > + return -EFAULT; > + > + ret = i915_gem_object_lock_interruptible(obj, NULL); > + if (ret) > + return ret; > + > + /* Make sure userptr is unbound for next attempt, so we don't use stale pages. */ > + ret = i915_gem_object_userptr_unbind(obj, false); > + i915_gem_object_unlock(obj); > + if (ret) > + return ret; > + > + notifier_seq = mmu_interval_read_begin(&obj->userptr.notifier); > + > + pvec = kvmalloc_array(num_pages, sizeof(struct page *), GFP_KERNEL); > + if (!pvec) > + return -ENOMEM; > + > + if (!i915_gem_object_is_readonly(obj)) > + gup_flags |= FOLL_WRITE; > + > + pinned = ret = 0; > + while (pinned < num_pages) { > + ret = pin_user_pages_fast(obj->userptr.ptr + pinned * PAGE_SIZE, > + num_pages - pinned, gup_flags, > + &pvec[pinned]); > + if (ret < 0) > + goto out; > + > + pinned += ret; > + } > + ret = 0; > + > + spin_lock(&i915->mm.notifier_lock); I think we can improve a lot on the locking here by having the object lock protect the object state and only take the driver-wide notifier lock in execbuf / userptr_invalidate. If we in addition use an rwlock as a notifier lock taken in read mode in execbuf, any potential global lock contention can be practically eliminated. But that's perhaps for a future improvement. > + > + if (mmu_interval_read_retry(&obj->userptr.notifier, > + !obj->userptr.page_ref ? notifier_seq : > + obj->userptr.notifier_seq)) { > + ret = -EAGAIN; > + goto out_unlock; > + } > + > + if (!obj->userptr.page_ref++) { > + obj->userptr.pvec = pvec; > + obj->userptr.notifier_seq = notifier_seq; > + > + pvec = NULL; > + } In addition, if we can call get_pages() here to take the page_ref, we can eliminate one page_ref and the use of _userptr_submit_fini(). That would of course require the object lock, but we'd already hold that for the object state as above. > + > +out_unlock: > + spin_unlock(&i915->mm.notifier_lock); > + > +out: > + if (pvec) { > + unpin_user_pages(pvec, pinned); > + kvfree(pvec); > + } > + > + return ret; > +} > + > +int i915_gem_object_userptr_submit_done(struct drm_i915_gem_object *obj) > +{ > + if (mmu_interval_read_retry(&obj->userptr.notifier, > + obj->userptr.notifier_seq)) { > + /* We collided with the mmu notifier, need to retry */ > + > + return -EAGAIN; > + } > + > + return 0; > +} > + > +void i915_gem_object_userptr_submit_fini(struct drm_i915_gem_object *obj) > +{ > + i915_gem_object_userptr_drop_ref(obj); > +} > + > +int i915_gem_object_userptr_validate(struct drm_i915_gem_object *obj) > +{ > + int err; > + > + err = i915_gem_object_userptr_submit_init(obj); > + if (err) > + return err; > + > + err = i915_gem_object_lock_interruptible(obj, NULL); > + if (!err) { > + /* > + * Since we only check validity, not use the pages, > + * it doesn't matter if we collide with the mmu notifier, > + * and -EAGAIN handling is not required. > + */ > + err = i915_gem_object_pin_pages(obj); > + if (!err) > + i915_gem_object_unpin_pages(obj); > + > + i915_gem_object_unlock(obj); > + } > + > + i915_gem_object_userptr_submit_fini(obj); > + return err; > } > > static void > i915_gem_userptr_release(struct drm_i915_gem_object *obj) > { > - i915_gem_userptr_release__mmu_notifier(obj); > - i915_gem_userptr_release__mm_struct(obj); > + GEM_WARN_ON(obj->userptr.page_ref); > + > + mmu_interval_notifier_remove(&obj->userptr.notifier); > + obj->userptr.notifier.mm = NULL; > } > > static int > @@ -686,7 +419,6 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { > .name = "i915_gem_object_userptr", > .flags = I915_GEM_OBJECT_IS_SHRINKABLE | > I915_GEM_OBJECT_NO_MMAP | > - I915_GEM_OBJECT_ASYNC_CANCEL | > I915_GEM_OBJECT_IS_PROXY, > .get_pages = i915_gem_userptr_get_pages, > .put_pages = i915_gem_userptr_put_pages, > @@ -793,6 +525,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev, > i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC); > > obj->userptr.ptr = args->user_ptr; > + obj->userptr.notifier_seq = ULONG_MAX; > if (args->flags & I915_USERPTR_READ_ONLY) > i915_gem_object_set_readonly(obj); > > @@ -800,9 +533,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev, > * at binding. This means that we need to hook into the mmu_notifier > * in order to detect if the mmu is destroyed. > */ > - ret = i915_gem_userptr_init__mm_struct(obj); > - if (ret == 0) > - ret = i915_gem_userptr_init__mmu_notifier(obj); > + ret = i915_gem_userptr_init__mmu_notifier(obj); > if (ret == 0) > ret = drm_gem_handle_create(file, &obj->base, &handle); > > @@ -821,15 +552,7 @@ i915_gem_userptr_ioctl(struct drm_device *dev, > int i915_gem_init_userptr(struct drm_i915_private *dev_priv) > { > #ifdef CONFIG_MMU_NOTIFIER > - spin_lock_init(&dev_priv->mm_lock); > - hash_init(dev_priv->mm_structs); > - > - dev_priv->mm.userptr_wq = > - alloc_workqueue("i915-userptr-acquire", > - WQ_HIGHPRI | WQ_UNBOUND, > - 0); > - if (!dev_priv->mm.userptr_wq) > - return -ENOMEM; > + spin_lock_init(&dev_priv->mm.notifier_lock); > #endif > > return 0; > @@ -837,7 +560,4 @@ int i915_gem_init_userptr(struct drm_i915_private *dev_priv) > > void i915_gem_cleanup_userptr(struct drm_i915_private *dev_priv) > { > -#ifdef CONFIG_MMU_NOTIFIER > - destroy_workqueue(dev_priv->mm.userptr_wq); > -#endif > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index fc41cf6442a9..72927d356c1a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -558,11 +558,10 @@ struct i915_gem_mm { > > #ifdef CONFIG_MMU_NOTIFIER > /** > - * Workqueue to fault in userptr pages, flushed by the execbuf > - * when required but otherwise left to userspace to try again > - * on EAGAIN. > + * notifier_lock for mmu notifiers, memory may not be allocated > + * while holding this lock. > */ > - struct workqueue_struct *userptr_wq; > + spinlock_t notifier_lock; > #endif > > /* shrinker accounting, also useful for userland debugging */ > @@ -942,8 +941,6 @@ struct drm_i915_private { > struct i915_ggtt ggtt; /* VM representing the global address space */ > > struct i915_gem_mm mm; > - DECLARE_HASHTABLE(mm_structs, 7); > - spinlock_t mm_lock; > > /* Kernel Modesetting */ > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 22be1e7bf2dd..6288cd5d898e 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1053,10 +1053,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv) > err_unlock: > i915_gem_drain_workqueue(dev_priv); > > - if (ret != -EIO) { > + if (ret != -EIO) > intel_uc_cleanup_firmwares(&dev_priv->gt.uc); > - i915_gem_cleanup_userptr(dev_priv); > - } > > if (ret == -EIO) { > /* > @@ -1113,7 +1111,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv) > intel_wa_list_free(&dev_priv->gt_wa_list); > > intel_uc_cleanup_firmwares(&dev_priv->gt.uc); > - i915_gem_cleanup_userptr(dev_priv); > > i915_gem_drain_freed_objects(dev_priv); > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx