* [PATCH] drm/i915: Encourage our shrinker more when our shmemfs allocations fails
@ 2017-05-20 9:33 Chris Wilson
2017-05-20 9:53 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-05-22 8:23 ` [PATCH] " Joonas Lahtinen
0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2017-05-20 9:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Commit 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than
incur the oom for gfx allocations") made the bold decision to try and
avoid the oomkiller by reporting -ENOMEM to userspace if our allocation
failed after attempting to free enough buffer objects. In short, it
appears we were giving up too easily (even before we start wondering if
one pass of reclaim is as strong as we would like). Part of the problem
is that if we only shrink just enough pages for our expected allocation,
the likelihood of those pages becoming available to us is less than 100%
To counter-act that we ask for twice the number of pages to be made
available. Furthermore, we allow the shrinker to pull pages from the
active list in later passes.
Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem.c | 50 ++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a637cc05cc4a..a975422f23f6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2337,8 +2337,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
struct page *page;
unsigned long last_pfn = 0; /* suppress gcc warning */
unsigned int max_segment;
+ gfp_t noreclaim;
int ret;
- gfp_t gfp;
/* Assert that the object is not currently in any GPU domain. As it
* wasn't in the GTT, there shouldn't be any way it could have been in
@@ -2367,22 +2367,32 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
* Fail silently without starting the shrinker
*/
mapping = obj->base.filp->f_mapping;
- gfp = mapping_gfp_constraint(mapping, ~(__GFP_IO | __GFP_RECLAIM));
- gfp |= __GFP_NORETRY | __GFP_NOWARN;
+ noreclaim = mapping_gfp_constraint(mapping,
+ ~(__GFP_IO | __GFP_RECLAIM));
+ noreclaim |= __GFP_NORETRY | __GFP_NOWARN;
+
sg = st->sgl;
st->nents = 0;
for (i = 0; i < page_count; i++) {
- page = shmem_read_mapping_page_gfp(mapping, i, gfp);
- if (unlikely(IS_ERR(page))) {
- i915_gem_shrink(dev_priv,
- page_count,
- I915_SHRINK_BOUND |
- I915_SHRINK_UNBOUND |
- I915_SHRINK_PURGEABLE);
+ const unsigned int shrink[] = {
+ I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE,
+ I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE | I915_SHRINK_ACTIVE,
+ I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE,
+ 0,
+ }, *s = shrink;
+ gfp_t gfp = noreclaim;
+
+ do {
page = shmem_read_mapping_page_gfp(mapping, i, gfp);
- }
- if (unlikely(IS_ERR(page))) {
- gfp_t reclaim;
+ if (likely(!IS_ERR(page)))
+ break;
+
+ if (!*s) {
+ ret = PTR_ERR(page);
+ goto err_sg;
+ }
+
+ i915_gem_shrink(dev_priv, 2 * page_count, *s++);
/* We've tried hard to allocate the memory by reaping
* our own buffer, now let the real VM do its job and
@@ -2392,15 +2402,13 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
* defer the oom here by reporting the ENOMEM back
* to userspace.
*/
- reclaim = mapping_gfp_mask(mapping);
- reclaim |= __GFP_NORETRY; /* reclaim, but no oom */
-
- page = shmem_read_mapping_page_gfp(mapping, i, reclaim);
- if (IS_ERR(page)) {
- ret = PTR_ERR(page);
- goto err_sg;
+ if (!*s) {
+ /* reclaim and warn, but no oom */
+ gfp = mapping_gfp_mask(mapping);
+ gfp |= __GFP_NORETRY;
}
- }
+ } while (1);
+
if (!i ||
sg->length >= max_segment ||
page_to_pfn(page) != last_pfn + 1) {
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Encourage our shrinker more when our shmemfs allocations fails
2017-05-20 9:33 [PATCH] drm/i915: Encourage our shrinker more when our shmemfs allocations fails Chris Wilson
@ 2017-05-20 9:53 ` Patchwork
2017-05-22 8:23 ` [PATCH] " Joonas Lahtinen
1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2017-05-20 9:53 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Encourage our shrinker more when our shmemfs allocations fails
URL : https://patchwork.freedesktop.org/series/24740/
State : success
== Summary ==
Series 24740v1 drm/i915: Encourage our shrinker more when our shmemfs allocations fails
https://patchwork.freedesktop.org/api/1.0/series/24740/revisions/1/mbox/
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:449s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:432s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:587s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:511s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:491s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:487s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:419s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:412s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:423s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:496s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:464s
fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:466s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:460s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:572s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:470s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:499s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:438s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:536s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:407s
b3bb44f6882d7ba6e1ae2fab5ab2c49b38cddd5e drm-tip: 2017y-05m-19d-19h-37m-15s UTC integration manifest
3af975c drm/i915: Encourage our shrinker more when our shmemfs allocations fails
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4760/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Encourage our shrinker more when our shmemfs allocations fails
2017-05-20 9:33 [PATCH] drm/i915: Encourage our shrinker more when our shmemfs allocations fails Chris Wilson
2017-05-20 9:53 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-05-22 8:23 ` Joonas Lahtinen
2017-05-22 8:46 ` Chris Wilson
1 sibling, 1 reply; 4+ messages in thread
From: Joonas Lahtinen @ 2017-05-22 8:23 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter
On la, 2017-05-20 at 10:33 +0100, Chris Wilson wrote:
> Commit 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than
> incur the oom for gfx allocations") made the bold decision to try and
> avoid the oomkiller by reporting -ENOMEM to userspace if our allocation
> failed after attempting to free enough buffer objects. In short, it
> appears we were giving up too easily (even before we start wondering if
> one pass of reclaim is as strong as we would like). Part of the problem
> is that if we only shrink just enough pages for our expected allocation,
> the likelihood of those pages becoming available to us is less than 100%
> To counter-act that we ask for twice the number of pages to be made
> available. Furthermore, we allow the shrinker to pull pages from the
> active list in later passes.
>
> Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
2x factor seems like it might backfire in the future, I think I'd be
more comfortable with some fixed amount.
I'm also not sure if this logic should be outside of i915 module, needs
some A-b at least.
Code itself is,
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Encourage our shrinker more when our shmemfs allocations fails
2017-05-22 8:23 ` [PATCH] " Joonas Lahtinen
@ 2017-05-22 8:46 ` Chris Wilson
0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2017-05-22 8:46 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: Daniel Vetter, intel-gfx
On Mon, May 22, 2017 at 11:23:58AM +0300, Joonas Lahtinen wrote:
> On la, 2017-05-20 at 10:33 +0100, Chris Wilson wrote:
> > Commit 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than
> > incur the oom for gfx allocations") made the bold decision to try and
> > avoid the oomkiller by reporting -ENOMEM to userspace if our allocation
> > failed after attempting to free enough buffer objects. In short, it
> > appears we were giving up too easily (even before we start wondering if
> > one pass of reclaim is as strong as we would like). Part of the problem
> > is that if we only shrink just enough pages for our expected allocation,
> > the likelihood of those pages becoming available to us is less than 100%
> > To counter-act that we ask for twice the number of pages to be made
> > available. Furthermore, we allow the shrinker to pull pages from the
> > active list in later passes.
> >
> > Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> 2x factor seems like it might backfire in the future, I think I'd be
> more comfortable with some fixed amount.
>
> I'm also not sure if this logic should be outside of i915 module, needs
> some A-b at least.
The idea for doing it here was to priorities reclaim from the driver for
our allocations. It is also outside via the shrinker. but we have
neutered the direct reclaim paths to avoid stalling on the gpu "unnecessarily".
Hmm, once upon a time I had a patch to do
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index da79ebcc8ba0..08023da14e70 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -322,6 +322,12 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
sc->nr_to_scan - freed,
I915_SHRINK_BOUND |
I915_SHRINK_UNBOUND);
+ if (freed < sc->nr_to_scan && current_is_kswapd())
+ freed += i915_gem_shrink(dev_priv,
+ sc->nr_to_scan - freed,
+ I915_SHRINK_ACTIVE |
+ I915_SHRINK_BOUND |
+ I915_SHRINK_PURGEABLE);
if (unlock)
mutex_unlock(&dev->struct_mutex);
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-22 8:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-20 9:33 [PATCH] drm/i915: Encourage our shrinker more when our shmemfs allocations fails Chris Wilson
2017-05-20 9:53 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-05-22 8:23 ` [PATCH] " Joonas Lahtinen
2017-05-22 8:46 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.