dma-buf: implement vmap refcounting in the interface logic

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. v4: Fix a brown-paper-bag bug spotted by Aaron Plattner. Cc: Aaron Plattner <aplattner@nvidia.com> Reviewed-by: Aaron Plattner <aplattner@nvidia.com> Tested-by: Aaron Plattner <aplattner@nvidia.com> Reviewed-by: Rob Clark <rob@ti.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>

dma-buf: implement vmap refcounting in the interface logic
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. v4: Fix a brown-paper-bag bug spotted by Aaron Plattner. Cc: Aaron Plattner <aplattner@nvidia.com> Reviewed-by: Aaron Plattner <aplattner@nvidia.com> Tested-by: Aaron Plattner <aplattner@nvidia.com> Reviewed-by: Rob Clark <rob@ti.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
Daniel Vetter · Sumit Semwal
1 parent d895cb1af1
Showing 3 changed files with 46 additions and 7 deletions Side-by-side Diff
Documentation/dma-buf-sharing.txt
drivers/base/dma-buf.c
include/linux/dma-buf.h
@@ -302,7 +302,11 @@
       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
  
@@ -39,6 +39,8 @@
  
 	dmabuf = file->private_data;
  
+	BUG_ON(dmabuf->vmapping_counter);
+
 	dmabuf->ops->release(dmabuf);
 	kfree(dmabuf);
 	return 0;
  
@@ -481,12 +483,34 @@
  */
 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);
  
@@ -500,8 +524,17 @@
 	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);
@@ -119,8 +119,10 @@
 	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;
 };
...	...	@@ -302,7 +302,11 @@
302	302	void dma_buf_vunmap(struct dma_buf dmabuf, void vaddr)
303	303
304	304	The vmap call can fail if there is no vmap support in the exporter, or if it
305		- runs out of vmalloc space. Fallback to kmap should be implemented.
	305	+ runs out of vmalloc space. Fallback to kmap should be implemented. Note that
	306	+ the dma-buf layer keeps a reference count for all vmap access and calls down
	307	+ into the exporter's vmap function only when no vmapping exists, and only
	308	+ unmaps it once. Protection against concurrent vmap/vunmap calls is provided
	309	+ by taking the dma_buf->lock mutex.
306	310
307	311	3. Finish access
308	312
...	...	@@ -39,6 +39,8 @@
39	39
40	40	dmabuf = file->private_data;
41	41
	42	+ BUG_ON(dmabuf->vmapping_counter);
	43	+
42	44	dmabuf->ops->release(dmabuf);
43	45	kfree(dmabuf);
44	46	return 0;
45	47
...	...	@@ -481,12 +483,34 @@
481	483	*/
482	484	void dma_buf_vmap(struct dma_buf dmabuf)
483	485	{
	486	+ void *ptr;
	487	+
484	488	if (WARN_ON(!dmabuf))
485	489	return NULL;
486	490
487		- if (dmabuf->ops->vmap)
488		- return dmabuf->ops->vmap(dmabuf);
489		- return NULL;
	491	+ if (!dmabuf->ops->vmap)
	492	+ return NULL;
	493	+
	494	+ mutex_lock(&dmabuf->lock);
	495	+ if (dmabuf->vmapping_counter) {
	496	+ dmabuf->vmapping_counter++;
	497	+ BUG_ON(!dmabuf->vmap_ptr);
	498	+ ptr = dmabuf->vmap_ptr;
	499	+ goto out_unlock;
	500	+ }
	501	+
	502	+ BUG_ON(dmabuf->vmap_ptr);
	503	+
	504	+ ptr = dmabuf->ops->vmap(dmabuf);
	505	+ if (IS_ERR_OR_NULL(ptr))
	506	+ goto out_unlock;
	507	+
	508	+ dmabuf->vmap_ptr = ptr;
	509	+ dmabuf->vmapping_counter = 1;
	510	+
	511	+out_unlock:
	512	+ mutex_unlock(&dmabuf->lock);
	513	+ return ptr;
490	514	}
491	515	EXPORT_SYMBOL_GPL(dma_buf_vmap);
492	516
...	...	@@ -500,8 +524,17 @@
500	524	if (WARN_ON(!dmabuf))
501	525	return;
502	526
503		- if (dmabuf->ops->vunmap)
504		- dmabuf->ops->vunmap(dmabuf, vaddr);
	527	+ BUG_ON(!dmabuf->vmap_ptr);
	528	+ BUG_ON(dmabuf->vmapping_counter == 0);
	529	+ BUG_ON(dmabuf->vmap_ptr != vaddr);
	530	+
	531	+ mutex_lock(&dmabuf->lock);
	532	+ if (--dmabuf->vmapping_counter == 0) {
	533	+ if (dmabuf->ops->vunmap)
	534	+ dmabuf->ops->vunmap(dmabuf, vaddr);
	535	+ dmabuf->vmap_ptr = NULL;
	536	+ }
	537	+ mutex_unlock(&dmabuf->lock);
505	538	}
506	539	EXPORT_SYMBOL_GPL(dma_buf_vunmap);
...	...	@@ -119,8 +119,10 @@
119	119	struct file *file;
120	120	struct list_head attachments;
121	121	const struct dma_buf_ops *ops;
122		- /* mutex to serialize list manipulation and attach/detach */
	122	+ /* mutex to serialize list manipulation, attach/detach and vmap/unmap */
123	123	struct mutex lock;
	124	+ unsigned vmapping_counter;
	125	+ void *vmap_ptr;
124	126	void *priv;
125	127	};
126	128