[Intel-gfx] [PATCH] drm/i915: Avoid GPU stalls from kswapd

[Chris Wilson]

On Mon, Jun 22, 2015 at 08:41:05PM +0100, Dave Gordon wrote:
> On 22/06/15 16:35, Daniel Vetter wrote:
> > On Mon, Jun 22, 2015 at 03:18:12PM +0100, Chris Wilson wrote:
> >> On Mon, Jun 22, 2015 at 04:11:01PM +0200, Daniel Vetter wrote:
> >>> On Fri, Jun 19, 2015 at 02:04:16PM +0100, Chris Wilson wrote:
> >>>> Exclude active GPU pages from the purview of the background shrinker
> >>>> (kswapd), as these cause uncontrollable GPU stalls. Given that the
> >>>> shrinker is rerun until the freelists are satisfied, we should have
> >>>> opportunity in subsequent passes to recover the pages once idle. If the
> >>>> machine does run out of memory entirely, we have the forced idling in the
> >>>> oom-notifier as a means of releasing all the pages we can before an oom
> >>>> is prematurely executed.
> >>>>
> >>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>>> ---
> >>>>  drivers/gpu/drm/i915/i915_drv.h          | 1 +
> >>>>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 8 ++++++--
> >>>>  2 files changed, 7 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>>> index 71f4ca5088e2..e0dcd018379f 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>>> @@ -3185,6 +3185,7 @@ unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv,
> >>>>  #define I915_SHRINK_PURGEABLE 0x1
> >>>>  #define I915_SHRINK_UNBOUND 0x2
> >>>>  #define I915_SHRINK_BOUND 0x4
> >>>> +#define I915_SHRINK_ACTIVE 0x8
> >>>>  unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
> >>>>  void i915_gem_shrinker_init(struct drm_i915_private *dev_priv);
> >>>>  
> >>>> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>>> index bd1cf921aead..8d25ec8a6559 100644
> >>>> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>>> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>>> @@ -123,6 +123,10 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
> >>>>  			    obj->madv != I915_MADV_DONTNEED)
> >>>>  				continue;
> >>>>  
> >>>> +			if ((flags & I915_SHRINK_ACTIVE) == 0 &&
> >>>
> >>> Isn't there a "I'm kswapd" process flag we could use to make the shrinker
> >>> a bit more clever between synchronous reclaim and kswap reclaim? Or has
> >>> synchronous reclaim died as a thing?
> >>
> >> If we cared we can use the lock-stealer as a flag for when waiting may
> >> be acceptable. But the better heuristic imo is to delay the stall until
> >> all other sources of cache memory have been exhausted. Then either
> >> kswapd or direct reclaim will flush the GPU in preference to killing
> >> processes.
> > 
> > Well I'm more concerned about fairness since atm the shrinker stall gives
> > us a very crude throttling of gpu processes. It makes sense not to stall
> > kswapd though since that would just punish the system overall for no
> > useful purpose at all. But punishing active processes would imo be a
> > feature. Since we hold the BKL while shrinking that'll also automatically
> > stall and gem users.
> > -Daniel
> 
> GPU scheduler should give us some throttling and fairness :)

Completely different topic, of concern here is fairness wrt to memory
allocation and pagecache thrashing. What you meant to say anyway was a
CFS would enforce fairness, a deadline scheduler would provide some
sembalance of predictability, and the current nop scheduler relies on
clients behaving cooperatively.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre