All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07  7:30 ` Sumit Semwal
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Semwal @ 2015-05-07  7:30 UTC (permalink / raw)
  To: linux-kernel, linux-media, dri-devel, linaro-mm-sig,
	linux-arm-kernel, rmk+kernel, airlied, kgene, thierry.reding,
	pawel, m.szyprowski, mchehab, gregkh
  Cc: Tomasz Stanislawski, linaro-kernel, intel-gfx, maarten.lankhorst,
	linux-tegra

Add reference counting on a kernel module that exports dma-buf and
implements its operations. This prevents the module from being unloaded
while DMABUF file is in use.

The original patch [1] was submitted by Tomasz, but he's since shifted
jobs and a ping didn't elicit any response.

  [tomasz: Original author]
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Acked-by: Daniel Vetter <daniel@ffwll.ch>
  [sumits: updated & rebased as per review comments]
Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>

[1]: https://lkml.org/lkml/2012/8/8/163
---
 drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
 drivers/gpu/drm/armada/armada_gem.c            | 1 +
 drivers/gpu/drm/drm_prime.c                    | 1 +
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
 drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
 drivers/gpu/drm/tegra/gem.c                    | 1 +
 drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
 drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
 drivers/staging/android/ion/ion.c              | 1 +
 include/linux/dma-buf.h                        | 2 ++
 14 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c5a9138a6a8d..9583eac0238f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -29,6 +29,7 @@
 #include <linux/anon_inodes.h>
 #include <linux/export.h>
 #include <linux/debugfs.h>
+#include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/poll.h>
 #include <linux/reservation.h>
@@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
 	BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
 
 	dmabuf->ops->release(dmabuf);
+	module_put(dmabuf->ops->owner);
 
 	mutex_lock(&db_list.lock);
 	list_del(&dmabuf->list_node);
@@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 		return ERR_PTR(-EINVAL);
 	}
 
+	if (!try_module_get(exp_info->ops->owner))
+		return ERR_PTR(-ENOENT);
+
 	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
-	if (dmabuf == NULL)
+	if (!dmabuf) {
+		module_put(exp_info->ops->owner);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	dmabuf->priv = exp_info->priv;
 	dmabuf->ops = exp_info->ops;
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
index 580e10acaa3a..d2c5fc1269b7 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
 }
 
 static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf	= armada_gem_prime_map_dma_buf,
 	.unmap_dma_buf	= armada_gem_prime_unmap_dma_buf,
 	.release	= drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7fec191b45f7..ed4a6e35dd91 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -289,6 +289,7 @@ static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.attach = drm_gem_map_attach,
 	.detach = drm_gem_map_detach,
 	.map_dma_buf = drm_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index cd485c091b30..35fa029353e9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -169,6 +169,7 @@ static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static struct dma_buf_ops exynos_dmabuf_ops = {
+	.owner			= THIS_MODULE,
 	.attach			= exynos_gem_attach_dma_buf,
 	.detach			= exynos_gem_detach_dma_buf,
 	.map_dma_buf		= exynos_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 7998da27c500..b9355d8d4862 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -213,6 +213,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size
 }
 
 static const struct dma_buf_ops i915_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.map_dma_buf = i915_gem_map_dma_buf,
 	.unmap_dma_buf = i915_gem_unmap_dma_buf,
 	.release = drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index 344fd789170d..977ece9be62c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -156,6 +156,7 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer,
 }
 
 static struct dma_buf_ops omap_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = omap_gem_map_dma_buf,
 	.unmap_dma_buf = omap_gem_unmap_dma_buf,
 	.release = omap_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 1217272a51f2..7af37a445524 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -611,6 +611,7 @@ static void tegra_gem_prime_vunmap(struct dma_buf *buf, void *vaddr)
 }
 
 static const struct dma_buf_ops tegra_gem_prime_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = tegra_gem_prime_map_dma_buf,
 	.unmap_dma_buf = tegra_gem_prime_unmap_dma_buf,
 	.release = tegra_gem_prime_release,
diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
index e2243edd1ce3..b67bd66c4636 100644
--- a/drivers/gpu/drm/udl/udl_dmabuf.c
+++ b/drivers/gpu/drm/udl/udl_dmabuf.c
@@ -187,6 +187,7 @@ static int udl_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static struct dma_buf_ops udl_dmabuf_ops = {
+	.owner			= THIS_MODULE,
 	.attach			= udl_attach_dma_buf,
 	.detach			= udl_detach_dma_buf,
 	.map_dma_buf		= udl_map_dma_buf,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
index 31fe32d8d65a..81aa6eae9031 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
@@ -103,6 +103,7 @@ static int vmw_prime_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 const struct dma_buf_ops vmw_prime_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.attach = vmw_prime_map_attach,
 	.detach = vmw_prime_map_detach,
 	.map_dma_buf = vmw_prime_map_dma_buf,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 644dec73d220..22cb21480fb8 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -365,6 +365,7 @@ static int vb2_dc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_dc_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_dc_dmabuf_ops_attach,
 	.detach = vb2_dc_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dc_dmabuf_ops_map,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 45c708e463b9..f659ee409148 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -568,6 +568,7 @@ static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_dma_sg_dmabuf_ops_attach,
 	.detach = vb2_dma_sg_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dma_sg_dmabuf_ops_map,
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 657ab302a5cf..a76ce4d989c8 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -353,6 +353,7 @@ static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_vmalloc_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_vmalloc_dmabuf_ops_attach,
 	.detach = vb2_vmalloc_dmabuf_ops_detach,
 	.map_dma_buf = vb2_vmalloc_dmabuf_ops_map,
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index b0b96ab31954..5af1355c874c 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1088,6 +1088,7 @@ static void ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, size_t start,
 }
 
 static struct dma_buf_ops dma_buf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = ion_map_dma_buf,
 	.unmap_dma_buf = ion_unmap_dma_buf,
 	.mmap = ion_mmap,
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 2f0b431b73e0..22d367080943 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -39,6 +39,7 @@ struct dma_buf_attachment;
 
 /**
  * struct dma_buf_ops - operations possible on struct dma_buf
+ * @owner: the module that implements dma_buf operations
  * @attach: [optional] allows different devices to 'attach' themselves to the
  *	    given buffer. It might return -EBUSY to signal that backing storage
  *	    is already allocated and incompatible with the requirements
@@ -72,6 +73,7 @@ struct dma_buf_attachment;
  * @vunmap: [optional] unmaps a vmap from the buffer
  */
 struct dma_buf_ops {
+	struct module *owner;
 	int (*attach)(struct dma_buf *, struct device *,
 			struct dma_buf_attachment *);
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07  7:30 ` Sumit Semwal
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Semwal @ 2015-05-07  7:30 UTC (permalink / raw)
  To: linux-kernel, linux-media, dri-devel, linaro-mm-sig,
	linux-arm-kernel, rmk+kernel, airlied, kgene, thierry.reding,
	pawel, m.szyprowski, mchehab, gregkh
  Cc: linaro-kernel, robdclark, daniel, intel-gfx, linux-tegra,
	inki.dae, maarten.lankhorst, Sumit Semwal, Tomasz Stanislawski

