From patchwork Mon Dec 17 23:58:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1889311 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id F1FAC3FCA5 for ; Tue, 18 Dec 2012 00:14:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752286Ab2LRAN4 (ORCPT ); Mon, 17 Dec 2012 19:13:56 -0500 Received: from mail-ee0-f54.google.com ([74.125.83.54]:53192 "EHLO mail-ee0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751253Ab2LRAN4 (ORCPT ); Mon, 17 Dec 2012 19:13:56 -0500 X-Greylist: delayed 1938 seconds by postgrey-1.27 at vger.kernel.org; Mon, 17 Dec 2012 19:13:55 EST Received: by mail-ee0-f54.google.com with SMTP id c13so1592eek.41 for ; Mon, 17 Dec 2012 16:13:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=Rnvx5AwqwfxUiKExD7GUquSTf3iBIRI2+lk/NYT5TQ4=; b=dGmGFc9Hw43CdX4EgiMmLb0gXZg+MWbbhYPKIQe0ylwiVNz3ZWFVLhHb+2pwdWuY3Y MClMjI+Fv5wx0b9XZukTTbfJP6b2CmFmEIA1Sn6v7N4AJZXaRO27J1ZVWrJXXyYBwqHM IFgzM8Qz6BYrRzaRp+OGsQ6QvDK83VG54YYQo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=Rnvx5AwqwfxUiKExD7GUquSTf3iBIRI2+lk/NYT5TQ4=; b=bTAvT517ggevokP2HHa5v13Sj4OnJQH4Xyjo3UNE5Tiplu6TzzX4Dcdfg1gkfJvohm FLs/aHlXfeVgSqakv6aIFlFYMpVcnv3l6vJP1hVkGJESdsfounuYqjNSS0oLZRNIhEoq mEUHgZPIpsRqrm4guiyU3k6Ne8ximTMILs2N8W0NCeLITkOLGRLDk1I1HfDS7peh7kEZ W7+P3ULKETxM547mlO4B8rkQmd6+/Xu1inDN6zW9TYAGWsAOsDhG9pMRL8RuEqWJOrhK 98/HZpf9A7pbnr6KcwHZBFv8jNiB+JSY5kPT9hH7oDcGrydZcStYNaemneMk7O2daLja loqQ== X-Received: by 10.14.173.65 with SMTP id u41mr475520eel.13.1355789295028; Mon, 17 Dec 2012 16:08:15 -0800 (PST) Received: from wespe.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id r1sm14377eeo.2.2012.12.17.16.08.13 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 17 Dec 2012 16:08:14 -0800 (PST) From: Daniel Vetter To: DRI Development , linaro-mm-sig@lists.linaro.org, linux-media@vger.kernel.org Cc: LKML , Daniel Vetter , Aaron Plattner Subject: [PATCH] [RFC] dma-buf: implement vmap refcounting in the interface logic Date: Tue, 18 Dec 2012 00:58:35 +0100 Message-Id: <1355788715-22177-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.11.7 In-Reply-To: <1355787110-19968-1-git-send-email-daniel.vetter@ffwll.ch> References: <1355787110-19968-1-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQmnH8rUrSgrdNUcAcVFVaZNYjJ9MDRkVl77mjsKcL2Cwk2dzFdvVtf0ITvYqx7xvHI4gQrt Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org All drivers which implement this need to have some sort of refcount to allow concurrent vmap usage. Hence implement this in the dma-buf core. To protect against concurrent calls we need a lock, which potentially causes new funny locking inversions. But this shouldn't be a problem for exporters with statically allocated backing storage, and more dynamic drivers have decent issues already anyway. Inspired by some refactoring patches from Aaron Plattner, who implemented the same idea, but only for drm/prime drivers. v2: Check in dma_buf_release that no dangling vmaps are left. Suggested by Aaron Plattner. We might want to do similar checks for attachments, but that's for another patch. Also fix up ERR_PTR return for vmap. v3: Check whether the passed-in vmap address matches with the cached one for vunmap. Eventually we might want to remove that parameter - compared to the kmap functions there's no need for the vaddr for unmapping. Suggested by Chris Wilson. Cc: Aaron Plattner Signed-off-by: Daniel Vetter --- Compile-tested only - Aaron has been bugging me too a bit too often about this on irc. Cheers, Daniel --- Documentation/dma-buf-sharing.txt | 6 +++++- drivers/base/dma-buf.c | 43 ++++++++++++++++++++++++++++++++++----- include/linux/dma-buf.h | 4 +++- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt index 0188903..4966b1b 100644 --- a/Documentation/dma-buf-sharing.txt +++ b/Documentation/dma-buf-sharing.txt @@ -302,7 +302,11 @@ Access to a dma_buf from the kernel context involves three steps: void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) The vmap call can fail if there is no vmap support in the exporter, or if it - runs out of vmalloc space. Fallback to kmap should be implemented. + runs out of vmalloc space. Fallback to kmap should be implemented. Note that + the dma-buf layer keeps a reference count for all vmap access and calls down + into the exporter's vmap function only when no vmapping exists, and only + unmaps it once. Protection against concurrent vmap/vunmap calls is provided + by taking the dma_buf->lock mutex. 3. Finish access diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index a3f79c4..67d3cd5 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -39,6 +39,8 @@ static int dma_buf_release(struct inode *inode, struct file *file) dmabuf = file->private_data; + BUG_ON(dmabuf->vmapping_counter); + dmabuf->ops->release(dmabuf); kfree(dmabuf); return 0; @@ -482,12 +484,34 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap); */ void *dma_buf_vmap(struct dma_buf *dmabuf) { + void *ptr; + if (WARN_ON(!dmabuf)) return NULL; - if (dmabuf->ops->vmap) - return dmabuf->ops->vmap(dmabuf); - return NULL; + if (!dmabuf->ops->vmap) + return NULL; + + mutex_lock(&dmabuf->lock); + if (dmabuf->vmapping_counter) { + dmabuf->vmapping_counter++; + BUG_ON(!dmabuf->vmap_ptr); + ptr = dmabuf->vmap_ptr; + goto out_unlock; + } + + BUG_ON(dmabuf->vmap_ptr); + + ptr = dmabuf->ops->vmap(dmabuf); + if (IS_ERR_OR_NULL(ptr)) + goto out_unlock; + + dmabuf->vmap_ptr = ptr; + dmabuf->vmapping_counter = 1; + +out_unlock: + mutex_unlock(&dmabuf->lock); + return ptr; } EXPORT_SYMBOL_GPL(dma_buf_vmap); @@ -501,7 +525,16 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) if (WARN_ON(!dmabuf)) return; - if (dmabuf->ops->vunmap) - dmabuf->ops->vunmap(dmabuf, vaddr); + BUG_ON(!dmabuf->vmap_ptr); + BUG_ON(dmabuf->vmapping_counter > 0); + BUG_ON(dmabuf->vmap_ptr != vaddr); + + mutex_lock(&dmabuf->lock); + if (--dmabuf->vmapping_counter == 0) { + if (dmabuf->ops->vunmap) + dmabuf->ops->vunmap(dmabuf, vaddr); + dmabuf->vmap_ptr = NULL; + } + mutex_unlock(&dmabuf->lock); } EXPORT_SYMBOL_GPL(dma_buf_vunmap); diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index bd2e52c..e3bf2f6 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -119,8 +119,10 @@ struct dma_buf { struct file *file; struct list_head attachments; const struct dma_buf_ops *ops; - /* mutex to serialize list manipulation and attach/detach */ + /* mutex to serialize list manipulation, attach/detach and vmap/unmap */ struct mutex lock; + unsigned vmapping_counter; + void *vmap_ptr; void *priv; };