From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/2] drm/i915: Unpin stolen pages Date: Mon, 3 Jun 2013 10:52:12 +0200 Message-ID: <20130603085212.GK15743@phenom.ffwll.local> References: <20130531184616.GD11399@cantiga.alporthouse.com> <1370036780-13482-1-git-send-email-ben@bwidawsk.net> <1370036780-13482-2-git-send-email-ben@bwidawsk.net> <20130531235102.GA1097@cantiga.alporthouse.com> <20130601113405.GF11399@cantiga.alporthouse.com> <20130603083648.GC17485@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f178.google.com (mail-ea0-f178.google.com [209.85.215.178]) by gabe.freedesktop.org (Postfix) with ESMTP id 9D54BE5D08 for ; Mon, 3 Jun 2013 01:52:17 -0700 (PDT) Received: by mail-ea0-f178.google.com with SMTP id q15so3347345ead.9 for ; Mon, 03 Jun 2013 01:52:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130603083648.GC17485@cantiga.alporthouse.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: Chris Wilson , Daniel Vetter , Ben Widawsky , Intel GFX List-Id: intel-gfx@lists.freedesktop.org On Mon, Jun 03, 2013 at 09:36:48AM +0100, Chris Wilson wrote: > On Sat, Jun 01, 2013 at 03:13:04PM +0200, Daniel Vetter wrote: > > On Sat, Jun 1, 2013 at 1:34 PM, Chris Wilson wrote: > > > On Sat, Jun 01, 2013 at 11:17:10AM +0200, Daniel Vetter wrote: > > >> On Sat, Jun 1, 2013 at 1:51 AM, Chris Wilson wrote: > > >> > That neatly explains the WARN. Not too happy about accumulating lots of > > >> > backing storage specific processing into free_object, but that can be > > >> > fixed up later (there is an obj->ops->release() pending). > > >> > > >> I'm more irked with the semantic overloading of object pinning. Might > > >> be cleaner to otherwise mark stolen obejcts as not shrinkable instead > > >> of pinning them for their entire lifetime. But we can bikeshed that > > >> later on ;-) > > > > > > Some merit to that argument, but it still feels correct to say that the > > > stolen pages are pinned for their lifetime. Given obj->ops->release(), > > > it does actually become simpler to not mess around with pin_count. So > > > later it is. > > > > I was more unhappy that pin_count has different meanings, until I've > > noticed that we've fixed that up already with the introduction of > > ->pages_pin_count. Shouldn't stolen mem just hold a reference on that > > one? After all unbinding from the gtt is ok with stolen memory, but > > dropping the backing storage in the shrinker won't work. Not that we > > currently use stolen for anything else than permanently pinned bos. > > As mentioned on irc, stolen does use the pages_pin_count for its > purposes. The purpose of this patch is purely to allow sanity checking > the pages_pin_count with a WARN_ON during free which seems sensible but > not strictly required. Yeah, silly me didn't read the code before sending a knee-jerk mail ;-) Series merged (hopefully in the right order and all, please check), thanks for the patches and review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch