From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 1/4] [v2] drm/i915: Remove node only when allocated Date: Thu, 15 Aug 2013 14:42:42 -0700 Message-ID: <20130815214241.GA22301@bwidawsk.net> References: <1376442549-5087-1-git-send-email-benjamin.widawsky@intel.com> <20130814080629.GM9296@phenom.ffwll.local> <20130814081550.GN9296@phenom.ffwll.local> <20130815140556.GD776@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail.bwidawsk.net (bwidawsk.net [166.78.191.112]) by gabe.freedesktop.org (Postfix) with ESMTP id 79D7EE5FDB for ; Thu, 15 Aug 2013 14:42:47 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130815140556.GD776@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: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Thu, Aug 15, 2013 at 04:05:56PM +0200, Daniel Vetter wrote: > On Wed, Aug 14, 2013 at 10:15:50AM +0200, Daniel Vetter wrote: > > 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. > > Ok, I've merged the entire pile. I think now's the time to throw a bit of > igt on top to exercise the corner cases here ... > -Daniel Was that a specific request for me to do something, I missed the message if so? When should I rework the remaining patches and resend? -- Ben Widawsky, Intel Open Source Technology Center