From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934815AbcJUJXJ (ORCPT ); Fri, 21 Oct 2016 05:23:09 -0400 Received: from mga06.intel.com ([134.134.136.31]:60868 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933159AbcJUJXD (ORCPT ); Fri, 21 Oct 2016 05:23:03 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,375,1473145200"; d="scan'208";a="775568080" Message-ID: <1477041778.6423.0.camel@linux.intel.com> Subject: Re: [PATCH v2] debugfs: improve DEFINE_DEBUGFS_ATTRIBUTE for !CONFIG_DEBUG_FS From: Andy Shevchenko To: Nicolai Stange , Greg Kroah-Hartman Cc: Arnd Bergmann , Rajneesh Bhardwaj , Darren Hart , linux-kernel@vger.kernel.org Date: Fri, 21 Oct 2016 12:22:58 +0300 In-Reply-To: <20161020200753.3252-1-nicstange@gmail.com> References: <5795991.0oAm3xDhR2@wuerfel> <20161020200753.3252-1-nicstange@gmail.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2016-10-20 at 22:07 +0200, Nicolai Stange wrote: > From: Arnd Bergmann > > 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 > > 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 > [nicstange@gmail.com: 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 > --- > 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 Intel Finland Oy