lkml.org 
[lkml]   [2016]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/2] debugfs: improve DEFINE_DEBUGFS_ATTRIBUTE for !CONFIG_DEBUGFS_FS
Date
Hi Arnd,

thanks for this (and sorry for the late reply)!

Arnd Bergmann <arnd@arndb.de> writes:

> The slp_s0_residency_usec debugfs file currently uses
> DEFINE_DEBUGFS_ATTRIBUTE(), but that macro cannot really be used to
> define files outside of the debugfs code, as it has no reference to
> the get/set functions if CONFIG_DEBUGFS_FS is not defined:
>
> drivers/platform/x86/intel_pmc_core.c:80:12: error: ‘pmc_core_dev_state_get’ defined but not used [-Werror=unused-function]
>
> This fixes the macro to always contain the reference, and instead rely
> on the stubbed-out debugfs_create_file to not actually refer to
> its arguments so the compiler can still drop the reference.
> This works because the attribute definition is always 'static',
> and the dead-code removal silently drops all static symbols
> that are not used.
>
> Fixes: c64688081490 ("debugfs: add support for self-protecting attribute file fops")
> Fixes: df2294fb6428 ("intel_pmc_core: Convert to DEFINE_DEBUGFS_ATTRIBUTE")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> include/linux/debugfs.h | 43 ++++++++++++++++++++-----------------------
> 1 file changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index 4d3f0d1aec73..e94f5f8dced3 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -62,6 +62,26 @@ static inline const struct file_operations *debugfs_real_fops(struct file *filp)
> return filp->f_path.dentry->d_fsdata;
> }
>
> +ssize_t debugfs_attr_read(struct file *file, char __user *buf,
> + size_t len, loff_t *ppos);
> +ssize_t debugfs_attr_write(struct file *file, const char __user *buf,
> + size_t len, loff_t *ppos);
> +
> +#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \
> +static int __fops ## _open(struct inode *inode, struct file *file) \
> +{ \
> + __simple_attr_check_format(__fmt, 0ull); \
> + return simple_attr_open(inode, file, __get, __set, __fmt); \
> +} \
> +static const struct file_operations __fops = { \
> + .owner = THIS_MODULE, \
> + .open = __fops ## _open, \
> + .release = simple_attr_release, \
> + .read = debugfs_attr_read, \
> + .write = debugfs_attr_write, \

This depends on GCC dead code elimination to always work for this
situation, otherwise we'd get undefined references to
debugfs_attr_read/write(), right?

In order to avoid having to test your patch against all those older
versions of GCC, can we have a safety net here and define some dummy
debugfs_attr_read/write() for the !CONFIG_DEBUGFS case?

If nothing else, it would IMHO make the !CONFIG_DEBUGFS case more
understandable because one had not to figure out that this actually
relies on dead code elimination to work.

Thanks,

Nicolai

\
 
 \ /
  Last update: 2016-10-13 12:00    [W:1.867 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site