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:15:50 +0200 Message-ID: <20130814081550.GN9296@phenom.ffwll.local> References: <1376442549-5087-1-git-send-email-benjamin.widawsky@intel.com> <20130814080629.GM9296@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f182.google.com (mail-ea0-f182.google.com [209.85.215.182]) by gabe.freedesktop.org (Postfix) with ESMTP id 16275E6281 for ; Wed, 14 Aug 2013 01:15:42 -0700 (PDT) Received: by mail-ea0-f182.google.com with SMTP id o10so4617617eaj.41 for ; Wed, 14 Aug 2013 01:15:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130814080629.GM9296@phenom.ffwll.local> 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 Wed, Aug 14, 2013 at 10:06:30AM +0200, Daniel Vetter wrote: > 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. Actually evict_everything isn't in the callpath, and there's no memory allocation where the shrinker might play havoc. Furthermore the pages are pinned so the shrinker shouldn't be able to sneak in at all. This is a bit unsettling, I need to think more about this. I'll wait with merging this for now. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch