From patchwork Wed Apr 20 17:13:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Harrison X-Patchwork-Id: 8893321 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 D58269F441 for ; Wed, 20 Apr 2016 17:14:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D5A35201C0 for ; Wed, 20 Apr 2016 17:14:17 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 86B9D20211 for ; Wed, 20 Apr 2016 17:14:16 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9493A6E231; Wed, 20 Apr 2016 17:14:11 +0000 (UTC) X-Original-To: Intel-GFX@lists.freedesktop.org Delivered-To: Intel-GFX@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 15A586EA8B for ; Wed, 20 Apr 2016 17:14:08 +0000 (UTC) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 20 Apr 2016 10:14:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,510,1455004800"; d="scan'208";a="689424341" Received: from johnharr-linux.isw.intel.com ([10.102.226.93]) by FMSMGA003.fm.intel.com with ESMTP; 20 Apr 2016 10:14:06 -0700 From: John.C.Harrison@Intel.com To: Intel-GFX@Lists.FreeDesktop.Org Date: Wed, 20 Apr 2016 18:13:27 +0100 Message-Id: <1461172435-4256-10-git-send-email-John.C.Harrison@Intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1461172435-4256-1-git-send-email-John.C.Harrison@Intel.com> References: <1461172435-4256-1-git-send-email-John.C.Harrison@Intel.com> Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Subject: [Intel-gfx] [PATCH v6 09/34] drm/i915: Added scheduler hook when closing DRM file handles 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=-5.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 From: John Harrison The scheduler decouples the submission of batch buffers to the driver with submission of batch buffers to the hardware. Thus it is possible for an application to close its DRM file handle while there is still work outstanding. That means the scheduler needs to know about file close events so it can remove the file pointer from such orphaned batch buffers and not attempt to dereference it later. v3: Updated to not wait for outstanding work to complete but merely remove the file handle reference. The wait was getting excessively complicated with inter-ring dependencies, pre-emption, and other such issues. v4: Changed some white space to keep the style checker happy. v5: Added function documentation and removed apparently objectionable white space. [Joonas Lahtinen] Used lighter weight spinlocks. v6: Updated to newer nightly (lots of ring -> engine renaming). Added 'for_each_scheduler_node()' helper macro. Updated to use 'to_i915()' instead of dev_private. Removed the return value from i915_scheduler_closefile() as it is not used for anything. [review feedback from Joonas Lahtinen] For: VIZ-1587 Signed-off-by: John Harrison Cc: Joonas Lahtinen Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_dma.c | 2 ++ drivers/gpu/drm/i915/i915_scheduler.c | 45 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_scheduler.h | 1 + 3 files changed, 48 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2ad4071..75dc313 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1509,6 +1509,8 @@ void i915_driver_lastclose(struct drm_device *dev) void i915_driver_preclose(struct drm_device *dev, struct drm_file *file) { + i915_scheduler_closefile(dev, file); + mutex_lock(&dev->struct_mutex); i915_gem_context_close(dev, file); i915_gem_release(dev, file); diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index 9d628b9..6dd9838 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -865,3 +865,48 @@ void i915_scheduler_process_work(struct intel_engine_cs *engine) if (do_submit) intel_runtime_pm_put(dev_priv); } + +/** + * i915_scheduler_closefile - notify the scheduler that a DRM file handle + * has been closed. + * @dev: DRM device + * @file: file being closed + * + * Goes through the scheduler's queues and removes all connections to the + * disappearing file handle that still exist. There is an argument to say + * that this should also flush such outstanding work through the hardware. + * However, with pre-emption, TDR and other such complications doing so + * becomes a locking nightmare. So instead, just warn with a debug message + * if the application is leaking uncompleted work and make sure a null + * pointer dereference will not follow. + */ +void i915_scheduler_closefile(struct drm_device *dev, struct drm_file *file) +{ + struct i915_scheduler_queue_entry *node; + struct drm_i915_private *dev_priv = to_i915(dev); + struct i915_scheduler *scheduler = dev_priv->scheduler; + struct intel_engine_cs *engine; + + if (!scheduler) + return; + + spin_lock_irq(&scheduler->lock); + + for_each_engine(engine, dev_priv) { + for_each_scheduler_node(node, engine->id) { + if (node->params.file != file) + continue; + + if (!I915_SQS_IS_COMPLETE(node)) + DRM_DEBUG_DRIVER("Closing file handle with outstanding work: %d:%d/%d on %s\n", + node->params.request->uniq, + node->params.request->seqno, + node->status, + engine->name); + + node->params.file = NULL; + } + } + + spin_unlock_irq(&scheduler->lock); +} diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h index c895c4c..40398bb 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.h +++ b/drivers/gpu/drm/i915/i915_scheduler.h @@ -105,6 +105,7 @@ enum { bool i915_scheduler_is_enabled(struct drm_device *dev); int i915_scheduler_init(struct drm_device *dev); void i915_scheduler_destroy(struct drm_i915_private *dev_priv); +void i915_scheduler_closefile(struct drm_device *dev, struct drm_file *file); void i915_scheduler_clean_node(struct i915_scheduler_queue_entry *node); int i915_scheduler_queue_execbuffer(struct i915_scheduler_queue_entry *qe); bool i915_scheduler_notify_request(struct drm_i915_gem_request *req);