On Thu, Feb 12, 2015 at 09:48:23AM +0000, Chris Wilson wrote: > On Thu, Feb 12, 2015 at 10:43:44AM +0100, Daniel Vetter 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? > > It would be possible for us to catch an object in the process of being > executed. More so, we *only* care about GTT pinning here, but still we > need to the lock to prevent that disappearing underneath us.
We still need to grab dev->struct_mutex of course to avoid seeing bo pinned for execbuf. Just thought we could avoid the list walk in set_tiling as a super-micro-opt. > > 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) > > > > 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? > > We killed that over a year ago in the conversion of error capture over > to vma for full-ppgtt prepartion... Right? No, that was left semantically unchanged in the switch. So I guess we should dump vma->pin_count and obj->framebuffer_references? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
