From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/4] [v2] drm/i915: Remove node only when allocated Date: Wed, 14 Aug 2013 10:06:30 +0200 Message-ID: <20130814080629.GM9296@phenom.ffwll.local> References: <1376442549-5087-1-git-send-email-benjamin.widawsky@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f176.google.com (mail-ea0-f176.google.com [209.85.215.176]) by gabe.freedesktop.org (Postfix) with ESMTP id 6B5C8E7F2A for ; Wed, 14 Aug 2013 01:06:23 -0700 (PDT) Received: by mail-ea0-f176.google.com with SMTP id q16so4564907ead.7 for ; Wed, 14 Aug 2013 01:06:22 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1376442549-5087-1-git-send-email-benjamin.widawsky@intel.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: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org, Ben Widawsky , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Tue, Aug 13, 2013 at 06:09:06PM -0700, Ben Widawsky wrote: > VMAs can be created and not bound. One may think of it as lazy cleanup, > and safely gloss over the conditions which manufacture it. In either > case, when the object backing the i915 vma is destroyed, we must cleanup > the vma without stumbling into a bunch of pitfalls that assume the vma > is bound. > > NOTE: I was pretty certain the above condition could only happen when we > introduced the use of VMAs being looked up at execbuf, and already > existing. Paulo has hit this though, so I must be missing something. As > I believe the patch is correct anyway, therefore I won't scratch my head > too hard. If we end up calling evict_everything from i915_gem_object_bind_to_vm then we'll hit this. One more reason for a testcase here ;-) I'll amend the commit message and merge this. I've also applied a tiny bikeshed I've created while reviewing existing vma_create/destroy callsites. -Daniel > > v2: use goto destroy as a compromise (Chris) > > Cc: Chris Wilson > Cc: Paulo Zanoni > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_gem.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 3d9e248b..4a58ead 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2606,6 +2606,9 @@ 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 (obj->pin_count) > return -EBUSY; > > @@ -2643,6 +2646,8 @@ int i915_vma_unbind(struct i915_vma *vma) > obj->map_and_fenceable = true; > > drm_mm_remove_node(&vma->node); > + > +destroy: > i915_gem_vma_destroy(vma); > > /* Since the unbound list is global, only move to that list if > -- > 1.8.3.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