On Mon, Mar 25, 2013 at 04:50:45PM +0530, Sumit Semwal wrote:

For debugging purposes, it is useful to have a name-string added while exporting buffers. Hence, dma_buf_export() is replaced with dma_buf_export_named(), which additionally takes 'exp_name' as a parameter.

For backward compatibility, and for lazy exporters who don't wish to name themselves, a #define dma_buf_export() is also made available, which adds a __FILE__ instead of 'exp_name'.

Cc: Daniel Vetter daniel.vetter@ffwll.ch [Thanks for the idea!] Signed-off-by: Sumit Semwal sumit.semwal@linaro.org

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

---

Documentation/dma-buf-sharing.txt | 13 +++++++++++-- drivers/base/dma-buf.c | 11 +++++++---- include/linux/dma-buf.h | 11 +++++++++-- 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt index 4966b1b..0b23261 100644 --- a/Documentation/dma-buf-sharing.txt +++ b/Documentation/dma-buf-sharing.txt @@ -52,14 +52,23 @@ The dma_buf buffer sharing API usage contains the following steps: associated with this buffer. Interface:

•  struct dma_buf *dma_buf_export(void *priv, struct dma_buf_ops *ops,
  

• 		     size_t size, int flags)
  

•  struct dma_buf *dma_buf_export_named(void *priv, struct dma_buf_ops *ops,
  

• 		     size_t size, int flags,
  

• 		     const char *exp_name)
  

If this succeeds, dma_buf_export allocates a dma_buf structure, and returns a pointer to the same. It also associates an anonymous file with this buffer, so it can be exported. On failure to allocate the dma_buf object, it returns NULL.

• 'exp_name' is the name of exporter - to facilitate information while
• debugging.
• Exporting modules which do not wish to provide any specific name may use the
• helper define 'dma_buf_export()', with the same arguments as above, but
• without the last argument; a __FILE__ pre-processor directive will be
• inserted in place of 'exp_name' instead.

2. Userspace gets a handle to pass around to potential buffer-users

Userspace entity requests for a file-descriptor (fd) which is a handle to the diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index 2a7cb0d..d89102a 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -77,22 +77,24 @@ static inline int is_dma_buf_file(struct file *file) } /**

• • dma_buf_export - Creates a new dma_buf, and associates an anon file

• • dma_buf_export_named - Creates a new dma_buf, and associates an anon file
  • with this buffer, so it can be exported.
  • Also connect the allocator specific data and ops to the buffer.
• • Additionally, provide a name string for exporter; useful in debugging.
  • @priv: [in] Attach private data of allocator to this buffer
  • @ops: [in] Attach allocator-defined dma buf ops to the new buffer.
  • @size: [in] Size of the buffer
  • @flags: [in] mode flags for the file.
• • @exp_name: [in] name of the exporting module - useful for debugging.
  • Returns, on success, a newly created dma_buf object, which wraps the
  • supplied private data and operations for dma_buf_ops. On either missing
  • ops, or error in allocating struct dma_buf, will return negative error.
  */

-struct dma_buf *dma_buf_export(void *priv, const struct dma_buf_ops *ops,

• 		size_t size, int flags)
  

+struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,

• 		size_t size, int flags, const char *exp_name)
  

{ struct dma_buf *dmabuf; struct file *file; @@ -114,6 +116,7 @@ struct dma_buf *dma_buf_export(void *priv, const struct dma_buf_ops *ops, dmabuf->priv = priv; dmabuf->ops = ops; dmabuf->size = size;

• dmabuf->exp_name = exp_name;

file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags); @@ -124,7 +127,7 @@ struct dma_buf *dma_buf_export(void *priv, const struct dma_buf_ops *ops, return dmabuf; } -EXPORT_SYMBOL_GPL(dma_buf_export); +EXPORT_SYMBOL_GPL(dma_buf_export_named); /** diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 9978b61..6f55c04 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -112,6 +112,7 @@ struct dma_buf_ops {

• @file: file pointer used for sharing buffers across, and for refcounting.
• @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.
  • @priv: exporter specific private data for this buffer object.
  */

struct dma_buf { @@ -123,6 +124,7 @@ struct dma_buf { struct mutex lock; unsigned vmapping_counter; void *vmap_ptr;

• const char *exp_name; void *priv;

}; @@ -162,8 +164,13 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf, struct device *dev); void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *dmabuf_attach); -struct dma_buf *dma_buf_export(void *priv, const struct dma_buf_ops *ops,

• 	       size_t size, int flags);
  

+struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,

• 	       size_t size, int flags, const char *);
  

+#define dma_buf_export(priv, ops, size, flags) \

• dma_buf_export_named(priv, ops, size, flags, __FILE__)

int dma_buf_fd(struct dma_buf *dmabuf, int flags); struct dma_buf *dma_buf_get(int fd); void dma_buf_put(struct dma_buf *dmabuf); -- 1.7.10.4

Hi Sumit,

Thanks for the patch.

On Monday 25 March 2013 16:50:46 Sumit Semwal wrote:

Add debugfs support to make it easier to print debug information about the dma-buf buffers.

Cc: Dave Airlie airlied@redhat.com [minor fixes on init and warning fix] Signed-off-by: Sumit Semwal sumit.semwal@linaro.org

---

changes since v1:

• fixes on init and warnings as reported and corrected by Dave Airlie.
• add locking while walking attachment list - reported by Daniel Vetter.

drivers/base/dma-buf.c | 162 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/dma-buf.h | 5 +- 2 files changed, 166 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index d89102a..7d867ed 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -27,9 +27,18 @@ #include <linux/dma-buf.h> #include <linux/anon_inodes.h> #include <linux/export.h> +#include <linux/debugfs.h> +#include <linux/seq_file.h>

static inline int is_dma_buf_file(struct file *);

+struct dma_buf_list {

• struct list_head head;
• struct mutex lock;

+};

+static struct dma_buf_list db_list;

static int dma_buf_release(struct inode *inode, struct file *file) { struct dma_buf *dmabuf; @@ -42,6 +51,11 @@ static int dma_buf_release(struct inode *inode, struct file *file) BUG_ON(dmabuf->vmapping_counter);

dmabuf->ops->release(dmabuf);

• mutex_lock(&db_list.lock);
• list_del(&dmabuf->list_node);
• mutex_unlock(&db_list.lock);
• kfree(dmabuf); return 0;

} @@ -125,6 +139,10 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops, mutex_init(&dmabuf->lock); INIT_LIST_HEAD(&dmabuf->attachments);

• mutex_lock(&db_list.lock);
• list_add(&dmabuf->list_node, &db_list.head);
• mutex_unlock(&db_list.lock);
• return dmabuf;

} EXPORT_SYMBOL_GPL(dma_buf_export_named); @@ -551,3 +569,147 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) mutex_unlock(&dmabuf->lock); } EXPORT_SYMBOL_GPL(dma_buf_vunmap);

+static int dma_buf_init_debugfs(void); +static void dma_buf_uninit_debugfs(void);

+static int __init dma_buf_init(void) +{

• mutex_init(&db_list.lock);
• INIT_LIST_HEAD(&db_list.head);
• dma_buf_init_debugfs();
• return 0;

+}

+subsys_initcall(dma_buf_init);

+static void __exit dma_buf_deinit(void)

This function is never called.

+{

• dma_buf_uninit_debugfs();

+}

If you moved those two functions at the end of the file you could get rid of the forward declarations above.

+#ifdef CONFIG_DEBUG_FS +static int dma_buf_describe(struct seq_file *s) +{

• int ret;
• struct dma_buf *buf_obj;
• struct dma_buf_attachment *attach_obj;
• int count = 0, attach_count;
• size_t size = 0;
• ret = mutex_lock_interruptible(&db_list.lock);
• if (ret)

• return ret;
  

