On Thu, 2016-10-20 at 22:07 +0200, Nicolai Stange wrote: > From: Arnd Bergmann <[email protected]> > > 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_DEBUG_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.
Thanks for the fix! Looks good to me. Reviewed-by: Andy Shevchenko <[email protected]> > > 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 <[email protected]> > [[email protected]: Add dummy implementations of debugfs_attr_read() > and > debugfs_attr_write() in order to protect against possibly broken > dead > code elimination and to improve readability. > Correct CONFIG_DEBUGFS_FS -> CONFIG_DEBUG_FS typo in changelog.] > Signed-off-by: Nicolai Stange <[email protected]> > --- > Compile-tested on top of next-20161020 with gcc-4.9.0 and gcc-6.2.1, > CONFIG_INTEL_PMC_CORE=y and both, CONFIG_DEBUG_FS=n and > CONFIG_DEBUG_FS=y. > > I verified that there isn't anything in the intel_pmc_core.o that > shouldn'tbe there with CONFIG_DEBUG_FS=n. > > Changes to v1: > - Add dummy implementations of > debugfs_attr_read()/debugfs_attr_write() > for the CONFIG_DEBUG_FS=n case. > - Fix a typo in the changelog. > > > include/linux/debugfs.h | 44 +++++++++++++++++++++++++++------------- > ---- > 1 file changed, 27 insertions(+), 17 deletions(-) > > diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h > index 4d3f0d1..1b413a9 100644 > --- a/include/linux/debugfs.h > +++ b/include/linux/debugfs.h > @@ -62,6 +62,21 @@ static inline const struct file_operations > *debugfs_real_fops(struct file *filp) > return filp->f_path.dentry->d_fsdata; > } > > +#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, > \ > + .llseek = generic_file_llseek, > \ > +} > + > #if defined(CONFIG_DEBUG_FS) > > struct dentry *debugfs_create_file(const char *name, umode_t mode, > @@ -99,21 +114,6 @@ ssize_t debugfs_attr_read(struct file *file, char > __user *buf, > 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, > \ > - .llseek = generic_file_llseek, > \ > -} > - > struct dentry *debugfs_rename(struct dentry *old_dir, struct dentry > *old_dentry, > struct dentry *new_dir, const char *new_name); > > @@ -233,8 +233,18 @@ static inline void debugfs_use_file_finish(int > srcu_idx) > __releases(&debugfs_srcu) > { } > > -#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) > \ > - static const struct file_operations __fops = { 0 } > +static inline ssize_t debugfs_attr_read(struct file *file, char > __user *buf, > + size_t len, loff_t *ppos) > +{ > + return -ENODEV; > +} > + > +static inline ssize_t debugfs_attr_write(struct file *file, > + const char __user *buf, > + size_t len, loff_t *ppos) > +{ > + return -ENODEV; > +} > > static inline struct dentry *debugfs_rename(struct dentry *old_dir, > struct dentry *old_dentry, > struct dentry *new_dir, char *new_name) -- Andy Shevchenko <[email protected]> Intel Finland Oy
