On Thu, Feb 12, 2015 at 07:53:18AM +0000, Chris Wilson wrote: > When we walk the list of vma, or even for protecting against concurrent > framebuffer creation, we must hold the struct_mutex or else a second > thread can corrupt the list as we walk it. > > Fixes regression from > commit d7f46fc4e7323887494db13f063a8e59861fefb0 > Author: Ben Widawsky <benjamin.widawsky@xxxxxxxxx> > Date: Fri Dec 6 14:10:55 2013 -0800 > > drm/i915: Make pin count per VMA > > References: https://bugs.freedesktop.org/show_bug.cgi?id=89085 > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_gem_tiling.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c > index 7a24bd1a51f6..6377b22269ad 100644 > --- a/drivers/gpu/drm/i915/i915_gem_tiling.c > +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c > @@ -335,9 +335,10 @@ i915_gem_set_tiling(struct drm_device *dev, void *data, > return -EINVAL; > } > > + mutex_lock(&dev->struct_mutex); > if (i915_gem_obj_is_pinned(obj) || obj->framebuffer_references) { Since the removal of userspace pinning we shouldn't be able to see pinned objects here which are _not_ framebuffers too. But we still need the lock for synchronization and to avoid races, but perhaps we could drop the list walk? Either way this is Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Cc: stable@xxxxxxxxxxxxxxx (we have some vague evidence that it blows up at last) I've also audited all the other callers of is_pinned, the only other suspicious one is the one in capture_bo. Perhaps we should also move that over to obj->framebuffer_references? Cheers, Daniel > - drm_gem_object_unreference_unlocked(&obj->base); > - return -EBUSY; > + ret = -EBUSY; > + goto err; > } > > if (args->tiling_mode == I915_TILING_NONE) { > @@ -369,7 +370,6 @@ i915_gem_set_tiling(struct drm_device *dev, void *data, > } > } > > - mutex_lock(&dev->struct_mutex); > if (args->tiling_mode != obj->tiling_mode || > args->stride != obj->stride) { > /* We need to rebind the object if its current allocation > @@ -424,6 +424,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data, > obj->bit_17 = NULL; > } > > +err: > drm_gem_object_unreference(&obj->base); > mutex_unlock(&dev->struct_mutex); > > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
