Re: [PATCH 3/3] dma-buf: remove dma_buf_debugfs_create_file()
On 20 June 2016 at 15:31, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Sun, Jun 19, 2016 at 02:31:31PM +0200, Mathias Krause wrote:
>> [...]
>> With no users left, we can remove dma_buf_debugfs_create_file().
>>
>> While at it, simplify the error handling in dma_buf_init_debugfs()
>> slightly.
>>
>> Signed-off-by: Mathias Krause <mini...@googlemail.com>
>
> ah, here's the 2nd part, feel free to ignore my earlier comments. On the
> series:
Yeah, I've split the original patch into three to separate bug fixes
(patch 1+2) from enhancements (this patch) -- just in case anybody
wants to backport the fixes.
Also, this way this patch can easily be left out without missing the
fixes, in case new debugfs files below dma_buf/ are expected in the
near future. Those might want to make use of
dma_buf_debugfs_create_file(). But, as there haven't been any since
its introduction in commit
b89e35636bc7 ("dma-buf: Add debugfs support") in 2013, I guess, we can
just remove that function. ;)
>
> Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Thanks,
Mathias
--
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
[PATCH 1/3] dma-buf: propagate errors from dma_buf_describe() on debugfs read
The callback function dma_buf_describe() returns an int not void so the
function pointer cast in dma_buf_show() is wrong. dma_buf_describe() can
also fail when acquiring the mutex gets interrupted so always returning
0 in dma_buf_show() is wrong, too.
Fix both issues by avoiding the indirection via dma_buf_show() and call
dma_buf_describe() directly. Rename it to dma_buf_debug_show() to get it
in line with the other functions.
This type mismatch was caught by the PaX RAP plugin.
Signed-off-by: Mathias Krause <mini...@googlemail.com>
Cc: Sumit Semwal <sumit.sem...@linaro.org>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: Brad Spengler <spen...@grsecurity.net>
Cc: PaX Team <pagee...@freemail.hu>
---
drivers/dma-buf/dma-buf.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 6355ab38d630..7094b19bb495 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -824,7 +824,7 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
EXPORT_SYMBOL_GPL(dma_buf_vunmap);
#ifdef CONFIG_DEBUG_FS
-static int dma_buf_describe(struct seq_file *s)
+static int dma_buf_debug_show(struct seq_file *s, void *unused)
{
int ret;
struct dma_buf *buf_obj;
@@ -879,17 +879,9 @@ static int dma_buf_describe(struct seq_file *s)
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);
+ return single_open(file, dma_buf_debug_show, NULL);
}
static const struct file_operations dma_buf_debug_fops = {
@@ -913,7 +905,7 @@ static int dma_buf_init_debugfs(void)
return err;
}
- err = dma_buf_debugfs_create_file("bufinfo", dma_buf_describe);
+ err = dma_buf_debugfs_create_file("bufinfo", NULL);
if (err)
pr_debug("dma_buf: debugfs: failed to create node bufinfo\n");
--
1.7.10.4
--
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
[PATCH 3/3] dma-buf: remove dma_buf_debugfs_create_file()
There is only a single user of dma_buf_debugfs_create_file() and that
one got the function pointer cast wrong. With that one fixed, there is
no need to have a wrapper for debugfs_create_file(), just call it
directly.
With no users left, we can remove dma_buf_debugfs_create_file().
While at it, simplify the error handling in dma_buf_init_debugfs()
slightly.
Signed-off-by: Mathias Krause <mini...@googlemail.com>
Cc: Sumit Semwal <sumit.sem...@linaro.org>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
---
drivers/dma-buf/dma-buf.c | 29 +
include/linux/dma-buf.h |2 --
2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f03e51561199..20ce0687b111 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -895,22 +895,22 @@ static struct dentry *dma_buf_debugfs_dir;
static int dma_buf_init_debugfs(void)
{
+ struct dentry *d;
int err = 0;
- dma_buf_debugfs_dir = debugfs_create_dir("dma_buf", NULL);
+ d = debugfs_create_dir("dma_buf", NULL);
+ if (IS_ERR(d))
+ return PTR_ERR(d);
- if (IS_ERR(dma_buf_debugfs_dir)) {
- err = PTR_ERR(dma_buf_debugfs_dir);
- dma_buf_debugfs_dir = NULL;
- return err;
- }
+ dma_buf_debugfs_dir = d;
- err = dma_buf_debugfs_create_file("bufinfo", NULL);
-
- if (err) {
+ d = debugfs_create_file("bufinfo", S_IRUGO, dma_buf_debugfs_dir,
+ NULL, _buf_debug_fops);
+ if (IS_ERR(d)) {
pr_debug("dma_buf: debugfs: failed to create node bufinfo\n");
debugfs_remove_recursive(dma_buf_debugfs_dir);
dma_buf_debugfs_dir = NULL;
+ err = PTR_ERR(d);
}
return err;
@@ -921,17 +921,6 @@ 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);
-
- return PTR_ERR_OR_ZERO(d);
-}
#else
static inline int dma_buf_init_debugfs(void)
{
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 4551c6f2a6c4..e0b0741ae671 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -242,6 +242,4 @@ 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 *));
#endif /* __DMA_BUF_H__ */
--
1.7.10.4
--
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
[PATCH 2/3] dma-buf: remove dma_buf directory on bufinfo file creation errors
Change the error handling in dma_buf_init_debugfs() to remove the
"dma_buf" directory if creating the "bufinfo" file fails. No need to
have an empty debugfs directory around.
Signed-off-by: Mathias Krause <mini...@googlemail.com>
Cc: Sumit Semwal <sumit.sem...@linaro.org>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
---
drivers/dma-buf/dma-buf.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 7094b19bb495..f03e51561199 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -907,8 +907,11 @@ static int dma_buf_init_debugfs(void)
err = dma_buf_debugfs_create_file("bufinfo", NULL);
- if (err)
+ if (err) {
pr_debug("dma_buf: debugfs: failed to create node bufinfo\n");
+ debugfs_remove_recursive(dma_buf_debugfs_dir);
+ dma_buf_debugfs_dir = NULL;
+ }
return err;
}
--
1.7.10.4
--
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
[PATCH 0/3] dma-buf: debugfs fixes
This small series is the v2 of the patch posted initially here: http://www.spinics.net/lists/linux-media/msg101347.html It not only fixes the type mix-up and addresses Daniel's remark (patch 1), it also smoothes out the error handling in dma_buf_init_debugfs() (patch 2) and removes the then unneeded function dma_buf_debugfs_create_file() (patch 3). Please apply! Mathias Krause (3): dma-buf: propagate errors from dma_buf_describe() on debugfs read dma-buf: remove dma_buf directory on bufinfo file creation errors dma-buf: remove dma_buf_debugfs_create_file() drivers/dma-buf/dma-buf.c | 44 ++-- include/linux/dma-buf.h |2 -- 2 files changed, 14 insertions(+), 32 deletions(-) -- 1.7.10.4 -- 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: propagate errors from dma_buf_describe() on debugfs read
On 19 June 2016 at 10:45, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Fri, Jun 17, 2016 at 08:57:03PM +0200, Mathias Krause wrote:
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 6355ab38d630..0f2a4592fdd2 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -881,10 +881,9 @@ static int dma_buf_describe(struct seq_file *s)
>>
>> static int dma_buf_show(struct seq_file *s, void *unused)
>> {
>> - void (*func)(struct seq_file *) = s->private;
>> + int (*func)(struct seq_file *) = s->private;
>>
>> - func(s);
>> - return 0;
>> + return func(s);
>
> Probably even better to just nuke that indirection. Set this pointer to
> NULL and inline dma_buf_describe into dma_buf_show.
Even further, we can get rid of dma_buf_debugfs_create_file() too as
it's only used to create this indirection.
I'll send a v2 just doing that.
Thanks,
Mathias
--
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
[PATCH] dma-buf: propagate errors from dma_buf_describe() on debugfs read
The callback function dma_buf_describe() returns an int not void so the
function pointer cast in dma_buf_show() is wrong. dma_buf_describe() can
also fail when acquiring the mutex gets interrupted so always returning
0 in dma_buf_show() is wrong, too.
Fix both issues by casting the function pointer to the correct type and
propagate its return value.
This type mismatch was caught by the PaX RAP plugin.
Signed-off-by: Mathias Krause <mini...@googlemail.com>
Cc: Sumit Semwal <sumit.sem...@linaro.org>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: PaX Team <pagee...@freemail.hu>
---
drivers/dma-buf/dma-buf.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 6355ab38d630..0f2a4592fdd2 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -881,10 +881,9 @@ static int dma_buf_describe(struct seq_file *s)
static int dma_buf_show(struct seq_file *s, void *unused)
{
- void (*func)(struct seq_file *) = s->private;
+ int (*func)(struct seq_file *) = s->private;
- func(s);
- return 0;
+ return func(s);
}
static int dma_buf_debug_open(struct inode *inode, struct file *file)
--
1.7.10.4
--
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
[PATCH] [media] OMAP1: fix use after free
Even though clk_put() is a no-op on most architectures it is not for some ARM implementations. To not fail on those, release the clock timer before freeing the surrounding structure. This bug was spotted by the semantic patch tool coccinelle using the script found at scripts/coccinelle/free/kfree.cocci. More information about semantic patching is available at http://coccinelle.lip6.fr/ Signed-off-by: Mathias Krause mini...@googlemail.com --- drivers/media/video/omap1_camera.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/omap1_camera.c b/drivers/media/video/omap1_camera.c index 0a2fb2b..9ed1513 100644 --- a/drivers/media/video/omap1_camera.c +++ b/drivers/media/video/omap1_camera.c @@ -1664,10 +1664,10 @@ static int __exit omap1_cam_remove(struct platform_device *pdev) res = pcdev-res; release_mem_region(res-start, resource_size(res)); - kfree(pcdev); - clk_put(pcdev-clk); + kfree(pcdev); + dev_info(pdev-dev, OMAP1 Camera Interface driver unloaded\n); return 0; -- 1.5.6.5 -- 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
