| Message ID | 1448015498-24846-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 20, 2015 at 10:31:38AM +0000, Chris Wilson wrote: > As paranoia, we want to ensure that the CPU's PTEs have been revoked for > the object before we return from i915_gem_release_mmap(). This allows us > to rely on there being no outstanding memory accesses and guarantees > serialisation of the code against concurrent access just by calling > i915_gem_release_mmap(). > > v2: Reduce the mb() into a wmb() following the revoke. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: "Goel, Akash" <akash.goel@intel.com > --- > drivers/gpu/drm/i915/i915_gem.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index df234d00b376..09c829f38786 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1903,11 +1903,21 @@ out: > void > i915_gem_release_mmap(struct drm_i915_gem_object *obj) > { > + /* Serialisation between user GTT access and our code depends upon > + * revoking the CPU's PTE whilst the mutex is held. The next user > + * pagefault then has to wait until we release the mutex. > + */ > + lockdep_assert_held(&obj->base.dev->struct_mutex); lockdep_assert_held is a nop without lockdep, that's why I prefer WARN_ON(!mutex_is_locked). Either way: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + > if (!obj->fault_mappable) > return; > > drm_vma_node_unmap(&obj->base.vma_node, > obj->base.dev->anon_inode->i_mapping); > + > + /* Ensure that the CPU's PTE are revoked before we return */ > + wmb(); > + > obj->fault_mappable = false; > } > > @@ -3212,9 +3222,6 @@ static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj) > if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0) > return; > > - /* Wait for any direct GTT access to complete */ > - mb(); > - > old_read_domains = obj->base.read_domains; > old_write_domain = obj->base.write_domain; > > -- > 2.6.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Nov 24, 2015 at 02:36:43PM +0100, Daniel Vetter wrote: > On Fri, Nov 20, 2015 at 10:31:38AM +0000, Chris Wilson wrote: > > As paranoia, we want to ensure that the CPU's PTEs have been revoked for > > the object before we return from i915_gem_release_mmap(). This allows us > > to rely on there being no outstanding memory accesses and guarantees > > serialisation of the code against concurrent access just by calling > > i915_gem_release_mmap(). > > > > v2: Reduce the mb() into a wmb() following the revoke. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > > Cc: "Goel, Akash" <akash.goel@intel.com > > --- > > drivers/gpu/drm/i915/i915_gem.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index df234d00b376..09c829f38786 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -1903,11 +1903,21 @@ out: > > void > > i915_gem_release_mmap(struct drm_i915_gem_object *obj) > > { > > + /* Serialisation between user GTT access and our code depends upon > > + * revoking the CPU's PTE whilst the mutex is held. The next user > > + * pagefault then has to wait until we release the mutex. > > + */ > > + lockdep_assert_held(&obj->base.dev->struct_mutex); > > lockdep_assert_held is a nop without lockdep, that's why I prefer > WARN_ON(!mutex_is_locked). Either way: I thought someone proposed adding i915_assert_held() or something at one point? Or did I just imagine it? > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > + > > if (!obj->fault_mappable) > > return; > > > > drm_vma_node_unmap(&obj->base.vma_node, > > obj->base.dev->anon_inode->i_mapping); > > + > > + /* Ensure that the CPU's PTE are revoked before we return */ > > + wmb(); > > + > > obj->fault_mappable = false; > > } > > > > @@ -3212,9 +3222,6 @@ static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj) > > if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0) > > return; > > > > - /* Wait for any direct GTT access to complete */ > > - mb(); > > - > > old_read_domains = obj->base.read_domains; > > old_write_domain = obj->base.write_domain; > > > > -- > > 2.6.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index df234d00b376..09c829f38786 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1903,11 +1903,21 @@ out: void i915_gem_release_mmap(struct drm_i915_gem_object *obj) { + /* Serialisation between user GTT access and our code depends upon + * revoking the CPU's PTE whilst the mutex is held. The next user + * pagefault then has to wait until we release the mutex. + */ + lockdep_assert_held(&obj->base.dev->struct_mutex); + if (!obj->fault_mappable) return; drm_vma_node_unmap(&obj->base.vma_node, obj->base.dev->anon_inode->i_mapping); + + /* Ensure that the CPU's PTE are revoked before we return */ + wmb(); + obj->fault_mappable = false; } @@ -3212,9 +3222,6 @@ static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj) if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0) return; - /* Wait for any direct GTT access to complete */ - mb(); - old_read_domains = obj->base.read_domains; old_write_domain = obj->base.write_domain;
As paranoia, we want to ensure that the CPU's PTEs have been revoked for the object before we return from i915_gem_release_mmap(). This allows us to rely on there being no outstanding memory accesses and guarantees serialisation of the code against concurrent access just by calling i915_gem_release_mmap(). v2: Reduce the mb() into a wmb() following the revoke. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: "Goel, Akash" <akash.goel@intel.com --- drivers/gpu/drm/i915/i915_gem.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)