From patchwork Mon Dec 2 15:31:53 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mika Kuoppala X-Patchwork-Id: 3265931 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 65716C0D4A for ; Mon, 2 Dec 2013 15:32:41 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C032D20259 for ; Mon, 2 Dec 2013 15:32:36 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 8F53720258 for ; Mon, 2 Dec 2013 15:32:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C48BFFAB8C; Mon, 2 Dec 2013 07:32:34 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 7546EFABB1 for ; Mon, 2 Dec 2013 07:32:33 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 02 Dec 2013 07:32:06 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.93,811,1378882800"; d="scan'208";a="445365660" Received: from rosetta.fi.intel.com (HELO rosetta) ([10.237.72.65]) by orsmga002.jf.intel.com with ESMTP; 02 Dec 2013 07:32:04 -0800 Received: by rosetta (Postfix, from userid 1000) id 7049A8006F; Mon, 2 Dec 2013 17:32:03 +0200 (EET) From: Mika Kuoppala To: intel-gfx@lists.freedesktop.org Date: Mon, 2 Dec 2013 17:31:53 +0200 Message-Id: <1385998313-5783-1-git-send-email-mika.kuoppala@intel.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1385995666-3541-1-git-send-email-mika.kuoppala@intel.com> References: <1385995666-3541-1-git-send-email-mika.kuoppala@intel.com> Cc: miku@iki.fi Subject: [Intel-gfx] [RFC] drm/i915: reference count batch object on requests X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 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); }