Re: [PATCH] dma-buf: Add debugfs support
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. 0001-dma_buf-fix-debugfs-init.patch Description: Binary data 0002-dma-buf-fix-warning-in-seq_printf.patch Description: Binary data
Re: [PATCH] dma-buf: Add debugfs support
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. -- 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
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
On 17 December 2012 14:25, Sumit Semwal 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 > wrote: >> >> Op 14-12-12 10:36, sumit.sem...@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. > -- Thanks and regards, Sumit Semwal Linaro Kernel Engineer - Graphics working group Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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
Re: [PATCH] dma-buf: Add debugfs support
On 14 December 2012 16:04, Daniel Vetter wrote: > > Missed one ... > > On Fri, Dec 14, 2012 at 10:36 AM, 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 -- 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
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
Hi, On 12/14/2012 10:36 AM, sumit.sem...@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 -- 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
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
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.sem...@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 -- 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
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
On Fri, Dec 14, 2012 at 5:57 AM, Maarten Lankhorst wrote: > Op 14-12-12 10:36, sumit.sem...@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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
Op 14-12-12 10:36, sumit.sem...@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 -- 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
Re: [PATCH] dma-buf: Add debugfs support
Missed one ... On Fri, Dec 14, 2012 at 10:36 AM, 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 -- 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
Re: [PATCH] dma-buf: Add debugfs support
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(&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 stru
[PATCH] dma-buf: Add debugfs support
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(&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, + write, &dma_buf_debug_fops); + + if (IS_E