On Thu, 12 Feb 2015, Daniel Vetter <dan...@ffwll.ch> wrote: > 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.widaw...@intel.com> >> 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 <ch...@chris-wilson.co.uk> >> --- >> 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.vet...@ffwll.ch> > Cc: sta...@vger.kernel.org (we have some vague evidence that it blows up > at last)
Pushed to drm-intel-fixes, thanks for the patch and review. BR, Jani. > > 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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx