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@xxxxxxxxxxxxxxxxxx> > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx