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. -Chris -- Chris Wilson, Intel Open Source Technology Centre
