From: Daniel Vetter <daniel@ffwll.ch>
To: Nick Hoath <nicholas.hoath@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Fix an incorrect free rather than derefence issue.
Date: Fri, 13 Feb 2015 10:32:15 +0100 [thread overview]
Message-ID: <20150213093215.GH24485@phenom.ffwll.local> (raw)
In-Reply-To: <1423744161-21554-1-git-send-email-nicholas.hoath@intel.com>
On Thu, Feb 12, 2015 at 12:29:21PM +0000, Nick Hoath wrote:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88652
>
> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
Commit message is missing the absolutely crucial detail about which patch
introduced this regression:
commit 6d3d8274bc45de4babb62d64562d92af984dd238
Author: Nick Hoath <nicholas.hoath@intel.com>
AuthorDate: Thu Jan 15 13:10:39 2015 +0000
drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request
Another thing I've noticed is that we explicitly drop the context
reference for the request before dropping the request reference. Without
clearing the req->ctx pointer. That has a very high chance to leading to
tears, imo the context unreferenceing should be pushed into
i915_gem_request_free.
Except that it's there already, which means we have a double unref now?
Also this patch is for the legacy ringbuffer code, but the referenced bug
is for gen8+ execlists. We're definitely not running this code here I
think.
Imo step one is to drop all the explicit ctx refcounting for req->ctx and
always rely on the implicit reference. Then see what happens.
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1765989..dc10d86 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2661,7 +2661,7 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
> intel_lr_context_unpin(ring, submit_req->ctx);
>
> i915_gem_context_unreference(submit_req->ctx);
> - kfree(submit_req);
> + i915_gem_request_unreference(submit_req);
> }
>
> /*
> --
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-02-13 9:31 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-12 12:29 [PATCH] drm/i915: Fix an incorrect free rather than derefence issue Nick Hoath
2015-02-13 9:32 ` Daniel Vetter [this message]
2015-02-13 9:58 ` Nick Hoath
2015-02-13 12:04 ` Daniel Vetter
2015-02-13 12:28 ` shuang.he
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=20150213093215.GH24485@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=nicholas.hoath@intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox