| Message ID | 1392383167-19948-3-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 14, 2014 at 02:06:07PM +0100, Daniel Vetter wrote: > Otherwise we end up tearing down fences when e.g. the client quits > way too early. Might or might not fix a fence pin_count BUG Ville has > reported. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> This one should also have: Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 675ad96a43e1..fa00b26a9cf7 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2731,12 +2731,14 @@ int i915_vma_unbind(struct i915_vma *vma) > * cause memory corruption through use-after-free. > */ > > - i915_gem_object_finish_gtt(obj); > + if (i915_is_ggtt(vma->vm)) { > + i915_gem_object_finish_gtt(obj); > > - /* release the fence reg _after_ flushing */ > - ret = i915_gem_object_put_fence(obj); > - if (ret) > - return ret; > + /* release the fence reg _after_ flushing */ > + ret = i915_gem_object_put_fence(obj); > + if (ret) > + return ret; > + } > > trace_i915_vma_unbind(vma); > > -- > 1.8.5.2
On Fri, Feb 14, 2014 at 02:06:07PM +0100, Daniel Vetter wrote: > Otherwise we end up tearing down fences when e.g. the client quits > way too early. Might or might not fix a fence pin_count BUG Ville has > reported. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Good catch. I am slightly worried that our management of GGTT is a little haphazard, but given the slightly differing use-cases in i915_vma_unbind and set_cache_level, this looks cleaner than any alternative I quickly sketched. This and the preceding patches are Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Fri, Feb 14, 2014 at 02:06:07PM +0100, Daniel Vetter wrote: > Otherwise we end up tearing down fences when e.g. the client quits > way too early. Might or might not fix a fence pin_count BUG Ville has > reported. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> For patches 1 and 3: Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 675ad96a43e1..fa00b26a9cf7 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2731,12 +2731,14 @@ int i915_vma_unbind(struct i915_vma *vma) > * cause memory corruption through use-after-free. > */ > > - i915_gem_object_finish_gtt(obj); > + if (i915_is_ggtt(vma->vm)) { > + i915_gem_object_finish_gtt(obj); > > - /* release the fence reg _after_ flushing */ > - ret = i915_gem_object_put_fence(obj); > - if (ret) > - return ret; > + /* release the fence reg _after_ flushing */ > + ret = i915_gem_object_put_fence(obj); > + if (ret) > + return ret; > + } > > trace_i915_vma_unbind(vma); > > -- > 1.8.5.2
On Wed, May 14, 2014 at 07:14:13PM +0300, Ville Syrjälä wrote: > On Fri, Feb 14, 2014 at 02:06:07PM +0100, Daniel Vetter wrote: > > Otherwise we end up tearing down fences when e.g. the client quits > > way too early. Might or might not fix a fence pin_count BUG Ville has > > reported. > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > For patches 1 and 3: > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Merged, thanks for the review and writing the testcase. -Daniel > > > --- > > drivers/gpu/drm/i915/i915_gem.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index 675ad96a43e1..fa00b26a9cf7 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -2731,12 +2731,14 @@ int i915_vma_unbind(struct i915_vma *vma) > > * cause memory corruption through use-after-free. > > */ > > > > - i915_gem_object_finish_gtt(obj); > > + if (i915_is_ggtt(vma->vm)) { > > + i915_gem_object_finish_gtt(obj); > > > > - /* release the fence reg _after_ flushing */ > > - ret = i915_gem_object_put_fence(obj); > > - if (ret) > > - return ret; > > + /* release the fence reg _after_ flushing */ > > + ret = i915_gem_object_put_fence(obj); > > + if (ret) > > + return ret; > > + } > > > > trace_i915_vma_unbind(vma); > > > > -- > > 1.8.5.2 > > -- > Ville Syrjälä > Intel OTC
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 675ad96a43e1..fa00b26a9cf7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2731,12 +2731,14 @@ int i915_vma_unbind(struct i915_vma *vma) * cause memory corruption through use-after-free. */ - i915_gem_object_finish_gtt(obj); + if (i915_is_ggtt(vma->vm)) { + i915_gem_object_finish_gtt(obj); - /* release the fence reg _after_ flushing */ - ret = i915_gem_object_put_fence(obj); - if (ret) - return ret; + /* release the fence reg _after_ flushing */ + ret = i915_gem_object_put_fence(obj); + if (ret) + return ret; + } trace_i915_vma_unbind(vma);
Otherwise we end up tearing down fences when e.g. the client quits way too early. Might or might not fix a fence pin_count BUG Ville has reported. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_gem.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)