[PATCH 3/3] dma-buf: remove dma_buf_debugfs_create_file()
On 20 June 2016 at 15:31, Daniel Vetter 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
>
> 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
Thanks,
Mathias
[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
Cc: Sumit Semwal
Cc: Daniel Vetter
---
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
[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
Cc: Sumit Semwal
Cc: Daniel Vetter
---
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
[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
Cc: Sumit Semwal
Cc: Daniel Vetter
Cc: Brad Spengler
Cc: PaX Team
---
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
[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
[PATCH] dma-buf: propagate errors from dma_buf_describe() on debugfs read
On 19 June 2016 at 10:45, Daniel Vetter 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
[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
Cc: Sumit Semwal
Cc: Daniel Vetter
Cc: PaX Team
---
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
[PATCH] drm/i915: Remove bogus __init annotation from DMI callbacks
The __init annotations for the DMI callback functions are wrong as this
code can be called even after the module has been initialized, e.g. like
this:
# echo 1 > /sys/bus/pci/devices/:00:02.0/remove
# modprobe i915
# echo 1 > /sys/bus/pci/rescan
The first command will remove the PCI device from the kernel's device
list so the second command won't see it right away. But as it registers
a PCI driver it'll see it on the third command. If the system happens to
match one of the DMI table entries we'll try to call a function in long
released memory and generate an Oops, at best.
Fix this by removing the bogus annotation.
Modpost should have caught that one but it ignores section reference
mismatches from the .rodata section. :/
Fixes: 25e341cfc33d ("drm/i915: quirk away broken OpRegion VBT")
Fixes: 8ca4013d702d ("CHROMIUM: i915: Add DMI override to skip CRT...")
Fixes: 425d244c8670 ("drm/i915: ignore LVDS on intel graphics systems...")
Signed-off-by: Mathias Krause
Cc: Daniel Vetter
Cc: Duncan Laurie
Cc: Jarod Wilson
Cc: Rusty Russell # Can modpost be fixed?
Cc: stable at vger.kernel.org
---
In the long run me might want to move the DMI tests to some __init code
to be able to mark the DMI tables as __initconst, thereby allowing to
release this memory after module initialization. That would safe us some
~11 kB of memory, as the DMI data shouldn't change at run-time.
drivers/gpu/drm/i915/intel_bios.c |2 +-
drivers/gpu/drm/i915/intel_crt.c |2 +-
drivers/gpu/drm/i915/intel_lvds.c |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index a66955037e4e..eee79e1c3222 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1123,7 +1123,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
}
}
-static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id
*id)
+static int intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
{
DRM_DEBUG_KMS("Falling back to manually reading VBT from "
"VBIOS ROM for %s\n",
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index e8abfce40976..9212e6504e0f 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -804,7 +804,7 @@ static const struct drm_encoder_funcs intel_crt_enc_funcs =
{
.destroy = intel_encoder_destroy,
};
-static int __init intel_no_crt_dmi_callback(const struct dmi_system_id *id)
+static int intel_no_crt_dmi_callback(const struct dmi_system_id *id)
{
DRM_INFO("Skipping CRT initialization for %s\n", id->ident);
return 1;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c
index 881361c0f27e..fdf40267249c 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -538,7 +538,7 @@ static const struct drm_encoder_funcs intel_lvds_enc_funcs
= {
.destroy = intel_encoder_destroy,
};
-static int __init intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
+static int intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
{
DRM_INFO("Skipping LVDS initialization for %s\n", id->ident);
return 1;
--
1.7.10.4
