On Mon, Jun 03, 2013 at 10:52:12AM +0200, Daniel Vetter wrote: > 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 <chris at chris-wilson.co.uk> 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 <chris at chris-wilson.co.uk> 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 Order looks good. Thanks. -- Ben Widawsky, Intel Open Source Technology Center