Add reference counting on a kernel module that exports dma-buf and
implements its operations. This prevents the module from being unloaded
while DMABUF file is in use.

The original patch [1] was submitted by Tomasz, but he's since shifted
jobs and a ping didn't elicit any response.

  [tomasz: Original author]
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Acked-by: Daniel Vetter <daniel@ffwll.ch>
  [sumits: updated & rebased as per review comments]
Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>

[1]: https://lkml.org/lkml/2012/8/8/163
---
 drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
 drivers/gpu/drm/armada/armada_gem.c            | 1 +
 drivers/gpu/drm/drm_prime.c                    | 1 +
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
 drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
 drivers/gpu/drm/tegra/gem.c                    | 1 +
 drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
 drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
 drivers/staging/android/ion/ion.c              | 1 +
 include/linux/dma-buf.h                        | 2 ++
 14 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c5a9138a6a8d..9583eac0238f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -29,6 +29,7 @@
 #include <linux/anon_inodes.h>
 #include <linux/export.h>
 #include <linux/debugfs.h>
+#include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/poll.h>
 #include <linux/reservation.h>
@@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
 	BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
 
 	dmabuf->ops->release(dmabuf);
+	module_put(dmabuf->ops->owner);
 
 	mutex_lock(&db_list.lock);
 	list_del(&dmabuf->list_node);
@@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 		return ERR_PTR(-EINVAL);
 	}
 
+	if (!try_module_get(exp_info->ops->owner))
+		return ERR_PTR(-ENOENT);
+
 	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
-	if (dmabuf == NULL)
+	if (!dmabuf) {
+		module_put(exp_info->ops->owner);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	dmabuf->priv = exp_info->priv;
 	dmabuf->ops = exp_info->ops;
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
index 580e10acaa3a..d2c5fc1269b7 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
 }
 
 static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf	= armada_gem_prime_map_dma_buf,
 	.unmap_dma_buf	= armada_gem_prime_unmap_dma_buf,
 	.release	= drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7fec191b45f7..ed4a6e35dd91 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -289,6 +289,7 @@ static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.attach = drm_gem_map_attach,
 	.detach = drm_gem_map_detach,
 	.map_dma_buf = drm_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index cd485c091b30..35fa029353e9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -169,6 +169,7 @@ static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static struct dma_buf_ops exynos_dmabuf_ops = {
+	.owner			= THIS_MODULE,
 	.attach			= exynos_gem_attach_dma_buf,
 	.detach			= exynos_gem_detach_dma_buf,
 	.map_dma_buf		= exynos_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 7998da27c500..b9355d8d4862 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -213,6 +213,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size
 }
 
 static const struct dma_buf_ops i915_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.map_dma_buf = i915_gem_map_dma_buf,
 	.unmap_dma_buf = i915_gem_unmap_dma_buf,
 	.release = drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index 344fd789170d..977ece9be62c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -156,6 +156,7 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer,
 }
 
 static struct dma_buf_ops omap_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = omap_gem_map_dma_buf,
 	.unmap_dma_buf = omap_gem_unmap_dma_buf,
 	.release = omap_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 1217272a51f2..7af37a445524 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -611,6 +611,7 @@ static void tegra_gem_prime_vunmap(struct dma_buf *buf, void *vaddr)
 }
 
 static const struct dma_buf_ops tegra_gem_prime_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = tegra_gem_prime_map_dma_buf,
 	.unmap_dma_buf = tegra_gem_prime_unmap_dma_buf,
 	.release = tegra_gem_prime_release,
diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
index e2243edd1ce3..b67bd66c4636 100644
--- a/drivers/gpu/drm/udl/udl_dmabuf.c
+++ b/drivers/gpu/drm/udl/udl_dmabuf.c
@@ -187,6 +187,7 @@ static int udl_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static struct dma_buf_ops udl_dmabuf_ops = {
+	.owner			= THIS_MODULE,
 	.attach			= udl_attach_dma_buf,
 	.detach			= udl_detach_dma_buf,
 	.map_dma_buf		= udl_map_dma_buf,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
index 31fe32d8d65a..81aa6eae9031 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
@@ -103,6 +103,7 @@ static int vmw_prime_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 const struct dma_buf_ops vmw_prime_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.attach = vmw_prime_map_attach,
 	.detach = vmw_prime_map_detach,
 	.map_dma_buf = vmw_prime_map_dma_buf,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 644dec73d220..22cb21480fb8 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -365,6 +365,7 @@ static int vb2_dc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_dc_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_dc_dmabuf_ops_attach,
 	.detach = vb2_dc_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dc_dmabuf_ops_map,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 45c708e463b9..f659ee409148 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -568,6 +568,7 @@ static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_dma_sg_dmabuf_ops_attach,
 	.detach = vb2_dma_sg_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dma_sg_dmabuf_ops_map,
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 657ab302a5cf..a76ce4d989c8 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -353,6 +353,7 @@ static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_vmalloc_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_vmalloc_dmabuf_ops_attach,
 	.detach = vb2_vmalloc_dmabuf_ops_detach,
 	.map_dma_buf = vb2_vmalloc_dmabuf_ops_map,
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index b0b96ab31954..5af1355c874c 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1088,6 +1088,7 @@ static void ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, size_t start,
 }
 
 static struct dma_buf_ops dma_buf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = ion_map_dma_buf,
 	.unmap_dma_buf = ion_unmap_dma_buf,
 	.mmap = ion_mmap,
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 2f0b431b73e0..22d367080943 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -39,6 +39,7 @@ struct dma_buf_attachment;
 
 /**
  * struct dma_buf_ops - operations possible on struct dma_buf
+ * @owner: the module that implements dma_buf operations
  * @attach: [optional] allows different devices to 'attach' themselves to the
  *	    given buffer. It might return -EBUSY to signal that backing storage
  *	    is already allocated and incompatible with the requirements
@@ -72,6 +73,7 @@ struct dma_buf_attachment;
  * @vunmap: [optional] unmaps a vmap from the buffer
  */
 struct dma_buf_ops {
+	struct module *owner;
 	int (*attach)(struct dma_buf *, struct device *,
 			struct dma_buf_attachment *);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07  7:30 ` Sumit Semwal
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Semwal @ 2015-05-07  7:30 UTC (permalink / raw)
  To: linux-arm-kernel

Add reference counting on a kernel module that exports dma-buf and
implements its operations. This prevents the module from being unloaded
while DMABUF file is in use.

The original patch [1] was submitted by Tomasz, but he's since shifted
jobs and a ping didn't elicit any response.

  [tomasz: Original author]
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Acked-by: Daniel Vetter <daniel@ffwll.ch>
  [sumits: updated & rebased as per review comments]
Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>

[1]: https://lkml.org/lkml/2012/8/8/163
---
 drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
 drivers/gpu/drm/armada/armada_gem.c            | 1 +
 drivers/gpu/drm/drm_prime.c                    | 1 +
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
 drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
 drivers/gpu/drm/tegra/gem.c                    | 1 +
 drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
 drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
 drivers/staging/android/ion/ion.c              | 1 +
 include/linux/dma-buf.h                        | 2 ++
 14 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c5a9138a6a8d..9583eac0238f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -29,6 +29,7 @@
 #include <linux/anon_inodes.h>
 #include <linux/export.h>
 #include <linux/debugfs.h>
+#include <linux/module.h>
 #include <linux/seq_file.h>
 #include <linux/poll.h>
 #include <linux/reservation.h>
@@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
 	BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
 
 	dmabuf->ops->release(dmabuf);
+	module_put(dmabuf->ops->owner);
 
 	mutex_lock(&db_list.lock);
 	list_del(&dmabuf->list_node);
@@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 		return ERR_PTR(-EINVAL);
 	}
 
+	if (!try_module_get(exp_info->ops->owner))
+		return ERR_PTR(-ENOENT);
+
 	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
-	if (dmabuf == NULL)
+	if (!dmabuf) {
+		module_put(exp_info->ops->owner);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	dmabuf->priv = exp_info->priv;
 	dmabuf->ops = exp_info->ops;
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
index 580e10acaa3a..d2c5fc1269b7 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
 }
 
 static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf	= armada_gem_prime_map_dma_buf,
 	.unmap_dma_buf	= armada_gem_prime_unmap_dma_buf,
 	.release	= drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7fec191b45f7..ed4a6e35dd91 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -289,6 +289,7 @@ static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.attach = drm_gem_map_attach,
 	.detach = drm_gem_map_detach,
 	.map_dma_buf = drm_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index cd485c091b30..35fa029353e9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -169,6 +169,7 @@ static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static struct dma_buf_ops exynos_dmabuf_ops = {
+	.owner			= THIS_MODULE,
 	.attach			= exynos_gem_attach_dma_buf,
 	.detach			= exynos_gem_detach_dma_buf,
 	.map_dma_buf		= exynos_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 7998da27c500..b9355d8d4862 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -213,6 +213,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start, size
 }
 
 static const struct dma_buf_ops i915_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.map_dma_buf = i915_gem_map_dma_buf,
 	.unmap_dma_buf = i915_gem_unmap_dma_buf,
 	.release = drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index 344fd789170d..977ece9be62c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -156,6 +156,7 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer,
 }
 
 static struct dma_buf_ops omap_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = omap_gem_map_dma_buf,
 	.unmap_dma_buf = omap_gem_unmap_dma_buf,
 	.release = omap_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 1217272a51f2..7af37a445524 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -611,6 +611,7 @@ static void tegra_gem_prime_vunmap(struct dma_buf *buf, void *vaddr)
 }
 
 static const struct dma_buf_ops tegra_gem_prime_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = tegra_gem_prime_map_dma_buf,
 	.unmap_dma_buf = tegra_gem_prime_unmap_dma_buf,
 	.release = tegra_gem_prime_release,
diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
index e2243edd1ce3..b67bd66c4636 100644
--- a/drivers/gpu/drm/udl/udl_dmabuf.c
+++ b/drivers/gpu/drm/udl/udl_dmabuf.c
@@ -187,6 +187,7 @@ static int udl_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static struct dma_buf_ops udl_dmabuf_ops = {
+	.owner			= THIS_MODULE,
 	.attach			= udl_attach_dma_buf,
 	.detach			= udl_detach_dma_buf,
 	.map_dma_buf		= udl_map_dma_buf,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
index 31fe32d8d65a..81aa6eae9031 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_prime.c
@@ -103,6 +103,7 @@ static int vmw_prime_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 const struct dma_buf_ops vmw_prime_dmabuf_ops =  {
+	.owner = THIS_MODULE,
 	.attach = vmw_prime_map_attach,
 	.detach = vmw_prime_map_detach,
 	.map_dma_buf = vmw_prime_map_dma_buf,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 644dec73d220..22cb21480fb8 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -365,6 +365,7 @@ static int vb2_dc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_dc_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_dc_dmabuf_ops_attach,
 	.detach = vb2_dc_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dc_dmabuf_ops_map,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 45c708e463b9..f659ee409148 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -568,6 +568,7 @@ static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_dma_sg_dmabuf_ops_attach,
 	.detach = vb2_dma_sg_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dma_sg_dmabuf_ops_map,
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 657ab302a5cf..a76ce4d989c8 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -353,6 +353,7 @@ static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 }
 
 static struct dma_buf_ops vb2_vmalloc_dmabuf_ops = {
+	.owner = THIS_MODULE,
 	.attach = vb2_vmalloc_dmabuf_ops_attach,
 	.detach = vb2_vmalloc_dmabuf_ops_detach,
 	.map_dma_buf = vb2_vmalloc_dmabuf_ops_map,
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index b0b96ab31954..5af1355c874c 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1088,6 +1088,7 @@ static void ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, size_t start,
 }
 
 static struct dma_buf_ops dma_buf_ops = {
+	.owner = THIS_MODULE,
 	.map_dma_buf = ion_map_dma_buf,
 	.unmap_dma_buf = ion_unmap_dma_buf,
 	.mmap = ion_mmap,
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 2f0b431b73e0..22d367080943 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -39,6 +39,7 @@ struct dma_buf_attachment;
 
 /**
  * struct dma_buf_ops - operations possible on struct dma_buf
+ * @owner: the module that implements dma_buf operations
  * @attach: [optional] allows different devices to 'attach' themselves to the
  *	    given buffer. It might return -EBUSY to signal that backing storage
  *	    is already allocated and incompatible with the requirements
@@ -72,6 +73,7 @@ struct dma_buf_attachment;
  * @vunmap: [optional] unmaps a vmap from the buffer
  */
 struct dma_buf_ops {
+	struct module *owner;
 	int (*attach)(struct dma_buf *, struct device *,
 			struct dma_buf_attachment *);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH] dma-buf: add ref counting for module as exporter
  2015-05-07  7:30 ` Sumit Semwal
  (?)
@ 2015-05-07  8:10   ` Greg KH
  -1 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2015-05-07  8:10 UTC (permalink / raw)
  To: Sumit Semwal
  Cc: Tomasz Stanislawski, inki.dae, linaro-kernel, pawel, mchehab,
	airlied, intel-gfx, linux-kernel, dri-devel, linaro-mm-sig,
	maarten.lankhorst, kgene, thierry.reding, linux-tegra,
	rmk+kernel, m.szyprowski, linux-arm-kernel, linux-media

On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
> Add reference counting on a kernel module that exports dma-buf and
> implements its operations. This prevents the module from being unloaded
> while DMABUF file is in use.
> 
> The original patch [1] was submitted by Tomasz, but he's since shifted
> jobs and a ping didn't elicit any response.
> 
>   [tomasz: Original author]
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Acked-by: Daniel Vetter <daniel@ffwll.ch>
>   [sumits: updated & rebased as per review comments]
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> 
> [1]: https://lkml.org/lkml/2012/8/8/163
> ---
>  drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
>  drivers/gpu/drm/armada/armada_gem.c            | 1 +
>  drivers/gpu/drm/drm_prime.c                    | 1 +
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
>  drivers/gpu/drm/tegra/gem.c                    | 1 +
>  drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
>  drivers/staging/android/ion/ion.c              | 1 +
>  include/linux/dma-buf.h                        | 2 ++
>  14 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index c5a9138a6a8d..9583eac0238f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -29,6 +29,7 @@
>  #include <linux/anon_inodes.h>
>  #include <linux/export.h>
>  #include <linux/debugfs.h>
> +#include <linux/module.h>
>  #include <linux/seq_file.h>
>  #include <linux/poll.h>
>  #include <linux/reservation.h>
> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>  	BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
>  
>  	dmabuf->ops->release(dmabuf);
> +	module_put(dmabuf->ops->owner);
>  
>  	mutex_lock(&db_list.lock);
>  	list_del(&dmabuf->list_node);
> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> +	if (!try_module_get(exp_info->ops->owner))
> +		return ERR_PTR(-ENOENT);
> +
>  	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
> -	if (dmabuf == NULL)
> +	if (!dmabuf) {
> +		module_put(exp_info->ops->owner);
>  		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	dmabuf->priv = exp_info->priv;
>  	dmabuf->ops = exp_info->ops;
> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
> index 580e10acaa3a..d2c5fc1269b7 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
>  }
>  
>  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
> +	.owner = THIS_MODULE,
>  	.map_dma_buf	= armada_gem_prime_map_dma_buf,
>  	.unmap_dma_buf	= armada_gem_prime_unmap_dma_buf,
>  	.release	= drm_gem_dmabuf_release,

The "easier" way to do this is change the registration function to add
the module owner automatically, which keeps you from having to modify
all of the individual drivers.  Look at how pci and usb do this for
their driver registration functions.  That should result in a much
smaller patch, that always works properly for everyone (there's no way
for driver to get it wrong.)

thanks,

greg k-h
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07  8:10   ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2015-05-07  8:10 UTC (permalink / raw)
  To: Sumit Semwal
  Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig,
	linux-arm-kernel, rmk+kernel, airlied, kgene, thierry.reding,
	pawel, m.szyprowski, mchehab, linaro-kernel, robdclark, daniel,
	intel-gfx, linux-tegra, inki.dae, maarten.lankhorst,
	Tomasz Stanislawski

On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
> Add reference counting on a kernel module that exports dma-buf and
> implements its operations. This prevents the module from being unloaded
> while DMABUF file is in use.
> 
> The original patch [1] was submitted by Tomasz, but he's since shifted
> jobs and a ping didn't elicit any response.
> 
>   [tomasz: Original author]
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Acked-by: Daniel Vetter <daniel@ffwll.ch>
>   [sumits: updated & rebased as per review comments]
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> 
> [1]: https://lkml.org/lkml/2012/8/8/163
> ---
>  drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
>  drivers/gpu/drm/armada/armada_gem.c            | 1 +
>  drivers/gpu/drm/drm_prime.c                    | 1 +
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
>  drivers/gpu/drm/tegra/gem.c                    | 1 +
>  drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
>  drivers/staging/android/ion/ion.c              | 1 +
>  include/linux/dma-buf.h                        | 2 ++
>  14 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index c5a9138a6a8d..9583eac0238f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -29,6 +29,7 @@
>  #include <linux/anon_inodes.h>
>  #include <linux/export.h>
>  #include <linux/debugfs.h>
> +#include <linux/module.h>
>  #include <linux/seq_file.h>
>  #include <linux/poll.h>
>  #include <linux/reservation.h>
> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>  	BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
>  
>  	dmabuf->ops->release(dmabuf);
> +	module_put(dmabuf->ops->owner);
>  
>  	mutex_lock(&db_list.lock);
>  	list_del(&dmabuf->list_node);
> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> +	if (!try_module_get(exp_info->ops->owner))
> +		return ERR_PTR(-ENOENT);
> +
>  	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
> -	if (dmabuf == NULL)
> +	if (!dmabuf) {
> +		module_put(exp_info->ops->owner);
>  		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	dmabuf->priv = exp_info->priv;
>  	dmabuf->ops = exp_info->ops;
> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
> index 580e10acaa3a..d2c5fc1269b7 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
>  }
>  
>  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
> +	.owner = THIS_MODULE,
>  	.map_dma_buf	= armada_gem_prime_map_dma_buf,
>  	.unmap_dma_buf	= armada_gem_prime_unmap_dma_buf,
>  	.release	= drm_gem_dmabuf_release,

The "easier" way to do this is change the registration function to add
the module owner automatically, which keeps you from having to modify
all of the individual drivers.  Look at how pci and usb do this for
their driver registration functions.  That should result in a much
smaller patch, that always works properly for everyone (there's no way
for driver to get it wrong.)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07  8:10   ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2015-05-07  8:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
> Add reference counting on a kernel module that exports dma-buf and
> implements its operations. This prevents the module from being unloaded
> while DMABUF file is in use.
> 
> The original patch [1] was submitted by Tomasz, but he's since shifted
> jobs and a ping didn't elicit any response.
> 
>   [tomasz: Original author]
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Acked-by: Daniel Vetter <daniel@ffwll.ch>
>   [sumits: updated & rebased as per review comments]
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> 
> [1]: https://lkml.org/lkml/2012/8/8/163
> ---
>  drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
>  drivers/gpu/drm/armada/armada_gem.c            | 1 +
>  drivers/gpu/drm/drm_prime.c                    | 1 +
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
>  drivers/gpu/drm/tegra/gem.c                    | 1 +
>  drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
>  drivers/staging/android/ion/ion.c              | 1 +
>  include/linux/dma-buf.h                        | 2 ++
>  14 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index c5a9138a6a8d..9583eac0238f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -29,6 +29,7 @@
>  #include <linux/anon_inodes.h>
>  #include <linux/export.h>
>  #include <linux/debugfs.h>
> +#include <linux/module.h>
>  #include <linux/seq_file.h>
>  #include <linux/poll.h>
>  #include <linux/reservation.h>
> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>  	BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
>  
>  	dmabuf->ops->release(dmabuf);
> +	module_put(dmabuf->ops->owner);
>  
>  	mutex_lock(&db_list.lock);
>  	list_del(&dmabuf->list_node);
> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> +	if (!try_module_get(exp_info->ops->owner))
> +		return ERR_PTR(-ENOENT);
> +
>  	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
> -	if (dmabuf == NULL)
> +	if (!dmabuf) {
> +		module_put(exp_info->ops->owner);
>  		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	dmabuf->priv = exp_info->priv;
>  	dmabuf->ops = exp_info->ops;
> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
> index 580e10acaa3a..d2c5fc1269b7 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
>  }
>  
>  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
> +	.owner = THIS_MODULE,
>  	.map_dma_buf	= armada_gem_prime_map_dma_buf,
>  	.unmap_dma_buf	= armada_gem_prime_unmap_dma_buf,
>  	.release	= drm_gem_dmabuf_release,

The "easier" way to do this is change the registration function to add
the module owner automatically, which keeps you from having to modify
all of the individual drivers.  Look at how pci and usb do this for
their driver registration functions.  That should result in a much
smaller patch, that always works properly for everyone (there's no way
for driver to get it wrong.)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] dma-buf: add ref counting for module as exporter
  2015-05-07  8:10   ` Greg KH
  (?)
  (?)
@ 2015-05-07 10:46     ` Sumit Semwal
  -1 siblings, 0 replies; 10+ messages in thread
From: Sumit Semwal @ 2015-05-07 10:46 UTC (permalink / raw)
  To: Greg KH
  Cc: Tomasz Stanislawski, Linaro Kernel Mailman List, Pawel Osciak,
	Mauro Carvalho Chehab, intel-gfx, LKML, DRI mailing list,
	Linaro MM SIG Mailman List, Maarten Lankhorst, kgene,
	linux-tegra, rmk+kernel, Marek Szyprowski, linux-arm-kernel,
	linux-media

On 7 May 2015 at 13:40, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
>> Add reference counting on a kernel module that exports dma-buf and
>> implements its operations. This prevents the module from being unloaded
>> while DMABUF file is in use.
>>
>> The original patch [1] was submitted by Tomasz, but he's since shifted
>> jobs and a ping didn't elicit any response.
>>
>>   [tomasz: Original author]
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>> Acked-by: Daniel Vetter <daniel@ffwll.ch>
>>   [sumits: updated & rebased as per review comments]
>> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
>>
>> [1]: https://lkml.org/lkml/2012/8/8/163
>> ---
>>  drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
>>  drivers/gpu/drm/armada/armada_gem.c            | 1 +
>>  drivers/gpu/drm/drm_prime.c                    | 1 +
>>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
>>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
>>  drivers/gpu/drm/tegra/gem.c                    | 1 +
>>  drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
>>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
>>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
>>  drivers/staging/android/ion/ion.c              | 1 +
>>  include/linux/dma-buf.h                        | 2 ++
>>  14 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index c5a9138a6a8d..9583eac0238f 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/anon_inodes.h>
>>  #include <linux/export.h>
>>  #include <linux/debugfs.h>
>> +#include <linux/module.h>
>>  #include <linux/seq_file.h>
>>  #include <linux/poll.h>
>>  #include <linux/reservation.h>
>> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>>       BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
>>
>>       dmabuf->ops->release(dmabuf);
>> +     module_put(dmabuf->ops->owner);
>>
>>       mutex_lock(&db_list.lock);
>>       list_del(&dmabuf->list_node);
>> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>>               return ERR_PTR(-EINVAL);
>>       }
>>
>> +     if (!try_module_get(exp_info->ops->owner))
>> +             return ERR_PTR(-ENOENT);
>> +
>>       dmabuf = kzalloc(alloc_size, GFP_KERNEL);
>> -     if (dmabuf == NULL)
>> +     if (!dmabuf) {
>> +             module_put(exp_info->ops->owner);
>>               return ERR_PTR(-ENOMEM);
>> +     }
>>
>>       dmabuf->priv = exp_info->priv;
>>       dmabuf->ops = exp_info->ops;
>> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
>> index 580e10acaa3a..d2c5fc1269b7 100644
>> --- a/drivers/gpu/drm/armada/armada_gem.c
>> +++ b/drivers/gpu/drm/armada/armada_gem.c
>> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
>>  }
>>
>>  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
>> +     .owner = THIS_MODULE,
>>       .map_dma_buf    = armada_gem_prime_map_dma_buf,
>>       .unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
>>       .release        = drm_gem_dmabuf_release,
>
> The "easier" way to do this is change the registration function to add
> the module owner automatically, which keeps you from having to modify
> all of the individual drivers.  Look at how pci and usb do this for
> their driver registration functions.  That should result in a much
> smaller patch, that always works properly for everyone (there's no way
> for driver to get it wrong.)
>
Thanks Greg; but of course, you're right! We already have a
DEFINE_DMA_BUF_EXPORT_INFO macro, so this is far easier incorporated
into that.

I will spin up the (much simpler) patch and repost!

> thanks,
>
> greg k-h


Best regards,
Sumit.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07 10:46     ` Sumit Semwal
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Semwal @ 2015-05-07 10:46 UTC (permalink / raw)
  To: Greg KH
  Cc: LKML, linux-media, DRI mailing list, Linaro MM SIG Mailman List,
	linux-arm-kernel, rmk+kernel, Dave Airlie, kgene, Thierry Reding,
	Pawel Osciak, Marek Szyprowski, Mauro Carvalho Chehab,
	Linaro Kernel Mailman List, Rob Clark, Daniel Vetter, intel-gfx,
	linux-tegra, inki.dae, Maarten Lankhorst, Tomasz Stanislawski

On 7 May 2015 at 13:40, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
>> Add reference counting on a kernel module that exports dma-buf and
>> implements its operations. This prevents the module from being unloaded
>> while DMABUF file is in use.
>>
>> The original patch [1] was submitted by Tomasz, but he's since shifted
>> jobs and a ping didn't elicit any response.
>>
>>   [tomasz: Original author]
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>> Acked-by: Daniel Vetter <daniel@ffwll.ch>
>>   [sumits: updated & rebased as per review comments]
>> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
>>
>> [1]: https://lkml.org/lkml/2012/8/8/163
>> ---
>>  drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
>>  drivers/gpu/drm/armada/armada_gem.c            | 1 +
>>  drivers/gpu/drm/drm_prime.c                    | 1 +
>>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
>>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
>>  drivers/gpu/drm/tegra/gem.c                    | 1 +
>>  drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
>>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
>>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
>>  drivers/staging/android/ion/ion.c              | 1 +
>>  include/linux/dma-buf.h                        | 2 ++
>>  14 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index c5a9138a6a8d..9583eac0238f 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/anon_inodes.h>
>>  #include <linux/export.h>
>>  #include <linux/debugfs.h>
>> +#include <linux/module.h>
>>  #include <linux/seq_file.h>
>>  #include <linux/poll.h>
>>  #include <linux/reservation.h>
>> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>>       BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
>>
>>       dmabuf->ops->release(dmabuf);
>> +     module_put(dmabuf->ops->owner);
>>
>>       mutex_lock(&db_list.lock);
>>       list_del(&dmabuf->list_node);
>> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>>               return ERR_PTR(-EINVAL);
>>       }
>>
>> +     if (!try_module_get(exp_info->ops->owner))
>> +             return ERR_PTR(-ENOENT);
>> +
>>       dmabuf = kzalloc(alloc_size, GFP_KERNEL);
>> -     if (dmabuf == NULL)
>> +     if (!dmabuf) {
>> +             module_put(exp_info->ops->owner);
>>               return ERR_PTR(-ENOMEM);
>> +     }
>>
>>       dmabuf->priv = exp_info->priv;
>>       dmabuf->ops = exp_info->ops;
>> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
>> index 580e10acaa3a..d2c5fc1269b7 100644
>> --- a/drivers/gpu/drm/armada/armada_gem.c
>> +++ b/drivers/gpu/drm/armada/armada_gem.c
>> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
>>  }
>>
>>  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
>> +     .owner = THIS_MODULE,
>>       .map_dma_buf    = armada_gem_prime_map_dma_buf,
>>       .unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
>>       .release        = drm_gem_dmabuf_release,
>
> The "easier" way to do this is change the registration function to add
> the module owner automatically, which keeps you from having to modify
> all of the individual drivers.  Look at how pci and usb do this for
> their driver registration functions.  That should result in a much
> smaller patch, that always works properly for everyone (there's no way
> for driver to get it wrong.)
>
Thanks Greg; but of course, you're right! We already have a
DEFINE_DMA_BUF_EXPORT_INFO macro, so this is far easier incorporated
into that.

