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
next prev 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