• seq_printf(s, "\nDma-buf Objects:\n");
• seq_printf(s, "\texp_name\tsize\tflags\tmode\tcount\n");
• list_for_each_entry(buf_obj, &db_list.head, list_node) {

• ret = mutex_lock_interruptible(&buf_obj->lock);
  

• if (ret) {
  

• 	seq_printf(s,
  

• 		  "\tERROR locking buffer object: skipping\n");
  

• 	goto skip_buffer;
  

• }
  

• seq_printf(s, "\t");
  

• seq_printf(s, "\t%s\t%08zu\t%08x\t%08x\t%08d\n",
  

• 		buf_obj->exp_name, buf_obj->size,
  

• 		buf_obj->file->f_flags, buf_obj->file->f_mode,
  

• 		buf_obj->file->f_count.counter);
  

• seq_printf(s, "\t\tAttached Devices:\n");
  

• attach_count = 0;
  

• list_for_each_entry(attach_obj, &buf_obj->attachments, node) {
  

• 	seq_printf(s, "\t\t");
  

• 	seq_printf(s, "%s\n", attach_obj->dev->init_name);
  

• 	attach_count++;
  

• }
  

• seq_printf(s, "\n\t\tTotal %d devices attached\n",
  

• 		attach_count);
  

• count++;
  

• size += buf_obj->size;
  

+skip_buffer:

• mutex_unlock(&buf_obj->lock);
  

• }
• seq_printf(s, "\nTotal %d objects, %zu bytes\n", count, size);
• mutex_unlock(&db_list.lock);
• return 0;

+}

+static int dma_buf_show(struct seq_file *s, void *unused) +{

• void (*func)(struct seq_file *) = s->private;
• func(s);
• return 0;

+}

+static int dma_buf_debug_open(struct inode *inode, struct file *file) +{

• return single_open(file, dma_buf_show, inode->i_private);

+}

+static const struct file_operations dma_buf_debug_fops = {

• .open = dma_buf_debug_open,
• .read = seq_read,
• .llseek = seq_lseek,
• .release = single_release,

+};

+static struct dentry *dma_buf_debugfs_dir;

+static int dma_buf_init_debugfs(void) +{

• int err = 0;
• dma_buf_debugfs_dir = debugfs_create_dir("dma_buf", NULL);
• if (IS_ERR(dma_buf_debugfs_dir)) {

• err = PTR_ERR(dma_buf_debugfs_dir);
  

• dma_buf_debugfs_dir = NULL;
  

• return err;
  

• }
• err = dma_buf_debugfs_create_file("bufinfo", dma_buf_describe);
• if (err)

• pr_debug("dma_buf: debugfs: failed to create node bufinfo\n");
  

• return err;

+}

+static void dma_buf_uninit_debugfs(void) +{

• if (dma_buf_debugfs_dir)

• debugfs_remove_recursive(dma_buf_debugfs_dir);
  

+}

+int dma_buf_debugfs_create_file(const char *name,

• 		int (*write)(struct seq_file *))
  

+{

• struct dentry *d;
• d = debugfs_create_file(name, S_IRUGO, dma_buf_debugfs_dir,

• 	write, &dma_buf_debug_fops);
  

• if (IS_ERR(d))

• return PTR_ERR(d);
  

• return 0;

+} +#else +static inline int dma_buf_init_debugfs(void) +{

• return 0;

+} +static inline void dma_buf_uninit_debugfs(void) +{ +} +#endif diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 6f55c04..dfac5ed 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -113,6 +113,7 @@ 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.

• • @list_node: node for dma_buf accounting and debugging.
  • @priv: exporter specific private data for this buffer object.
  */

struct dma_buf { @@ -125,6 +126,7 @@ struct dma_buf { unsigned vmapping_counter; void *vmap_ptr; const char *exp_name;

• struct list_head list_node; void *priv;

};

@@ -192,5 +194,6 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, unsigned long); void *dma_buf_vmap(struct dma_buf *); void dma_buf_vunmap(struct dma_buf *, void *vaddr);

+int dma_buf_debugfs_create_file(const char *name,

• 		int (*write)(struct seq_file *));
  

This function is only called internally from the same file, could it be declared as static ?

#endif /* __DMA_BUF_H__ */