[PATCH] dma-buf: Add debugfs support

2012-12-20 Thread Dave Airlie
On Thu, Dec 20, 2012 at 11:26 AM, Dave Airlie  wrote:
> On Fri, Dec 14, 2012 at 7:36 PM,   wrote:
>> From: Sumit Semwal 
>>
>> Add debugfs support to make it easier to print debug information
>> about the dma-buf buffers.
>>

I've attached two patches that make it work on my system, and fix the warnings,

I've used it to debug some leaks I was seeing, feel free to squash
these patches into the original patch.

Dave.
-- next part --
A non-text attachment was scrubbed...
Name: 0001-dma_buf-fix-debugfs-init.patch
Type: application/octet-stream
Size: 1065 bytes
Desc: not available
URL: 

-- next part --
A non-text attachment was scrubbed...
Name: 0002-dma-buf-fix-warning-in-seq_printf.patch
Type: application/octet-stream
Size: 1215 bytes
Desc: not available
URL: 



[PATCH] dma-buf: Add debugfs support

2012-12-20 Thread Dave Airlie
On Fri, Dec 14, 2012 at 7:36 PM,   wrote:
> From: Sumit Semwal 
>
> Add debugfs support to make it easier to print debug information
> about the dma-buf buffers.
>

I like thie idea,

/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c: In function
?dma_buf_describe?:
/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c:563:5:
warning: format ?%d? expects argument of type ?int?, but argument 6
has type ?long int? [-Wformat]
/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c: At top level:
/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c:528:123:
warning: ?dma_buf_init? defined but not used [-Wunused-function]

not sure my compiler does though.

Dave.


Re: [PATCH] dma-buf: Add debugfs support

2012-12-19 Thread Dave Airlie
On Fri, Dec 14, 2012 at 7:36 PM,  sumit.sem...@ti.com wrote:
 From: Sumit Semwal sumit.sem...@linaro.org

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


I like thie idea,

/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c: In function
‘dma_buf_describe’:
/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c:563:5:
warning: format ‘%d’ expects argument of type ‘int’, but argument 6
has type ‘long int’ [-Wformat]
/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c: At top level:
/home/airlied/devel/kernel/drm-2.6/drivers/base/dma-buf.c:528:123:
warning: ‘dma_buf_init’ defined but not used [-Wunused-function]

not sure my compiler does though.

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


Re: [PATCH] dma-buf: Add debugfs support

2012-12-19 Thread Dave Airlie
On Thu, Dec 20, 2012 at 11:26 AM, Dave Airlie airl...@gmail.com wrote:
 On Fri, Dec 14, 2012 at 7:36 PM,  sumit.sem...@ti.com wrote:
 From: Sumit Semwal sumit.sem...@linaro.org

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


I've attached two patches that make it work on my system, and fix the warnings,

I've used it to debug some leaks I was seeing, feel free to squash
these patches into the original patch.

Dave.


0001-dma_buf-fix-debugfs-init.patch
Description: Binary data


0002-dma-buf-fix-warning-in-seq_printf.patch
Description: Binary data
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-17 Thread Sumit Semwal
Hi Maarten,

On 14 December 2012 17:27, Maarten Lankhorst wrote:

> Op 14-12-12 10:36, sumit.semwal at ti.com schreef:
> > From: Sumit Semwal 
> >
> > Add debugfs support to make it easier to print debug information
> > about the dma-buf buffers.
> >
> I like the idea, I don't know if it could be done in a free manner, but
> for bonus points
> could we also have the dma-buf fd be obtainable that way from a debugfs
> entry?
>
> Doing so would allow me to 'steal' a dma-buf from an existing mapping
> easily, and test against that.

Also I think the name of the device and process that exported the dma-buf
> would be useful
> to have as well, even if in case of the device that would mean changing
> the api slightly to record it.
>
> I was thinking of having a directory structure like this:
>
> /sys/kernel/debug/dma_buf/stats
>
> and then for each dma-buf:
>
> /sys/kernel/debug/dma-buf/exporting_file.c/-fd
> /sys/kernel/debug/dma-buf/exporting_file.c/-attachments
> /sys/kernel/debug/dma-buf/exporting_file.c/-info
>
> Opening the fd file would give you back the original fd, or fail with -EIO
> if refcount was dropped to 0.
>
> Would something like this be doable? I don't know debugfs that well, but I
> don't see why it wouldn't be,
>
Let me think more about it, but I am inclined to add simple support first,
and then add more features to dma_buf debugfs as it grows.

I still would want to take Daniel's suggestion on dma_buf_export_named()
before I push this patch, so I guess I'll try to work a little more and
prepare it for 3.9?

I quite like your idea of .../dma-buf//...  , which would
need the above as well :)

>
> ~Maarten
>
> Best regards,
~Sumit.
-- next part --
An HTML attachment was scrubbed...
URL: 



Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-17 Thread Francesco Lavra
Hi,

On 12/14/2012 10:36 AM, sumit.sem...@ti.com wrote:
 From: Sumit Semwal sumit.sem...@linaro.org
 
 Add debugfs support to make it easier to print debug information
 about the dma-buf buffers.
 
 Signed-off-by: Sumit Semwal sumit.sem...@ti.com
 ---
  drivers/base/dma-buf.c  |  149 
 +++
  include/linux/dma-buf.h |6 +-
  2 files changed, 154 insertions(+), 1 deletion(-)
[...]
 diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
 index bd2e52c..160453f 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.
 + * @list_node: node for dma_buf accounting and debugging.
   * @priv: exporter specific private data for this buffer object.
   */
  struct dma_buf {
 @@ -121,6 +122,8 @@ struct dma_buf {
   const struct dma_buf_ops *ops;
   /* mutex to serialize list manipulation and attach/detach */
   struct mutex lock;
 +
 + struct list_head list_node;
   void *priv;
  };
  
 @@ -183,5 +186,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 *));

Why is this function declared in the public header file?

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


Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-17 Thread Sumit Semwal
Hi Maarten,

On 14 December 2012 17:27, Maarten Lankhorst m.b.lankho...@gmail.comwrote:

 Op 14-12-12 10:36, sumit.sem...@ti.com schreef:
  From: Sumit Semwal sumit.sem...@linaro.org
 
  Add debugfs support to make it easier to print debug information
  about the dma-buf buffers.
 
 I like the idea, I don't know if it could be done in a free manner, but
 for bonus points
 could we also have the dma-buf fd be obtainable that way from a debugfs
 entry?

 Doing so would allow me to 'steal' a dma-buf from an existing mapping
 easily, and test against that.

Also I think the name of the device and process that exported the dma-buf
 would be useful
 to have as well, even if in case of the device that would mean changing
 the api slightly to record it.

 I was thinking of having a directory structure like this:

 /sys/kernel/debug/dma_buf/stats

 and then for each dma-buf:

 /sys/kernel/debug/dma-buf/exporting_file.c/number-fd
 /sys/kernel/debug/dma-buf/exporting_file.c/number-attachments
 /sys/kernel/debug/dma-buf/exporting_file.c/number-info

 Opening the fd file would give you back the original fd, or fail with -EIO
 if refcount was dropped to 0.

 Would something like this be doable? I don't know debugfs that well, but I
 don't see why it wouldn't be,

Let me think more about it, but I am inclined to add simple support first,
and then add more features to dma_buf debugfs as it grows.

I still would want to take Daniel's suggestion on dma_buf_export_named()
before I push this patch, so I guess I'll try to work a little more and
prepare it for 3.9?

I quite like your idea of .../dma-buf/exporting_file.c/...  , which would
need the above as well :)


 ~Maarten

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


Re: [PATCH] dma-buf: Add debugfs support

2012-12-17 Thread Sumit Semwal
On 14 December 2012 16:04, Daniel Vetter dan...@ffwll.ch wrote:

 Missed one ...

 On Fri, Dec 14, 2012 at 10:36 AM,  sumit.sem...@ti.com wrote:
  +   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++;
  +   }

 You need to hold dmabuf-lock while walking the attachment list.
 -Daniel


Thanks Daniel!

Will update in next version.

 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch



Best regards,

