From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Fix list corruption in vma_unbind Date: Thu, 29 Aug 2013 23:06:24 +0200 Message-ID: <20130829210427.GA8633@phenom.ffwll.local> References: <1377798631-9362-1-git-send-email-daniel.vetter@ffwll.ch> <20130829185738.GJ4726@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f175.google.com (mail-ea0-f175.google.com [209.85.215.175]) by gabe.freedesktop.org (Postfix) with ESMTP id B7797E690F for ; Thu, 29 Aug 2013 14:06:19 -0700 (PDT) Received: by mail-ea0-f175.google.com with SMTP id m14so505829eaj.34 for ; Thu, 29 Aug 2013 14:06:19 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130829185738.GJ4726@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson , Daniel Vetter , Intel Graphics Development , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Thu, Aug 29, 2013 at 07:57:38PM +0100, Chris Wilson wrote: > On Thu, Aug 29, 2013 at 07:50:31PM +0200, Daniel Vetter wrote: > > The saga around the breadcrumb vmas used by execbuf continues ... > > > > This time around we've managed to unconditionally move the object to > > the unbound list on the last vma unbind even though it might never > > have been on either the bound or unbound list. Hilarity ensued. > > > > Chris Wilson tracked this one down but compared to his patches I've > > simply opted to completely separate the unbound case for not-yet bound > > vmas. Otherwise we imo end up with semantically hard to parse checks > > around the list_move_tail(global_list, ...). > > > > This is exercised by the new swapping variants of > > igt/tests/gem_evict_everything. > > > > Cc: Chris Wilson > > Cc: Ben Widawsky > > Bugzilla: https://bugs.freedesktop.org/attachment.cgi?id=84818 > > Signed-off-by: Daniel Vetter > > Reviewed-by: Chris Wilson Merged, but I've dropped the paragraph about the igt tests again - I just can't hit the bug any more :( Also I've fixed the bugzilla link, it pointed at an attachment instead of the bug. -Daniel > > > --- > > drivers/gpu/drm/i915/i915_gem.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index d9eee14..d12dfc3 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -2624,8 +2624,11 @@ int i915_vma_unbind(struct i915_vma *vma) > > if (list_empty(&vma->vma_link)) > > return 0; > > > > - if (!drm_mm_node_allocated(&vma->node)) > > - goto destroy; > > + if (!drm_mm_node_allocated(&vma->node)) { > > + i915_gem_vma_destroy(vma); > > + > > I'm equivocal on this particular whitespace. For such short branches, it > looks neater with a tight return. > > > + return 0; > > + } > > -- > Chris Wilson, Intel Open Source Technology Centre -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch