Re: [PATCH] drm/i915: Remove the failed context from the fpriv->context_idr

From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Remove the failed context from the	fpriv->context_idr
Date: Mon, 10 Aug 2015 19:33:44 +0300	[thread overview]
Message-ID: <87vbcnw1av.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <1439038956-8691-1-git-send-email-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> If we encounter an allocation failure during ppggt creation (trivial
> even with 16Gib+ RAM!), we need to remove the dead context from the
> fpriv->context_idr along with the references.
>
> gem_exec_ctx: page allocation failure: order:0, mode:0x8004
> CPU: 3 PID: 27272 Comm: gem_exec_ctx Tainted: G        W       4.2.0-rc5+ #37
>  0000000000000000 ffff880086ff7a78 ffffffff816b947a ffff88041ed90038
>  0000000000008004 ffff880086ff7b08 ffffffff8114b1a5 ffff880086ff7ac8
>  ffffffff8108d848 0000000000000000 ffffffff81ce84b8 0000000000000000
> Call Trace:
>  [<ffffffff816b947a>] dump_stack+0x45/0x57
>  [<ffffffff8114b1a5>] warn_alloc_failed+0xd5/0x120
>  [<ffffffff8108d848>] ? __wake_up+0x48/0x60
>  [<ffffffff8114e0ed>] __alloc_pages_nodemask+0x73d/0x8e0
>  [<ffffffffc0472238>] ? i915_gem_execbuffer2+0x148/0x240 [i915]
>  [<ffffffffc0474240>] __setup_page_dma+0x30/0x110 [i915]
>  [<ffffffffc0477f61>] gen8_ppgtt_init+0x31/0x2f0 [i915]
>  [<ffffffffc04785e0>] i915_ppgtt_init+0x30/0x80 [i915]
>  [<ffffffffc0478928>] i915_ppgtt_create+0x48/0xc0 [i915]
>  [<ffffffffc046c9c2>] i915_gem_create_context+0x1c2/0x390 [i915]
>  [<ffffffffc046d9cb>] i915_gem_context_create_ioctl+0x5b/0xa0 [i915]
>
> leading to an oops in i915_gem_context_close. Also note that this
> benchmark should not be running out of memory in the first place...
>
> Testcase: igt/benchmark/gem_exec_ctx -b create # ppgtt >= 2
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index d0f6d90f6d0e..d87e422228a5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -420,6 +420,7 @@ err_unpin:
>  	if (vma)
>  		i915_vma_unpin(vma);
                ^^
That would be nice to have.

>  err_destroy:
> +	idr_remove(&file_priv->context_idr, ctx->user_handle);
>  	i915_gem_context_unreference(ctx);
>  	return ERR_PTR(ret);

This is not against drm-intel-next-queued. However in context
of dinq:

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  }
> -- 
> 2.5.0
>
> _______________________________________________
> 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