[Intel-gfx] [PATCH 06/16] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock

[Chris Wilson]

Quoting Chris Wilson (2017-08-12 11:26:08)
> Quoting Joonas Lahtinen (2017-08-10 17:22:49)
> > On ke, 2017-07-26 at 14:26 +0100, Chris Wilson wrote:
> > > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > > @@ -92,9 +92,6 @@ static bool swap_available(void)
> > >  
> > >  static bool can_release_pages(struct drm_i915_gem_object *obj)
> > >  {
> > > -     if (!i915_gem_object_has_pages(obj))
> > > -             return false;
> > 
> > So you think the inaccuracies we get for being lockless don't matter
> > compared to better forward progress?
> 
> Actually, looking at this again, it is all improvement. We no longer
> have a scan over the inactive/active list here, just the bound/unbound
> list, so the locking is equivalent. And we gain better accounting going
> in (no longer do we have an untracked phase for objects that have pages
> prior to ever seeing the struct_mutex and being placed on the unbound_list.
> 
> We still have an unpleasant walk over many 10s of thousands of objects.
> Hmm, I wonder if sticking a cond_resched_lock() in there is sensible.
> Probably.

Scratch that, too complicated. If we do end up rescheduling, the count
may be even more wrong, or we restart entirely defeating the purpose.

It's not even that useful. Every time the system decides to reclaim a
page, we free a random number of pages. Not that I have a better answer,
just unease that we end up sacrificing too many of our own objects for
the buffercache.  At the back of my mind, I've considered bumping the
shrinker->batch to say 1024 (equivalent to 4MiB), but the heuristic is so
nebulous it's hard to know the impact it will have on the balance between
slabs.
-Chris