All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations
@ 2017-03-22 11:05 Chris Wilson
  2017-03-22 11:23 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-03-22 13:11 ` [PATCH] " Joonas Lahtinen
  0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2017-03-22 11:05 UTC (permalink / raw)
  To: intel-gfx

Since gfx allocations tend to be large, unmovable and disposable, report
the allocation failure back to userspace as an ENOMEM rather than incur
the oomkiller. We have already tried to make room by purging our own
cached gfx objects, and the oomkiller doesn't attribute ownership of gfx
objects so will likely pick the wrong candidate. Instead, let userspace
see the ENOMEM.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bb65072f688d..9d710bd8c6c9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2321,7 +2321,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 	st->nents = 0;
 	for (i = 0; i < page_count; i++) {
 		page = shmem_read_mapping_page_gfp(mapping, i, gfp);
-		if (IS_ERR(page)) {
+		if (unlikely(IS_ERR(page))) {
 			i915_gem_shrink(dev_priv,
 					page_count,
 					I915_SHRINK_BOUND |
@@ -2329,12 +2329,21 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 					I915_SHRINK_PURGEABLE);
 			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
 		}
-		if (IS_ERR(page)) {
+		if (unlikely(IS_ERR(page))) {
+			gfp_t reclaim;
+
 			/* We've tried hard to allocate the memory by reaping
 			 * our own buffer, now let the real VM do its job and
 			 * go down in flames if truly OOM.
+			 *
+			 * However, since graphics tend to be disposable,
+			 * defer the oom here by reporting the ENOMEM back
+			 * to userspace.
 			 */
-			page = shmem_read_mapping_page(mapping, i);
+			reclaim = mapping_gfp_constraint(mapping, 0);
+			reclaim |= __GFP_NORETRY; /* reclaim, but no oom */
+
+			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
 			if (IS_ERR(page)) {
 				ret = PTR_ERR(page);
 				goto err_sg;
-- 
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] 5+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations
  2017-03-22 11:05 [PATCH] drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations Chris Wilson
@ 2017-03-22 11:23 ` Patchwork
  2017-03-22 13:11 ` [PATCH] " Joonas Lahtinen
  1 sibling, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-03-22 11:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations
URL   : https://patchwork.freedesktop.org/series/21681/
State : success

== Summary ==

Series 21681v1 drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations
https://patchwork.freedesktop.org/api/1.0/series/21681/revisions/1/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 455s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 580s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 534s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 543s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 502s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 497s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 435s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 429s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 515s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 492s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 488s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 478s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 598s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 484s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 529s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 417s

1595c9b1bafa002ccfaa484f1dd3b2d7b9303a64 drm-tip: 2017y-03m-22d-09h-09m-59s UTC integration manifest
1003f0f drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4261/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations
  2017-03-22 11:05 [PATCH] drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations Chris Wilson
  2017-03-22 11:23 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-03-22 13:11 ` Joonas Lahtinen
  2017-03-22 13:24   ` Daniel Vetter
  1 sibling, 1 reply; 5+ messages in thread
From: Joonas Lahtinen @ 2017-03-22 13:11 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Vetter, Daniel

On ke, 2017-03-22 at 11:05 +0000, Chris Wilson wrote:
> Since gfx allocations tend to be large, unmovable and disposable, report
> the allocation failure back to userspace as an ENOMEM rather than incur
> the oomkiller. We have already tried to make room by purging our own
> cached gfx objects, and the oomkiller doesn't attribute ownership of gfx
> objects so will likely pick the wrong candidate. Instead, let userspace
> see the ENOMEM.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

A-b from Daniel.

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] 5+ messages in thread

* Re: [PATCH] drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations
  2017-03-22 13:11 ` [PATCH] " Joonas Lahtinen
@ 2017-03-22 13:24   ` Daniel Vetter
  2017-03-22 19:35     ` Chris Wilson
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2017-03-22 13:24 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: Vetter, Daniel, intel-gfx

On Wed, Mar 22, 2017 at 03:11:58PM +0200, Joonas Lahtinen wrote:
> On ke, 2017-03-22 at 11:05 +0000, Chris Wilson wrote:
> > Since gfx allocations tend to be large, unmovable and disposable, report
> > the allocation failure back to userspace as an ENOMEM rather than incur
> > the oomkiller. We have already tried to make room by purging our own
> > cached gfx objects, and the oomkiller doesn't attribute ownership of gfx
> > objects so will likely pick the wrong candidate. Instead, let userspace
> > see the ENOMEM.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> A-b from Daniel.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

We suck. Oh well, Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Eventually I guess we should try to fix the accounting of gem bo ...

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations
  2017-03-22 13:24   ` Daniel Vetter
@ 2017-03-22 19:35     ` Chris Wilson
  0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-03-22 19:35 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Vetter, Daniel, intel-gfx

On Wed, Mar 22, 2017 at 02:24:51PM +0100, Daniel Vetter wrote:
> On Wed, Mar 22, 2017 at 03:11:58PM +0200, Joonas Lahtinen wrote:
> > On ke, 2017-03-22 at 11:05 +0000, Chris Wilson wrote:
> > > Since gfx allocations tend to be large, unmovable and disposable, report
> > > the allocation failure back to userspace as an ENOMEM rather than incur
> > > the oomkiller. We have already tried to make room by purging our own
> > > cached gfx objects, and the oomkiller doesn't attribute ownership of gfx
> > > objects so will likely pick the wrong candidate. Instead, let userspace
> > > see the ENOMEM.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > A-b from Daniel.
> > 
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> We suck. Oh well, Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Been mulling this over. It's not 100% (mostly deliberate in that I
didn't adjust the mapping's gfp and just this allocations), so some
oomkiller still creeps in, but it does greatly improve predictability of
failure. The only drawback is where a small client is unable to allocate
memory -- however, we already strived to recover enough memory from our
own pool, and gfx for the large part are recoverable, and userspace
should take ENOMEM in its stride. (Give or take the SIGBUS, but you get
those anyway on out of memory during GTT mmaps, and I only know of one
driver that took pains to handle that.) Still better than indescriminate
SIGKILL.

With that in mind, pushed.
-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	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-22 19:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-22 11:05 [PATCH] drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations Chris Wilson
2017-03-22 11:23 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-03-22 13:11 ` [PATCH] " Joonas Lahtinen
2017-03-22 13:24   ` Daniel Vetter
2017-03-22 19:35     ` 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.