public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: miku@iki.fi
Subject: [RFC] drm/i915: reference count batch object on requests
Date: Mon,  2 Dec 2013 17:31:53 +0200	[thread overview]
Message-ID: <1385998313-5783-1-git-send-email-mika.kuoppala@intel.com> (raw)
In-Reply-To: <1385995666-3541-1-git-send-email-mika.kuoppala@intel.com>

We used to lean on active_list to handle the references
to batch objects. But there are useful cases when same,
albeit simple, batch can be executing on multiple rings
concurrently. For this case the active_list reference count
handling is just not enough as batch could be freed by
ring A request retirement as it is still running on ring B.

Fix this by doing proper batch_obj reference counting.

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

Notes:
    This is a patch which ameliorates the
    [PATCH] tests/gem_reset_stats: add close-pending-fork
    
    Chris wasn't happy about the refcounting as it might hide
    the true problem. But I haven't been able to find the real culprit,
    thus the RFC.
---
 drivers/gpu/drm/i915/i915_gem.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 40d9dcf..858538f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2145,13 +2145,12 @@ int __i915_add_request(struct intel_ring_buffer *ring,
 	request->head = request_start;
 	request->tail = request_ring_position;
 
-	/* Whilst this request exists, batch_obj will be on the
-	 * active_list, and so will hold the active reference. Only when this
-	 * request is retired will the the batch_obj be moved onto the
-	 * inactive_list and lose its active reference. Hence we do not need
-	 * to explicitly hold another reference here.
+	/* Active list has one reference but that is not enough as same
+	 * batch_obj can be active on multiple rings
 	 */
 	request->batch_obj = obj;
+	if (request->batch_obj)
+		drm_gem_object_reference(&request->batch_obj->base);
 
 	/* Hold a reference to the current context so that we can inspect
 	 * it later in case a hangcheck error event fires.
@@ -2340,6 +2339,9 @@ static void i915_gem_free_request(struct drm_i915_gem_request *request)
 	if (request->ctx)
 		i915_gem_context_unreference(request->ctx);
 
+	if (request->batch_obj)
+		drm_gem_object_unreference(&request->batch_obj->base);
+
 	kfree(request);
 }
 
-- 
1.7.9.5

  parent reply	other threads:[~2013-12-02 15:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-02 14:47 [PATCH] tests/gem_reset_stats: add close-pending-fork Mika Kuoppala
2013-12-02 15:03 ` Chris Wilson
2013-12-02 16:32   ` Mika Kuoppala
2013-12-03 17:03     ` Daniel Vetter
2013-12-04 14:39       ` Mika Kuoppala
2013-12-04 15:48         ` Daniel Vetter
2013-12-02 15:31 ` Mika Kuoppala [this message]
2013-12-03 17:10   ` [RFC] drm/i915: reference count batch object on requests Daniel Vetter
2013-12-04 11:24     ` Chris Wilson
2013-12-04 12:08       ` Daniel Vetter
2013-12-04 12:11         ` Chris Wilson
2013-12-04 13:28     ` Mika Kuoppala

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=1385998313-5783-1-git-send-email-mika.kuoppala@intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=miku@iki.fi \
    /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