Sumit Semwal
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-17 Thread Sumit Semwal
On 17 December 2012 14:25, Sumit Semwal sumit.sem...@linaro.org wrote:
Apologies for re-sending, since the gmail ui 'decided' to set some
formatting options by default!
 Hi Maarten,

 On 14 December 2012 17:27, Maarten Lankhorst m.b.lankho...@gmail.com
 wrote:

 Op 14-12-12 10:36, sumit.sem...@ti.com schreef:
  From: Sumit Semwal sumit.sem...@linaro.org
 
  Add debugfs support to make it easier to print debug information
  about the dma-buf buffers.
 
 I like the idea, I don't know if it could be done in a free manner, but
 for bonus points
 could we also have the dma-buf fd be obtainable that way from a debugfs
 entry?

 Doing so would allow me to 'steal' a dma-buf from an existing mapping
 easily, and test against that.

 Also I think the name of the device and process that exported the dma-buf
 would be useful
 to have as well, even if in case of the device that would mean changing
 the api slightly to record it.

 I was thinking of having a directory structure like this:

 /sys/kernel/debug/dma_buf/stats

 and then for each dma-buf:

 /sys/kernel/debug/dma-buf/exporting_file.c/number-fd
 /sys/kernel/debug/dma-buf/exporting_file.c/number-attachments
 /sys/kernel/debug/dma-buf/exporting_file.c/number-info

 Opening the fd file would give you back the original fd, or fail with -EIO
 if refcount was dropped to 0.

 Would something like this be doable? I don't know debugfs that well, but I
 don't see why it wouldn't be,

 Let me think more about it, but I am inclined to add simple support first,
 and then add more features to dma_buf debugfs as it grows.

 I still would want to take Daniel's suggestion on dma_buf_export_named()
 before I push this patch, so I guess I'll try to work a little more and
 prepare it for 3.9?

 I quite like your idea of .../dma-buf/exporting_file.c/...  , which would
 need the above as well :)


 ~Maarten

 Best regards,
 ~Sumit.




--
Thanks and regards,

Sumit Semwal

Linaro Kernel Engineer - Graphics working group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-16 Thread Francesco Lavra
Hi,

On 12/14/2012 10:36 AM, sumit.semwal at ti.com wrote:
> From: Sumit Semwal 
> 
> Add debugfs support to make it easier to print debug information
> about the dma-buf buffers.
> 
> Signed-off-by: Sumit Semwal 
> ---
>  drivers/base/dma-buf.c  |  149 
> +++
>  include/linux/dma-buf.h |6 +-
>  2 files changed, 154 insertions(+), 1 deletion(-)
[...]
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index bd2e52c..160453f 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.
> + * @list_node: node for dma_buf accounting and debugging.
>   * @priv: exporter specific private data for this buffer object.
>   */
>  struct dma_buf {
> @@ -121,6 +122,8 @@ struct dma_buf {
>   const struct dma_buf_ops *ops;
>   /* mutex to serialize list manipulation and attach/detach */
>   struct mutex lock;
> +
> + struct list_head list_node;
>   void *priv;
>  };
>  
> @@ -183,5 +186,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 *));

Why is this function declared in the public header file?

--
Francesco


[Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Maarten Lankhorst
Op 14-12-12 15:11, Rob Clark schreef:
> On Fri, Dec 14, 2012 at 5:57 AM, Maarten Lankhorst
>  wrote:
>> Op 14-12-12 10:36, sumit.semwal at ti.com schreef:
>>> From: Sumit Semwal 
>>>
>>> Add debugfs support to make it easier to print debug information
>>> about the dma-buf buffers.
>>>
>> I like the idea, I don't know if it could be done in a free manner, but for 
>> bonus points
>> could we also have the dma-buf fd be obtainable that way from a debugfs 
>> entry?
>>
>> Doing so would allow me to 'steal' a dma-buf from an existing mapping 
>> easily, and test against that.
>>
>> Also I think the name of the device and process that exported the dma-buf 
>> would be useful
>> to have as well, even if in case of the device that would mean changing the 
>> api slightly to record it.
>>
>> I was thinking of having a directory structure like this:
>>
>> /sys/kernel/debug/dma_buf/stats
>>
>> and then for each dma-buf:
>>
>> /sys/kernel/debug/dma-buf/exporting_file.c/-fd
>> /sys/kernel/debug/dma-buf/exporting_file.c/-attachments
>> /sys/kernel/debug/dma-buf/exporting_file.c/-info
>>
>> Opening the fd file would give you back the original fd, or fail with -EIO 
>> if refcount was dropped to 0.
>>
>> Would something like this be doable? I don't know debugfs that well, but I 
>> don't see why it wouldn't be,
> yeah.. but sort of back-door's the security benefits of an anonymous fd..
>
> BR,
> -R
If you have access to debugfs you're root, so what stops you from stealing it 
through a ptrace?

~Maarten


[PATCH] dma-buf: Add debugfs support

2012-12-14 Thread sumit.sem...@ti.com
From: Sumit Semwal 

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

Signed-off-by: Sumit Semwal 
---
 drivers/base/dma-buf.c  |  149 +++
 include/linux/dma-buf.h |6 +-
 2 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index a3f79c4..49fcf0f 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -27,9 +27,18 @@
 #include 
 #include 
 #include 
+#include 
+#include 

 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;
@@ -40,6 +49,11 @@ static int dma_buf_release(struct inode *inode, struct file 
*file)
dmabuf = file->private_data;

dmabuf->ops->release(dmabuf);
+
+   mutex_lock(_list.lock);
+   list_del(>list_node);
+   mutex_unlock(_list.lock);
+
kfree(dmabuf);
return 0;
 }
