All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Unpin last_context at reset
Date: Mon, 23 Jun 2014 13:07:50 +0300	[thread overview]
Message-ID: <20140623100750.GU27580@intel.com> (raw)
In-Reply-To: <1403118289-19550-1-git-send-email-ville.syrjala@linux.intel.com>

On Wed, Jun 18, 2014 at 10:04:48PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're forgetting to unpin the last_context from the ggtt at GPU reset
> time. This leads to the vma pin_count leaking at every reset if the
> last context wasn't the ring default context. Further use of the same
> context will trigger the pin_count check in i915_gem_object_pin() and
> userspace will be faced with EBUSY as a result.
> 
> This plaques kms_flip rather badly since it performs lots of resets,
> and every fd has its own default context these days.
> 
> Fix the problem by properly unpinning the last context at reset.
> 
> Testcase: igt/gem_ctx_exec/reset-pin-leak
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I pushed the igt, and also kms_flip hits this easily.

It looks to me that the bug has been there since the introduction of
i915_gem_context_reset() in [1], so I think we may want this patch
merged with cc: stable. Afterwards people can work on killing
i915_gem_context_reset() if they so wish.

[1]
 commit acce9ffa4807027965ebd948456fa8385bbee32e
 Author: Ben Widawsky <ben@bwidawsk.net>
 Date:   Fri Dec 6 14:11:03 2013 -0800

    drm/i915: Better reset handling for contexts

> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 3ffe308..e362c96 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -382,6 +382,9 @@ void i915_gem_context_reset(struct drm_device *dev)
>  			dctx->obj->active = 0;
>  		}
>  
> +		if (ring->last_context->obj && i == RCS)
> +			i915_gem_object_ggtt_unpin(ring->last_context->obj);
> +
>  		i915_gem_context_unreference(ring->last_context);
>  		i915_gem_context_reference(dctx);
>  		ring->last_context = dctx;
> -- 
> 1.8.5.5

-- 
Ville Syrjälä
Intel OTC

  parent reply	other threads:[~2014-06-23 10:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-18 19:04 [PATCH] drm/i915: Unpin last_context at reset ville.syrjala
2014-06-18 19:04 ` [PATCH igt] tests/gem_ctx_exec: Add reset-pin-leak subtest ville.syrjala
2014-06-19  7:47 ` [PATCH] drm/i915: Unpin last_context at reset Chris Wilson
2014-06-19 16:21   ` Mateo Lozano, Oscar
2014-06-23 10:07 ` Ville Syrjälä [this message]
2014-07-07 14:38   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140623100750.GU27580@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.