I will spin up the (much simpler) patch and repost!

> thanks,
>
> greg k-h


Best regards,
Sumit.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07 10:46     ` Sumit Semwal
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Semwal @ 2015-05-07 10:46 UTC (permalink / raw)
  To: Greg KH
  Cc: LKML, linux-media, DRI mailing list, Linaro MM SIG Mailman List,
	linux-arm-kernel, rmk+kernel, Dave Airlie, kgene, Thierry Reding,
	Pawel Osciak, Marek Szyprowski, Mauro Carvalho Chehab,
	Linaro Kernel Mailman List, Rob Clark, Daniel Vetter, intel-gfx,
	linux-tegra, inki.dae, Maarten Lankhorst, Tomasz Stanislawski

On 7 May 2015 at 13:40, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
>> Add reference counting on a kernel module that exports dma-buf and
>> implements its operations. This prevents the module from being unloaded
>> while DMABUF file is in use.
>>
>> The original patch [1] was submitted by Tomasz, but he's since shifted
>> jobs and a ping didn't elicit any response.
>>
>>   [tomasz: Original author]
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>> Acked-by: Daniel Vetter <daniel@ffwll.ch>
>>   [sumits: updated & rebased as per review comments]
>> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
>>
>> [1]: https://lkml.org/lkml/2012/8/8/163
>> ---
>>  drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
>>  drivers/gpu/drm/armada/armada_gem.c            | 1 +
>>  drivers/gpu/drm/drm_prime.c                    | 1 +
>>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
>>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
>>  drivers/gpu/drm/tegra/gem.c                    | 1 +
>>  drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
>>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
>>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
>>  drivers/staging/android/ion/ion.c              | 1 +
>>  include/linux/dma-buf.h                        | 2 ++
>>  14 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index c5a9138a6a8d..9583eac0238f 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/anon_inodes.h>
>>  #include <linux/export.h>
>>  #include <linux/debugfs.h>
>> +#include <linux/module.h>
>>  #include <linux/seq_file.h>
>>  #include <linux/poll.h>
>>  #include <linux/reservation.h>
>> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>>       BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
>>
>>       dmabuf->ops->release(dmabuf);
>> +     module_put(dmabuf->ops->owner);
>>
>>       mutex_lock(&db_list.lock);
>>       list_del(&dmabuf->list_node);
>> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>>               return ERR_PTR(-EINVAL);
>>       }
>>
>> +     if (!try_module_get(exp_info->ops->owner))
>> +             return ERR_PTR(-ENOENT);
>> +
>>       dmabuf = kzalloc(alloc_size, GFP_KERNEL);
>> -     if (dmabuf == NULL)
>> +     if (!dmabuf) {
>> +             module_put(exp_info->ops->owner);
>>               return ERR_PTR(-ENOMEM);
>> +     }
>>
>>       dmabuf->priv = exp_info->priv;
>>       dmabuf->ops = exp_info->ops;
>> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
>> index 580e10acaa3a..d2c5fc1269b7 100644
>> --- a/drivers/gpu/drm/armada/armada_gem.c
>> +++ b/drivers/gpu/drm/armada/armada_gem.c
>> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
>>  }
>>
>>  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
>> +     .owner = THIS_MODULE,
>>       .map_dma_buf    = armada_gem_prime_map_dma_buf,
>>       .unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
>>       .release        = drm_gem_dmabuf_release,
>
> The "easier" way to do this is change the registration function to add
> the module owner automatically, which keeps you from having to modify
> all of the individual drivers.  Look at how pci and usb do this for
> their driver registration functions.  That should result in a much
> smaller patch, that always works properly for everyone (there's no way
> for driver to get it wrong.)
>
Thanks Greg; but of course, you're right! We already have a
DEFINE_DMA_BUF_EXPORT_INFO macro, so this is far easier incorporated
into that.

I will spin up the (much simpler) patch and repost!

> thanks,
>
> greg k-h


Best regards,
Sumit.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH] dma-buf: add ref counting for module as exporter
@ 2015-05-07 10:46     ` Sumit Semwal
  0 siblings, 0 replies; 10+ messages in thread
