From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 2/9] drm/i915: Extract node allocation from bind Date: Wed, 7 May 2014 09:00:16 -0700 Message-ID: <20140507160015.GD5147@bwidawsk.net> References: <1399440098-17378-1-git-send-email-benjamin.widawsky@intel.com> <1399440098-17378-2-git-send-email-benjamin.widawsky@intel.com> <20140507070238.GD7322@nuc-i3427.alporthouse.com> <20140507154536.GB5147@bwidawsk.net> <20140507155308.GH7322@nuc-i3427.alporthouse.com> 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 722356E53E for ; Wed, 7 May 2014 09:00:25 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140507155308.GH7322@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Ben Widawsky , Intel GFX List-Id: intel-gfx@lists.freedesktop.org On Wed, May 07, 2014 at 04:53:08PM +0100, Chris Wilson wrote: > On Wed, May 07, 2014 at 08:45:38AM -0700, Ben Widawsky wrote: > > On Wed, May 07, 2014 at 08:02:38AM +0100, Chris Wilson wrote: > > > On Tue, May 06, 2014 at 10:21:31PM -0700, Ben Widawsky wrote: > > > > The DRM node allocation code was already a bit of an ugly bit of code > > > > within a complex function. Removing it serves the purpose of cleaning > > > > the function up. More importantly, it provides a way to have a > > > > preallocated (address space) VMA to easily skip this code. Something > > > > we're very likely to need. > > > > > > > > This is essentially a wrapper for DRM node allocation with an eviction + > > > > retry. It is something which could be moved to core DRM eventually, if > > > > other drivers had similar eviction semantics. > > > > > > > > This removes a goto used for something other than error unwinding, a > > > > generally reviled (I am neutral) practice, and replaces it with a > > > > function call to itself that should have the same effect. Note that it's > > > > not a recursive function as all the problem set reduction is done in the > > > > eviction code. > > > > > > > > Some might say this change is worse than before because we are using > > > > stack for each subsequent call. Frankly, I'd rather overflow the stack > > > > and blow it up than loop forever. In either case, this is addressed in > > > > the next patch. > > > > > > > > I believe, and intend, that other than the stack usage, there is no > > > > functional change here. > > > > > > Nope, but decoupling evict_something() further does introduce lots more > > > implied magic. > > > -Chris > > > > > > > Hmm. I really don't see what's actually upsetting. Can you be a bit more > > explicit about what's so bothersome to you for my edification? > > evict_something() make assumptions about the ranges of the vm which it > searches. At the moment, they mirror its parent's function. > -Chris > Ah, thanks. So is plumbing starting eviction offset into evict something an appealing solution here? > -- > Chris Wilson, Intel Open Source Technology Centre -- Ben Widawsky, Intel Open Source Technology Center