@@ -120,6 +134,10 @@ struct dma_buf *dma_buf_export(void *priv, const struct 
dma_buf_ops *ops,
mutex_init(>lock);
INIT_LIST_HEAD(>attachments);

+   mutex_lock(_list.lock);
+   list_add(>list_node, _list.head);
+   mutex_unlock(_list.lock);
+
return dmabuf;
 }
 EXPORT_SYMBOL_GPL(dma_buf_export);
@@ -505,3 +523,134 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
dmabuf->ops->vunmap(dmabuf, vaddr);
 }
 EXPORT_SYMBOL_GPL(dma_buf_vunmap);
+
+static int dma_buf_init_debugfs(void);
+static void dma_buf_uninit_debugfs(void);
+
+static void __init dma_buf_init(void)
+{
+   mutex_init(_list.lock);
+   INIT_LIST_HEAD(_list.head);
+   dma_buf_init_debugfs();
+}
+
+static void __exit dma_buf_deinit(void)
+{
+   dma_buf_uninit_debugfs();
+}
+
+#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(_list.lock);
+
+   if (ret)
+   return ret;
+
+   seq_printf(s, "\nDma-buf Objects:\n");
+   seq_printf(s, "\tsize\tflags\tmode\tcount\n");
+
+   list_for_each_entry(buf_obj, _list.head, list_node) {
+   seq_printf(s, "\t");
+
+   seq_printf(s, "%08zu\t%08x\t%08x\t%08d\n",
+   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, _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;
+   }
+
+   seq_printf(s, "\nTotal %d objects, %zu bytes\n", count, size);
+
+   mutex_unlock(_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, _buf_debug_fops);
+
+   if (IS_ERR(d))
+   return PTR_ERR(d);
+
+

[Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Maarten Lankhorst
Op 14-12-12 10:36, sumit.semwal at ti.com schreef:
> From: Sumit Semwal 
>
> Add debugfs support to make it easier to print debug information
> about the dma-buf buffers.
>
I like the idea, I don't know if it could be done in a free manner, but for 
bonus points
could we also have the dma-buf fd be obtainable that way from a debugfs entry?

Doing so would allow me to 'steal' a dma-buf from an existing mapping easily, 
and test against that.

Also I think the name of the device and process that exported the dma-buf would 
be useful
to have as well, even if in case of the device that would mean changing the api 
slightly to record it.

I was thinking of having a directory structure like this:

/sys/kernel/debug/dma_buf/stats

and then for each dma-buf:

/sys/kernel/debug/dma-buf/exporting_file.c/-fd
/sys/kernel/debug/dma-buf/exporting_file.c/-attachments
/sys/kernel/debug/dma-buf/exporting_file.c/-info

Opening the fd file would give you back the original fd, or fail with -EIO if 
refcount was dropped to 0.

Would something like this be doable? I don't know debugfs that well, but I 
don't see why it wouldn't be,

~Maarten



[PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Daniel Vetter
Missed one ...

On Fri, Dec 14, 2012 at 10:36 AM,   wrote:
> +   list_for_each_entry(attach_obj, _obj->attachments, node) {
> +   seq_printf(s, "\t\t");
> +
> +   seq_printf(s, "%s\n", attach_obj->dev->init_name);
> +   attach_count++;
> +   }

You need to hold dmabuf->lock while walking the attachment list.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Daniel Vetter
On Fri, Dec 14, 2012 at 10:36 AM,   wrote:
> From: Sumit Semwal 
>
> Add debugfs support to make it easier to print debug information
> about the dma-buf buffers.
>
> Signed-off-by: Sumit Semwal 

Looks line, only nitpick is that we have no idea who exported a given
buffer. So what about adding a new dma_buf_export_named which takes a
char * to identify the exporter, and then add a bit of cpp magic like
this:

#define dma_buf_export(args) dma_buf_export_named(args, stringify(__FILE))

At least for drm drivers using the prime helpers this should add the
file of the exporting driver, so would be good enough to identify the
exporter. Or we could add a dma_buf_export_dev which has a struct
device * as additional argument ... Otoh that wouldn't really work for
exporting dma_bufs from ION, so maybe a const char * is better.
-Daniel

> ---
>  drivers/base/dma-buf.c  |  149 
> +++
>  include/linux/dma-buf.h |6 +-
>  2 files changed, 154 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index a3f79c4..49fcf0f 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -27,9 +27,18 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>
>  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;
> @@ -40,6 +49,11 @@ static int dma_buf_release(struct inode *inode, struct 
> file *file)
> dmabuf = file->private_data;
>
> dmabuf->ops->release(dmabuf);
> +
> +   mutex_lock(_list.lock);
> +   list_del(>list_node);
> +   mutex_unlock(_list.lock);
> +
> kfree(dmabuf);
> return 0;
>  }
> @@ -120,6 +134,10 @@ struct dma_buf *dma_buf_export(void *priv, const struct 
> dma_buf_ops *ops,
> mutex_init(>lock);
> INIT_LIST_HEAD(>attachments);
>
> +   mutex_lock(_list.lock);
> +   list_add(>list_node, _list.head);
> +   mutex_unlock(_list.lock);
> +
> return dmabuf;
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_export);
> @@ -505,3 +523,134 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
> dmabuf->ops->vunmap(dmabuf, vaddr);
>  }
>  EXPORT_SYMBOL_GPL(dma_buf_vunmap);
> +
> +static int dma_buf_init_debugfs(void);
> +static void dma_buf_uninit_debugfs(void);
> +
> +static void __init dma_buf_init(void)
> +{
> +   mutex_init(_list.lock);
> +   INIT_LIST_HEAD(_list.head);
> +   dma_buf_init_debugfs();
> +}
> +
> +static void __exit dma_buf_deinit(void)
> +{
> +   dma_buf_uninit_debugfs();
> +}
> +
> +#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(_list.lock);
> +
> +   if (ret)
> +   return ret;
> +
> +   seq_printf(s, "\nDma-buf Objects:\n");
> +   seq_printf(s, "\tsize\tflags\tmode\tcount\n");
> +
> +   list_for_each_entry(buf_obj, _list.head, list_node) {
> +   seq_printf(s, "\t");
> +
> +   seq_printf(s, "%08zu\t%08x\t%08x\t%08d\n",
> +   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, _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;
> +   }
> +
> +   seq_printf(s, "\nTotal %d objects, %zu bytes\n", count, size);
> +
> +   mutex_unlock(_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 

[Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Rob Clark
On Fri, Dec 14, 2012 at 5:57 AM, Maarten Lankhorst
 wrote:
> Op 14-12-12 10:36, sumit.semwal at ti.com schreef:
>> From: Sumit Semwal 
>>
>> Add debugfs support to make it easier to print debug information
>> about the dma-buf buffers.
>>
> I like the idea, I don't know if it could be done in a free manner, but for 
> bonus points
> could we also have the dma-buf fd be obtainable that way from a debugfs entry?
>
> Doing so would allow me to 'steal' a dma-buf from an existing mapping easily, 
> and test against that.
>
> Also I think the name of the device and process that exported the dma-buf 
> would be useful
> to have as well, even if in case of the device that would mean changing the 
> api slightly to record it.
>
> I was thinking of having a directory structure like this:
>
> /sys/kernel/debug/dma_buf/stats
>
> and then for each dma-buf:
>
> /sys/kernel/debug/dma-buf/exporting_file.c/-fd
> /sys/kernel/debug/dma-buf/exporting_file.c/-attachments
> /sys/kernel/debug/dma-buf/exporting_file.c/-info
>
> Opening the fd file would give you back the original fd, or fail with -EIO if 
> refcount was dropped to 0.
>
> Would something like this be doable? I don't know debugfs that well, but I 
> don't see why it wouldn't be,

yeah.. but sort of back-door's the security benefits of an anonymous fd..

BR,
-R

> ~Maarten
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] dma-buf: Add debugfs support

2012-12-14 Thread sumit.semwal
From: Sumit Semwal sumit.sem...@linaro.org

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

Signed-off-by: Sumit Semwal sumit.sem...@ti.com
---
 drivers/base/dma-buf.c  |  149 +++
 include/linux/dma-buf.h |6 +-
 2 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index a3f79c4..49fcf0f 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;
@@ -40,6 +49,11 @@ static int dma_buf_release(struct inode *inode, struct file 
*file)
dmabuf = file-private_data;
 
dmabuf-ops-release(dmabuf);
+
+   mutex_lock(db_list.lock);
+   list_del(dmabuf-list_node);
+   mutex_unlock(db_list.lock);
+
kfree(dmabuf);
return 0;
 }
@@ -120,6 +134,10 @@ struct dma_buf *dma_buf_export(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);
@@ -505,3 +523,134 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
dmabuf-ops-vunmap(dmabuf, vaddr);
 }
 EXPORT_SYMBOL_GPL(dma_buf_vunmap);
+
+static int dma_buf_init_debugfs(void);
+static void dma_buf_uninit_debugfs(void);
+
+static void __init dma_buf_init(void)
+{
+   mutex_init(db_list.lock);
+   INIT_LIST_HEAD(db_list.head);
+   dma_buf_init_debugfs();
+}
+
+static void __exit dma_buf_deinit(void)
+{
+   dma_buf_uninit_debugfs();
+}
+
+#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, \tsize\tflags\tmode\tcount\n);
+
+   list_for_each_entry(buf_obj, db_list.head, list_node) {
+   seq_printf(s, \t);
+
+   seq_printf(s, %08zu\t%08x\t%08x\t%08d\n,
+   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;
+   }
+
+   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,
+  

Re: [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Daniel Vetter
On Fri, Dec 14, 2012 at 10:36 AM,  sumit.sem...@ti.com wrote:
 From: Sumit Semwal sumit.sem...@linaro.org

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

 Signed-off-by: Sumit Semwal sumit.sem...@ti.com

Looks line, only nitpick is that we have no idea who exported a given
buffer. So what about adding a new dma_buf_export_named which takes a
char * to identify the exporter, and then add a bit of cpp magic like
this:

#define dma_buf_export(args) dma_buf_export_named(args, stringify(__FILE))

At least for drm drivers using the prime helpers this should add the
file of the exporting driver, so would be good enough to identify the
exporter. Or we could add a dma_buf_export_dev which has a struct
device * as additional argument ... Otoh that wouldn't really work for
exporting dma_bufs from ION, so maybe a const char * is better.
-Daniel

 ---
  drivers/base/dma-buf.c  |  149 
 +++
  include/linux/dma-buf.h |6 +-
  2 files changed, 154 insertions(+), 1 deletion(-)

 diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
 index a3f79c4..49fcf0f 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;
 @@ -40,6 +49,11 @@ static int dma_buf_release(struct inode *inode, struct 
 file *file)
 dmabuf = file-private_data;

 dmabuf-ops-release(dmabuf);
 +
 +   mutex_lock(db_list.lock);
 +   list_del(dmabuf-list_node);
 +   mutex_unlock(db_list.lock);
 +
 kfree(dmabuf);
 return 0;
  }
 @@ -120,6 +134,10 @@ struct dma_buf *dma_buf_export(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);
 @@ -505,3 +523,134 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
 dmabuf-ops-vunmap(dmabuf, vaddr);
  }
  EXPORT_SYMBOL_GPL(dma_buf_vunmap);
 +
 +static int dma_buf_init_debugfs(void);
 +static void dma_buf_uninit_debugfs(void);
 +
 +static void __init dma_buf_init(void)
 +{
 +   mutex_init(db_list.lock);
 +   INIT_LIST_HEAD(db_list.head);
 +   dma_buf_init_debugfs();
 +}
 +
 +static void __exit dma_buf_deinit(void)
 +{
 +   dma_buf_uninit_debugfs();
 +}
 +
 +#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, \tsize\tflags\tmode\tcount\n);
 +
 +   list_for_each_entry(buf_obj, db_list.head, list_node) {
 +   seq_printf(s, \t);
 +
 +   seq_printf(s, %08zu\t%08x\t%08x\t%08d\n,
 +   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;
 +   }
 +
 +   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 

Re: [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Daniel Vetter
Missed one ...

On Fri, Dec 14, 2012 at 10:36 AM,  sumit.sem...@ti.com wrote:
 +   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++;
 +   }

You need to hold dmabuf-lock while walking the attachment list.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Maarten Lankhorst
Op 14-12-12 10:36, sumit.sem...@ti.com schreef:
 From: Sumit Semwal sumit.sem...@linaro.org

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

I like the idea, I don't know if it could be done in a free manner, but for 
bonus points
could we also have the dma-buf fd be obtainable that way from a debugfs entry?

Doing so would allow me to 'steal' a dma-buf from an existing mapping easily, 
and test against that.

Also I think the name of the device and process that exported the dma-buf would 
be useful
to have as well, even if in case of the device that would mean changing the api 
slightly to record it.

I was thinking of having a directory structure like this:

/sys/kernel/debug/dma_buf/stats

and then for each dma-buf:

/sys/kernel/debug/dma-buf/exporting_file.c/number-fd
/sys/kernel/debug/dma-buf/exporting_file.c/number-attachments
/sys/kernel/debug/dma-buf/exporting_file.c/number-info

Opening the fd file would give you back the original fd, or fail with -EIO if 
refcount was dropped to 0.

Would something like this be doable? I don't know debugfs that well, but I 
don't see why it wouldn't be,

~Maarten

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


Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Rob Clark
On Fri, Dec 14, 2012 at 5:57 AM, Maarten Lankhorst
m.b.lankho...@gmail.com wrote:
 Op 14-12-12 10:36, sumit.sem...@ti.com schreef:
 From: Sumit Semwal sumit.sem...@linaro.org

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

 I like the idea, I don't know if it could be done in a free manner, but for 
 bonus points
 could we also have the dma-buf fd be obtainable that way from a debugfs entry?

 Doing so would allow me to 'steal' a dma-buf from an existing mapping easily, 
 and test against that.

 Also I think the name of the device and process that exported the dma-buf 
 would be useful
 to have as well, even if in case of the device that would mean changing the 
 api slightly to record it.

 I was thinking of having a directory structure like this:

 /sys/kernel/debug/dma_buf/stats

 and then for each dma-buf:

 /sys/kernel/debug/dma-buf/exporting_file.c/number-fd
 /sys/kernel/debug/dma-buf/exporting_file.c/number-attachments
 /sys/kernel/debug/dma-buf/exporting_file.c/number-info

 Opening the fd file would give you back the original fd, or fail with -EIO if 
 refcount was dropped to 0.

 Would something like this be doable? I don't know debugfs that well, but I 
 don't see why it wouldn't be,

yeah.. but sort of back-door's the security benefits of an anonymous fd..

BR,
-R

 ~Maarten

 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support

2012-12-14 Thread Maarten Lankhorst
Op 14-12-12 15:11, Rob Clark schreef:
 On Fri, Dec 14, 2012 at 5:57 AM, Maarten Lankhorst
 m.b.lankho...@gmail.com wrote:
 Op 14-12-12 10:36, sumit.sem...@ti.com schreef:
 From: Sumit Semwal sumit.sem...@linaro.org

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

 I like the idea, I don't know if it could be done in a free manner, but for 
 bonus points
 could we also have the dma-buf fd be obtainable that way from a debugfs 
 entry?

 Doing so would allow me to 'steal' a dma-buf from an existing mapping 
 easily, and test against that.

 Also I think the name of the device and process that exported the dma-buf 
 would be useful
 to have as well, even if in case of the device that would mean changing the 
 api slightly to record it.

 I was thinking of having a directory structure like this:

 /sys/kernel/debug/dma_buf/stats

 and then for each dma-buf:

 /sys/kernel/debug/dma-buf/exporting_file.c/number-fd
 /sys/kernel/debug/dma-buf/exporting_file.c/number-attachments
 /sys/kernel/debug/dma-buf/exporting_file.c/number-info

 Opening the fd file would give you back the original fd, or fail with -EIO 
 if refcount was dropped to 0.

 Would something like this be doable? I don't know debugfs that well, but I 
 don't see why it wouldn't be,
 yeah.. but sort of back-door's the security benefits of an anonymous fd..

 BR,
 -R
If you have access to debugfs you're root, so what stops you from stealing it 
through a ptrace?

~Maarten
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel