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

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

[Chris Wilson]

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@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 &&
+			    obj->active)
+				continue;
+
 			drm_gem_object_reference(&obj->base);
 
 			/* For the unbound phase, this should be a no-op! */
@@ -166,7 +170,7 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
 	i915_gem_retire_requests(dev_priv->dev);
 
 	return i915_gem_shrink(dev_priv, LONG_MAX,
-			       I915_SHRINK_BOUND | I915_SHRINK_UNBOUND);
+			       I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE);
 }
 
 static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
@@ -221,7 +225,7 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
 			count += obj->base.size >> PAGE_SHIFT;
 
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		if (obj->pages_pin_count == num_vma_bound(obj))
+		if (!obj->active && obj->pages_pin_count == num_vma_bound(obj))
 			count += obj->base.size >> PAGE_SHIFT;
 	}
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid GPU stalls from kswapd

[Damien Lespiau]

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@chris-wilson.co.uk>

I'm not familiar at all with the shrinker code, but that patch seems to
do what's described in the commit message. It also sounds like a good
idea to not try to shrink the currently active objects and stall because
of that. So:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  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 &&
> +			    obj->active)
> +				continue;
> +
>  			drm_gem_object_reference(&obj->base);
>  
>  			/* For the unbound phase, this should be a no-op! */
> @@ -166,7 +170,7 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
>  	i915_gem_retire_requests(dev_priv->dev);
>  
>  	return i915_gem_shrink(dev_priv, LONG_MAX,
> -			       I915_SHRINK_BOUND | I915_SHRINK_UNBOUND);
> +			       I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE);
>  }
>  
>  static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
> @@ -221,7 +225,7 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
>  			count += obj->base.size >> PAGE_SHIFT;
>  
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> -		if (obj->pages_pin_count == num_vma_bound(obj))
> +		if (!obj->active && obj->pages_pin_count == num_vma_bound(obj))
>  			count += obj->base.size >> PAGE_SHIFT;
>  	}
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid GPU stalls from kswapd

[Daniel Vetter]

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@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?
-Daniel

> +			    obj->active)
> +				continue;
> +
>  			drm_gem_object_reference(&obj->base);
>  
>  			/* For the unbound phase, this should be a no-op! */
> @@ -166,7 +170,7 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
>  	i915_gem_retire_requests(dev_priv->dev);
>  
>  	return i915_gem_shrink(dev_priv, LONG_MAX,
> -			       I915_SHRINK_BOUND | I915_SHRINK_UNBOUND);
> +			       I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE);
>  }
>  
>  static bool i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
> @@ -221,7 +225,7 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
>  			count += obj->base.size >> PAGE_SHIFT;
>  
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> -		if (obj->pages_pin_count == num_vma_bound(obj))
> +		if (!obj->active && obj->pages_pin_count == num_vma_bound(obj))
>  			count += obj->base.size >> PAGE_SHIFT;
>  	}
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid GPU stalls from kswapd

[Chris Wilson]

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@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.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid GPU stalls from kswapd

[Daniel Vetter]

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@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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid GPU stalls from kswapd

[Chris Wilson]

On Mon, Jun 22, 2015 at 05:35:11PM +0200, 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@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.

But you are not punishing individual GPU processes, but all GPU
processes, so it doesn't seem that fair to me. In terms of mm,
throttling is performed on processes when they dirty pages. If we want
to hook into the system sense of fairness, we should look to similarly
charge every process as it accesses the bo for the first time (since
put_pages at least).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Avoid GPU stalls from kswapd

[Dave Gordon]

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@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 :)

.Dave.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx