From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 1/2] [RFC] drm/i915: Restore LRU evict order, with a twist! Date: Thu, 01 Jul 2010 21:36:41 +0100 Message-ID: <89k77n$o7tsl8@fmsmga001.fm.intel.com> References: <1278003225-937-1-git-send-email-chris@chris-wilson.co.uk> <877hlfhtiq.fsf@pollan.anholt.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 4334B9E801 for ; Thu, 1 Jul 2010 13:36:45 -0700 (PDT) In-Reply-To: <877hlfhtiq.fsf@pollan.anholt.net> 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: Eric Anholt , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 01 Jul 2010 12:49:33 -0700, Eric Anholt wrote: > On Thu, 1 Jul 2010 17:53:44 +0100, Chris Wilson wrote: > > +static int > > +i915_gem_eviction_roster_add(struct i915_gem_eviction_roster *roster, > > + struct drm_i915_gem_object *obj_priv) > > +{ > > This function could stand a brief comment of what it does. But frankly, > a lot of this file looks like a rewrite of drm_mm.c -- is there a reason > not to reuse that for free space tracking? I only sent this patch simply because it was the one in my tree and has seen some testing prior to forward porting to the current drm-intel-next. (Though obviously not any testing on HAS_BSD hardware!). Daniel Vetter reworked drm_mm.c for the same purpose, which the change in eviction logic here could reuse once that was ready. > Only downside seems to be > doing another lookup to figure out which objects take up the space when > we decide on which block to free, since the merging of blocks wouldn't > be aware of our list of objects we want to attach. IIRC, Daniel used a rollback to undo the incomplete evictions. I'll port his work to the current tree and rebase upon it. > (but then, if we're going to talk about cpu efficiency, it seems like we > could do a lot better than this implementation of _search that we call > over and over to try to find the a block with the same size/align > parameters each time) The fun we could have, though clarity here is paramount. Making the wrong choice over which object to evict costs far more in clflush than we could reasonably expend searching... Thanks for the review. This is something that I feel does need to be in shape for the next merge window as the 2D code is still susceptible to the page-fault-of-doom. -- Chris Wilson, Intel Open Source Technology Centre