From patchwork Mon Jan 11 10:44:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 8001291 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 734829F1C0 for ; Mon, 11 Jan 2016 10:47:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 970FB2028D for ; Mon, 11 Jan 2016 10:47:14 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 0E57B202B8 for ; Mon, 11 Jan 2016 10:47:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DA2E86E2DE; Mon, 11 Jan 2016 02:47:08 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id E45826E2D5 for ; Mon, 11 Jan 2016 02:46:58 -0800 (PST) Received: by mail-wm0-f65.google.com with SMTP id f206so25662096wmf.2 for ; Mon, 11 Jan 2016 02:46:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=JHFTjc9XTPAw0D37wq8616jIyeH5bXNwegHCNpO2YZ4=; b=G/ys2VhfFeUjsxdKheDfQ08mOw6OlhD8tRYl3FLNmhEIPEKENl4MoHJMV+uPa1TjvJ uwO3eXYbQPqYyDBRtdsfCUzFGm3zhBh+2vq1zG+WJEds+sd2s/1CbvaR8WjHEmPBiYpu weAi5IvWg+VIWqXXZvUGuqqn8FdhvwR/o916jR7DpcHFsOAo2TzLF+N60GyoDtqKkVSl CuRsGpwB9qHSz2BDfVJuz34kA8rw1KbMl4B2N6LHe9BOJU3Epu+lwmTVg1cVVNZIatyj Ssyh1STHhv0Dgxet6RRCZ0x4XSbDdejQnrR3T8IFpHygHb8Q0/C4qOrIMFapmldwBwWr AGPw== X-Received: by 10.194.175.198 with SMTP id cc6mr26734669wjc.24.1452509217743; Mon, 11 Jan 2016 02:46:57 -0800 (PST) Received: from haswell.alporthouse.com ([78.156.65.138]) by smtp.gmail.com with ESMTPSA id t3sm118879383wjz.11.2016.01.11.02.46.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 11 Jan 2016 02:46:56 -0800 (PST) From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Mon, 11 Jan 2016 10:44:58 +0000 Message-Id: <1452509174-16671-28-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.7.0.rc3 In-Reply-To: <1452509174-16671-1-git-send-email-chris@chris-wilson.co.uk> References: <1452503961-14837-1-git-send-email-chris@chris-wilson.co.uk> <1452509174-16671-1-git-send-email-chris@chris-wilson.co.uk> Subject: [Intel-gfx] [PATCH 114/190] drm/i915: Remove (struct_mutex) locking for wait-ioctl X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 With a bit of care (and leniency) we can iterate over the object and wait for previous rendering to complete with judicial use of atomic reference counting. The ABI requires us to ensure that an active object is eventually flushed (like the busy-ioctl) which is guaranteed by our management of requests (i.e. everything that is submitted to hardware is flushed in the same request). All we have to do is ensure that we can detect when the requests are complete for reporting when the object is idle (without triggering ETIME) - this is handled by __i915_wait_request. The biggest danger in the code is walking the object without holding any locks. We iterate over the set of last requests and carefully grab a reference upon it. (If it is changing beneath us, that is the usual userspace race and even with locking you get the same indeterminate results.) If the request is unreferenced beneath us, it will be disposed of into the request cache - so we have to carefully order the retrieval of the request pointer with its removal, and to do this we employ RCU on the request cache and upon the last_request pointer tracking. The impact of this is actually quite small - the return to userspace following the wait was already lockless. What we achieve here is completing an already finished wait without hitting the struct_mutex, our hold is quite short and so we are typically just a victim of contention rather than a cause. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 52 +++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ee715558ecea..f30207596ec6 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2440,54 +2440,40 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) { struct drm_i915_gem_wait *args = data; struct drm_i915_gem_object *obj; - struct drm_i915_gem_request *req[I915_NUM_RINGS]; - int i, n = 0; - int ret; + int i, ret = 0; if (args->flags != 0) return -EINVAL; - ret = i915_mutex_lock_interruptible(dev); - if (ret) - return ret; - obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->bo_handle)); - if (&obj->base == NULL) { - mutex_unlock(&dev->struct_mutex); + if (&obj->base == NULL) return -ENOENT; - } - /* Need to make sure the object gets inactive eventually. */ - i915_gem_object_flush_active(obj); - if (!i915_gem_object_is_active(obj)) + if (!__I915_BO_ACTIVE(obj)) goto out; - /* Do this after OLR check to make sure we make forward progress polling - * on this IOCTL with a timeout == 0 (like busy ioctl) - */ - if (args->timeout_ns == 0) { - ret = -ETIME; - goto out; - } - + rcu_read_lock(); for (i = 0; i < I915_NUM_RINGS; i++) { - if (obj->last_read[i].request == NULL) + struct drm_i915_gem_request *req; + + req = i915_gem_active_get_request_rcu(&obj->last_read[i]); + if (req == NULL) continue; - req[n++] = i915_gem_request_get(obj->last_read[i].request); + rcu_read_unlock(); + ret = __i915_wait_request(req, true, + args->timeout_ns >= 0 ? &args->timeout_ns : NULL, + to_rps_client(file)); + i915_gem_request_put(req); + if (ret) + goto out; + + rcu_read_lock(); } + rcu_read_unlock(); out: - drm_gem_object_unreference(&obj->base); - mutex_unlock(&dev->struct_mutex); - - for (i = 0; i < n; i++) { - if (ret == 0) - ret = __i915_wait_request(req[i], true, - args->timeout_ns > 0 ? &args->timeout_ns : NULL, - to_rps_client(file)); - i915_gem_request_put(req[i]); - } + drm_gem_object_unreference_unlocked(&obj->base); return ret; }