From: Sumit Semwal @ 2015-05-07 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 7 May 2015 at 13:40, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
>> Add reference counting on a kernel module that exports dma-buf and
>> implements its operations. This prevents the module from being unloaded
>> while DMABUF file is in use.
>>
>> The original patch [1] was submitted by Tomasz, but he's since shifted
>> jobs and a ping didn't elicit any response.
>>
>>   [tomasz: Original author]
>> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>> Acked-by: Daniel Vetter <daniel@ffwll.ch>
>>   [sumits: updated & rebased as per review comments]
>> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
>>
>> [1]: https://lkml.org/lkml/2012/8/8/163
>> ---
>>  drivers/dma-buf/dma-buf.c                      | 9 ++++++++-
>>  drivers/gpu/drm/armada/armada_gem.c            | 1 +
>>  drivers/gpu/drm/drm_prime.c                    | 1 +
>>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c     | 1 +
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c         | 1 +
>>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c      | 1 +
>>  drivers/gpu/drm/tegra/gem.c                    | 1 +
>>  drivers/gpu/drm/udl/udl_dmabuf.c               | 1 +
>>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c          | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 1 +
>>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 1 +
>>  drivers/staging/android/ion/ion.c              | 1 +
>>  include/linux/dma-buf.h                        | 2 ++
>>  14 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index c5a9138a6a8d..9583eac0238f 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/anon_inodes.h>
>>  #include <linux/export.h>
>>  #include <linux/debugfs.h>
>> +#include <linux/module.h>
>>  #include <linux/seq_file.h>
>>  #include <linux/poll.h>
>>  #include <linux/reservation.h>
>> @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>>       BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
>>
>>       dmabuf->ops->release(dmabuf);
>> +     module_put(dmabuf->ops->owner);
>>
>>       mutex_lock(&db_list.lock);
>>       list_del(&dmabuf->list_node);
>> @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>>               return ERR_PTR(-EINVAL);
>>       }
>>
>> +     if (!try_module_get(exp_info->ops->owner))
>> +             return ERR_PTR(-ENOENT);
>> +
>>       dmabuf = kzalloc(alloc_size, GFP_KERNEL);
>> -     if (dmabuf == NULL)
>> +     if (!dmabuf) {
>> +             module_put(exp_info->ops->owner);
>>               return ERR_PTR(-ENOMEM);
>> +     }
>>
>>       dmabuf->priv = exp_info->priv;
>>       dmabuf->ops = exp_info->ops;
>> diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
>> index 580e10acaa3a..d2c5fc1269b7 100644
>> --- a/drivers/gpu/drm/armada/armada_gem.c
>> +++ b/drivers/gpu/drm/armada/armada_gem.c
>> @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
>>  }
>>
>>  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
>> +     .owner = THIS_MODULE,
>>       .map_dma_buf    = armada_gem_prime_map_dma_buf,
>>       .unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
>>       .release        = drm_gem_dmabuf_release,
>
> The "easier" way to do this is change the registration function to add
> the module owner automatically, which keeps you from having to modify
> all of the individual drivers.  Look at how pci and usb do this for
> their driver registration functions.  That should result in a much
> smaller patch, that always works properly for everyone (there's no way
> for driver to get it wrong.)
>
Thanks Greg; but of course, you're right! We already have a
DEFINE_DMA_BUF_EXPORT_INFO macro, so this is far easier incorporated
into that.

I will spin up the (much simpler) patch and repost!

> thanks,
>
> greg k-h


Best regards,
Sumit.

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-05-07 10:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-07  7:30 [PATCH] dma-buf: add ref counting for module as exporter Sumit Semwal
2015-05-07  7:30 ` Sumit Semwal
2015-05-07  7:30 ` Sumit Semwal
2015-05-07  8:10 ` Greg KH
2015-05-07  8:10   ` Greg KH
2015-05-07  8:10   ` Greg KH
2015-05-07 10:46   ` Sumit Semwal
2015-05-07 10:46     ` Sumit Semwal
2015-05-07 10:46     ` Sumit Semwal
2015-05-07 10:46     ` Sumit Semwal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.