From patchwork Wed Jul 24 13:43:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 2832761 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id EF95D9F243 for ; Wed, 24 Jul 2013 13:57:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 491B4201CB for ; Wed, 24 Jul 2013 13:57:06 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 597E7201B5 for ; Wed, 24 Jul 2013 13:57:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 37799E6A85 for ; Wed, 24 Jul 2013 06:57:04 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ee0-f41.google.com (mail-ee0-f41.google.com [74.125.83.41]) by gabe.freedesktop.org (Postfix) with ESMTP id EB7D3E68AB for ; Wed, 24 Jul 2013 06:44:40 -0700 (PDT) Received: by mail-ee0-f41.google.com with SMTP id d51so256006eek.0 for ; Wed, 24 Jul 2013 06:44:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=QdKO+VPv1yGpDDUIl7rbJchsmrgKvPZstIkmFU/Y4Dg=; b=e6PQAp26st1VUec+oqlV15qjE6JrIzeHwe/r4x6Jg30NMuIMBJ08BHoibKcWKwxyVm /XyMic384kXSaJHZC4VFG+OM2zqeBOANuRoSKcOZgVNolsx8UZVP7TJHx5FaLOp8jTmg Lxl85t5XgmqJNI1gJGVbTq/E9hrbP54Fi8EXUREGzBECE59FHccFsZnPsn5eiAAaJuXd jYs9CqMOJFcNrJT0JmbogB7WEBFK9XTGHpcUA5EPs/DpiNpsUI4s1wwAmU+8D1Y8tyES dDquv9NfMbZkkuy6sd+8gzv3+Jy408tlZCET47l/PLoKgs9i2Ui1xZGfkrP9Jr/xzMw2 8CGQ== X-Received: by 10.14.32.197 with SMTP id o45mr38085477eea.9.1374673480240; Wed, 24 Jul 2013 06:44:40 -0700 (PDT) Received: from localhost.localdomain (stgt-5f718d03.pool.mediaWays.net. [95.113.141.3]) by mx.google.com with ESMTPSA id w43sm66276500eez.6.2013.07.24.06.44.38 for (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 24 Jul 2013 06:44:39 -0700 (PDT) From: David Herrmann To: dri-devel@lists.freedesktop.org Subject: [RFC 9/9] drm: support immediate unplugging Date: Wed, 24 Jul 2013 15:43:58 +0200 Message-Id: <1374673438-26251-10-git-send-email-dh.herrmann@gmail.com> X-Mailer: git-send-email 1.8.3.3 In-Reply-To: <1374673438-26251-1-git-send-email-dh.herrmann@gmail.com> References: <1374673438-26251-1-git-send-email-dh.herrmann@gmail.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, 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 Our current unplug-helpers remove all nodes from user-space and mark the device as unplugged. However, any pending user-space call is still continued. We wait for the last close() call to actually unload the device. This patch uses the drm_dev_get_active() infrastructure to allow immediate unplugging. Whenever drm_put_dev() or drm_unplug_dev() is called, we now call drm_dev_unregister(). This waits for outstanding user-space calls, marks the device as unplugged, "fake"-closes all open files, zaps mmaps and unregisters the device. If there are pending open-files, we will mark them as invalid but keep them around. The drm_release() callback will notice that and free the device when the last open-file is closed. So we still keep the device allocated as we did before, but we no longer keep it registered. This is analogous to driver-core which also keeps devices around (until last put_device() call), but allows immediate device deregistration via unregister_device(). Signed-off-by: David Herrmann --- Documentation/DocBook/drm.tmpl | 5 ++ drivers/gpu/drm/drm_fops.c | 85 +++++++++++++++++--------- drivers/gpu/drm/drm_stub.c | 133 ++++++++++++++++++++++++++++++----------- include/drm/drmP.h | 13 +--- 4 files changed, 162 insertions(+), 74 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 7d1278e..f668f0f 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -439,6 +439,11 @@ char *date; + + DRM Device Management +!Pdrivers/gpu/drm/drm_stub.c device management +!Edrivers/gpu/drm/drm_stub.c + diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index b75af7d..d6af563 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -477,34 +477,30 @@ int drm_lastclose(struct drm_device * dev) } /** - * Release file. + * drm_close() - Close file + * @filp: Open file to close * - * \param inode device inode - * \param file_priv DRM file private. - * \return zero on success or a negative number on failure. + * Close all open handles and clean up all resources associated with this open + * file. The open-file must not be used after this call returns. + * + * This does not actually free "file_priv" or "file_priv->minor" so following + * user-space entries can still test for file_priv->minor->dev and see whether + * it is valid. + * + * You should free "file_priv" in the real file ->release() callback. * - * If the hardware lock is held then free it, and take it again for the kernel - * context since it's necessary to reclaim buffers. Unlink the file private - * data from its list and free it. Decreases the open count and if it reaches - * zero calls drm_lastclose(). + * Caller must hold the global DRM mutex. */ -int drm_release(struct inode *inode, struct file *filp) +void drm_close(struct file *filp) { struct drm_file *file_priv = filp->private_data; struct drm_device *dev = file_priv->minor->dev; - int retcode = 0; - - mutex_lock(&drm_global_mutex); DRM_DEBUG("open_count = %d\n", dev->open_count); if (dev->driver->preclose) dev->driver->preclose(dev, file_priv); - /* ======================================================== - * Begin inline drm_release - */ - DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n", task_pid_nr(current), (long)old_encode_dev(file_priv->minor->device), @@ -599,26 +595,57 @@ int drm_release(struct inode *inode, struct file *filp) drm_prime_destroy_file_private(&file_priv->prime); put_pid(file_priv->pid); - kfree(file_priv); +} +EXPORT_SYMBOL(drm_close); - /* ======================================================== - * End inline drm_release - */ +/** + * drm_release - Release file + * @inode: Inode of DRM device node + * @filp: Open file to close + * + * Release callback which should be used for DRM device file-ops. Calls + * drm_close() for @filp and releases the DRM device if is is unplugged and we + * are the last user. + * + * RETURNS: + * This always returns 0. + */ +int drm_release(struct inode *inode, struct file *filp) +{ + struct drm_file *file_priv = filp->private_data; + struct drm_device *dev = file_priv->minor->dev; + bool active; + + mutex_lock(&drm_global_mutex); + /* If the device is still active, our context is valid and we need to + * close it properly. If it is not active, drm_dev_unregister() will + * have called drm_close() for us already (protected by + * drm_global_mutex). So skip it in this case. */ + active = drm_dev_get_active(dev); + + if (active) + drm_close(filp); + kfree(file_priv); + + /* If we are the last open-file and the device is still active, + * call lastclose() and continue. If the device is unplugged, + * then drm_dev_unregister() already called lastclose() and we + * can finally free the device as we are the last user. */ atomic_inc(&dev->counts[_DRM_STAT_CLOSES]); if (!--dev->open_count) { - if (atomic_read(&dev->ioctl_count)) { - DRM_ERROR("Device busy: %d\n", - atomic_read(&dev->ioctl_count)); - retcode = -EBUSY; - } else - retcode = drm_lastclose(dev); - if (drm_device_is_unplugged(dev)) - drm_put_dev(dev); + if (active) + drm_lastclose(dev); + else + drm_dev_free(dev); } + + if (active) + drm_dev_put_active(dev); + mutex_unlock(&drm_global_mutex); - return retcode; + return 0; } EXPORT_SYMBOL(drm_release); diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index c0e76c0..85a0292 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -1,13 +1,4 @@ -/** - * \file drm_stub.h - * Stub support - * - * \author Rickard E. (Rik) Faith - */ - /* - * Created: Fri Jan 19 10:48:35 2001 by faith@acm.org - * * Copyright 2001 VA Linux Systems, Inc., Sunnyvale, California. * All Rights Reserved. * @@ -29,6 +20,45 @@ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER * DEALINGS IN THE SOFTWARE. + * + * Authors: + * Rickard E. (Rik) Faith + */ + +/** + * DOC: device management + * + * DRM core provides basic device management and defines the lifetime. A bus + * driver is responsible of finding suitable devices and reacting to hotplug + * events. The normal device-core operations are available for DRM devices: + * drm_dev_alloc() allocates a new device which can be freed via drm_dev_free(). + * This device is not advertised to user-space and is not considered active + * until you call drm_dev_register(). This call will start the DRM device and + * notify user-space about it. Once a device is registered, any device callbacks + * are considered active and may be entered. + * + * For platform drivers, this is all you need to do. For hotpluggable drivers, a + * bus needs to listen for unplug events. Once a device is unplugged, use + * drm_dev_unregister() to deactivate the device, interrupt all pending + * user-space processes waiting for the device and mark the device as gone. Once + * the device is unregistered, DRM core will take care of freeing it so you must + * not access it afterwards. + * + * DRM core tracks device usage via drm_dev_get_active() and + * drm_dev_put_active(). Every user-space entry point must mark a device as + * active as long as it is used. The DRM-core helpers do this automatically, but + * if a driver provides their own file-ops, they must take care of this. DRM + * core guarantees that drm_dev_unregister() will not be called as long as the + * device is active. But if you don't mark the device as active, it may get + * unregistered (and even freed!) at any time. + * + * Drivers must use the ->load() and ->unload() callbacks to allocate/free + * private device resources. DRM core does not provide device ref-counting as + * this isn't needed for now. Instead, drivers must assume a device is gone once + * ->unload() was called. Internally, a device is kept alive until + * drm_dev_unregister() is called. It is kept allocated until + * drm_dev_unregister() is called _and_ the last user-space process closed the + * last node of the device. */ #include @@ -342,6 +372,9 @@ int drm_put_minor(struct drm_minor **minor_p) { struct drm_minor *minor = *minor_p; + if (!minor) + return 0; + DRM_DEBUG("release secondary minor %d\n", minor->index); if (minor->type == DRM_MINOR_LEGACY) @@ -362,7 +395,8 @@ EXPORT_SYMBOL(drm_put_minor); static void drm_unplug_minor(struct drm_minor *minor) { - drm_sysfs_device_remove(minor); + if (minor) + drm_sysfs_device_remove(minor); } /** @@ -374,33 +408,20 @@ static void drm_unplug_minor(struct drm_minor *minor) */ void drm_put_dev(struct drm_device *dev) { - DRM_DEBUG("\n"); - - if (!dev) { - DRM_ERROR("cleanup called no dev\n"); - return; - } - drm_dev_unregister(dev); - drm_dev_free(dev); } EXPORT_SYMBOL(drm_put_dev); +/** + * drm_unplug_dev() - Unplug DRM device + * @dev: Device to unplug + * + * Virtually unplug DRM device, mark it as invalid and wait for any pending + * user-space actions. @dev might be freed after this returns. + */ void drm_unplug_dev(struct drm_device *dev) { - /* for a USB device */ - if (drm_core_check_feature(dev, DRIVER_MODESET)) - drm_unplug_minor(dev->control); - drm_unplug_minor(dev->primary); - - mutex_lock(&drm_global_mutex); - - drm_device_set_unplugged(dev); - - if (dev->open_count == 0) { - drm_put_dev(dev); - } - mutex_unlock(&drm_global_mutex); + drm_dev_unregister(dev); } EXPORT_SYMBOL(drm_unplug_dev); @@ -495,6 +516,9 @@ EXPORT_SYMBOL(drm_dev_alloc); */ void drm_dev_free(struct drm_device *dev) { + drm_put_minor(&dev->control); + drm_put_minor(&dev->primary); + if (dev->driver->driver_features & DRIVER_GEM) drm_gem_destroy(dev); @@ -585,10 +609,21 @@ EXPORT_SYMBOL(drm_dev_register); * Mark DRM device as unplugged, wait for any pending user request and then * unregister the DRM device from the system. This does the reverse of * drm_dev_register(). + * + * If there is no pending open-file, this also frees the DRM device by calling + * drm_dev_free(). Otherwise, it leaves the device allocated and the last real + * ->release() callback will free the device. */ void drm_dev_unregister(struct drm_device *dev) { struct drm_map_list *r_list, *list_temp; + struct drm_file *file; + + /* Take fake open_count to prevent concurrent drm_release() calls from + * freeing the device. */ + mutex_lock(&drm_global_mutex); + ++dev->open_count; + mutex_unlock(&drm_global_mutex); /* Wait for pending users and then mark the device as unplugged. We * must not hold the global-mutex while doing this. Otherwise, calls @@ -597,6 +632,26 @@ void drm_dev_unregister(struct drm_device *dev) dev->unplugged = true; percpu_up_write(&dev->active); + mutex_lock(&drm_global_mutex); + + /* By setting "unplugged" we guarantee that no new drm_open will + * succeed. We also know, that no user-space process can call into a DRM + * device, anymore. So instead of waiting for the last close() we + * simulate close() for all active users. + * This allows us to unregister the device immediately but wait for the + * last real release to free it actually. */ + while (!list_empty(&dev->filelist)) { + file = list_entry(dev->filelist.next, struct drm_file, lhead); + + /* wake up pending tasks in drm_read() */ + wake_up_interruptible(&file->event_wait); + drm_close(file->filp); + } + + /* zap all memory mappings */ + if (dev->dev_mapping) + unmap_mapping_range(dev->dev_mapping, 0, ULLONG_MAX, 1); + drm_lastclose(dev); if (dev->driver->unload) @@ -610,12 +665,20 @@ void drm_dev_unregister(struct drm_device *dev) list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) drm_rmmap(dev, r_list->map); - if (drm_core_check_feature(dev, DRIVER_MODESET)) - drm_put_minor(&dev->control); - - drm_put_minor(&dev->primary); + /* Only unplug minors, do not free them. Following drm_open() calls + * access file_priv->minor->dev to see whether the device is still + * active so keep it around. drm_dev_free() frees them eventually. */ + drm_unplug_minor(dev->control); + drm_unplug_minor(dev->primary); list_del(&dev->driver_item); + + /* Release our fake open_count. If there are no pending open-files, + * free the device directly. */ + if (!--dev->open_count) + drm_dev_free(dev); + + mutex_unlock(&drm_global_mutex); } EXPORT_SYMBOL(drm_dev_unregister); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 9689173..7e13d5d 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1212,7 +1212,7 @@ struct drm_device { /** \name Hotplug Management */ /*@{ */ struct percpu_rw_semaphore active; /**< protect active users */ - atomic_t unplugged; /* device has been unplugged or gone away */ + bool unplugged; /* device has been unplugged or gone away */ /*@} */ }; @@ -1250,17 +1250,9 @@ static inline int drm_core_has_MTRR(struct drm_device *dev) #define drm_core_has_MTRR(dev) (0) #endif -static inline void drm_device_set_unplugged(struct drm_device *dev) -{ - smp_wmb(); - atomic_set(&dev->unplugged, 1); -} - static inline int drm_device_is_unplugged(struct drm_device *dev) { - int ret = atomic_read(&dev->unplugged); - smp_rmb(); - return ret; + return dev->unplugged; } static inline bool drm_modeset_is_locked(struct drm_device *dev) @@ -1287,6 +1279,7 @@ extern int drm_fasync(int fd, struct file *filp, int on); extern ssize_t drm_read(struct file *filp, char __user *buffer, size_t count, loff_t *offset); extern int drm_release(struct inode *inode, struct file *filp); +extern void drm_close(struct file *filp); /* Mapping support (drm_vm.h) */ extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);