[PATCH v2] dma-buf: add ref counting for module as exporter

2015-05-07 Thread Greg KH
On Thu, May 07, 2015 at 06:58:44PM +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 Stanislawski, but this
> is a simpler way to do it.
> 
> v2: move owner to struct dma_buf, and use DEFINE_DMA_BUF_EXPORT_INFO
> macro to simplify the change.
> 
> Signed-off-by: Sumit Semwal 
> 
> [1]: https://lkml.org/lkml/2012/8/8/163
> ---
>  drivers/dma-buf/dma-buf.c | 10 +-
>  include/linux/dma-buf.h   | 10 --
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index c5a9138a6a8d..0eff4bf56ef6 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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->owner);

The module can now go away.

>  
>   mutex_lock(_list.lock);
>   list_del(>list_node);

But you reference it here :(

Please drop the reference at the last possible moment.

thanks,

greg k-h


[PATCH v2] dma-buf: add ref counting for module as exporter

2015-05-07 Thread Sumit Semwal
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 Stanislawski, but this
is a simpler way to do it.

v2: move owner to struct dma_buf, and use DEFINE_DMA_BUF_EXPORT_INFO
macro to simplify the change.

Signed-off-by: Sumit Semwal 

[1]: https://lkml.org/lkml/2012/8/8/163
---
 drivers/dma-buf/dma-buf.c | 10 +-
 include/linux/dma-buf.h   | 10 --
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c5a9138a6a8d..0eff4bf56ef6 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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->owner);

mutex_lock(_list.lock);
list_del(>list_node);
@@ -302,14 +304,20 @@ struct dma_buf *dma_buf_export(const struct 
dma_buf_export_info *exp_info)
return ERR_PTR(-EINVAL);
}

+   if (!try_module_get(exp_info->owner))
+   return ERR_PTR(-ENOENT);
+
dmabuf = kzalloc(alloc_size, GFP_KERNEL);
-   if (dmabuf == NULL)
+   if (!dmabuf) {
+   module_put(exp_info->owner);
return ERR_PTR(-ENOMEM);
+   }

dmabuf->priv = exp_info->priv;
dmabuf->ops = exp_info->ops;
dmabuf->size = exp_info->size;
dmabuf->exp_name = exp_info->exp_name;
+   dmabuf->owner = exp_info->owner;
init_waitqueue_head(>poll);
dmabuf->cb_excl.poll = dmabuf->cb_shared.poll = >poll;
dmabuf->cb_excl.active = dmabuf->cb_shared.active = 0;
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 2f0b431b73e0..f98bd7068d55 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -115,6 +115,8 @@ struct dma_buf_ops {
  * @attachments: list of dma_buf_attachment that denotes all devices attached.
  * @ops: dma_buf_ops associated with this buffer object.
  * @exp_name: name of the exporter; useful for debugging.
+ * @owner: pointer to exporter module; used for refcounting when exporter is a
+ * kernel module.
  * @list_node: node for dma_buf accounting and debugging.
  * @priv: exporter specific private data for this buffer object.
  * @resv: reservation object linked to this dma-buf
@@ -129,6 +131,7 @@ struct dma_buf {
unsigned vmapping_counter;
void *vmap_ptr;
const char *exp_name;
+   struct module *owner;
struct list_head list_node;
void *priv;
struct reservation_object *resv;
@@ -164,7 +167,8 @@ struct dma_buf_attachment {

 /**
  * struct dma_buf_export_info - holds information needed to export a dma_buf
- * @exp_name:  name of the exporting module - useful for debugging.
+ * @exp_name:  name of the exporter - useful for debugging.
+ * @owner: pointer to exporter module - used for refcounting kernel module
  * @ops:   Attach allocator-defined dma buf ops to the new buffer
  * @size:  Size of the buffer
  * @flags: mode flags for the file
@@ -176,6 +180,7 @@ struct dma_buf_attachment {
  */
 struct dma_buf_export_info {
const char *exp_name;
+   struct module *owner;
const struct dma_buf_ops *ops;
size_t size;
int flags;
@@ -187,7 +192,8 @@ struct dma_buf_export_info {
  * helper macro for exporters; zeros and fills in most common values
  */
 #define DEFINE_DMA_BUF_EXPORT_INFO(a)  \
-   struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME }
+   struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME, \
+.owner = THIS_MODULE }

 /**
  * get_dma_buf - convenience wrapper for get_file.
-- 
1.9.1