diff mbox series

[-next,v2] sunrpc: Use no_printk() in dfprintk*() dummies

Message ID a93de2e8afa826745746b00fc5f64e513df5d52f.1697104757.git.geert+renesas@glider.be (mailing list archive)
State New, archived
Headers show
Series [-next,v2] sunrpc: Use no_printk() in dfprintk*() dummies | expand

Commit Message

Geert Uytterhoeven Oct. 12, 2023, 10:08 a.m. UTC 
When building NFS with W=1 and CONFIG_WERROR=y, but
CONFIG_SUNRPC_DEBUG=n:

    fs/nfs/nfs4proc.c: In function ‘nfs4_proc_create_session’:
    fs/nfs/nfs4proc.c:9276:19: error: variable ‘ptr’ set but not used [-Werror=unused-but-set-variable]
     9276 |         unsigned *ptr;
	  |                   ^~~
      CC      fs/nfs/callback.o
    fs/nfs/callback.c: In function ‘nfs41_callback_svc’:
    fs/nfs/callback.c:98:13: error: variable ‘error’ set but not used [-Werror=unused-but-set-variable]
       98 |         int error;
	  |             ^~~~~
      CC      fs/nfs/flexfilelayout/flexfilelayout.o
    fs/nfs/flexfilelayout/flexfilelayout.c: In function ‘ff_layout_io_track_ds_error’:
    fs/nfs/flexfilelayout/flexfilelayout.c:1230:13: error: variable ‘err’ set but not used [-Werror=unused-but-set-variable]
     1230 |         int err;
	  |             ^~~
      CC      fs/nfs/flexfilelayout/flexfilelayoutdev.o
    fs/nfs/flexfilelayout/flexfilelayoutdev.c: In function ‘nfs4_ff_alloc_deviceid_node’:
    fs/nfs/flexfilelayout/flexfilelayoutdev.c:55:16: error: variable ‘ret’ set but not used [-Werror=unused-but-set-variable]
       55 |         int i, ret = -ENOMEM;
	  |                ^~~

All these are due to variables that are set unconditionally, but are
used only when debugging is enabled.

Fix this by changing the dfprintk*() dummy macros from empty loops to
calls to the no_printk() helper.  This informs the compiler that the
passed debug parameters are actually used, and enables format specifier
checking as a bonus.

This requires removing the protection by CONFIG_SUNRPC_DEBUG of the
declaration of nlmdbg_cookie2a(), as its reference is now visible to the
compiler, but optimized away.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - s/uncontionally/unconditionally/,
  - Drop CONFIG_SUNRPC_DEBUG check in fs/lockd/svclock.c to fix build
    failure.
---
 fs/lockd/svclock.c           | 2 --
 include/linux/sunrpc/debug.h | 6 +++---
 2 files changed, 3 insertions(+), 5 deletions(-)

Comments

Jeffrey Layton Oct. 12, 2023, 10:13 a.m. UTC | #1 
On Thu, 2023-10-12 at 12:08 +0200, Geert Uytterhoeven wrote:
> When building NFS with W=1 and CONFIG_WERROR=y, but
> CONFIG_SUNRPC_DEBUG=n:
> 
>     fs/nfs/nfs4proc.c: In function ‘nfs4_proc_create_session’:
>     fs/nfs/nfs4proc.c:9276:19: error: variable ‘ptr’ set but not used [-Werror=unused-but-set-variable]
>      9276 |         unsigned *ptr;
> 	  |                   ^~~
>       CC      fs/nfs/callback.o
>     fs/nfs/callback.c: In function ‘nfs41_callback_svc’:
>     fs/nfs/callback.c:98:13: error: variable ‘error’ set but not used [-Werror=unused-but-set-variable]
>        98 |         int error;
> 	  |             ^~~~~
>       CC      fs/nfs/flexfilelayout/flexfilelayout.o
>     fs/nfs/flexfilelayout/flexfilelayout.c: In function ‘ff_layout_io_track_ds_error’:
>     fs/nfs/flexfilelayout/flexfilelayout.c:1230:13: error: variable ‘err’ set but not used [-Werror=unused-but-set-variable]
>      1230 |         int err;
> 	  |             ^~~
>       CC      fs/nfs/flexfilelayout/flexfilelayoutdev.o
>     fs/nfs/flexfilelayout/flexfilelayoutdev.c: In function ‘nfs4_ff_alloc_deviceid_node’:
>     fs/nfs/flexfilelayout/flexfilelayoutdev.c:55:16: error: variable ‘ret’ set but not used [-Werror=unused-but-set-variable]
>        55 |         int i, ret = -ENOMEM;
> 	  |                ^~~
> 
> All these are due to variables that are set unconditionally, but are
> used only when debugging is enabled.
> 
> Fix this by changing the dfprintk*() dummy macros from empty loops to
> calls to the no_printk() helper.  This informs the compiler that the
> passed debug parameters are actually used, and enables format specifier
> checking as a bonus.
> 
> This requires removing the protection by CONFIG_SUNRPC_DEBUG of the
> declaration of nlmdbg_cookie2a(), as its reference is now visible to the
> compiler, but optimized away.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - s/uncontionally/unconditionally/,
>   - Drop CONFIG_SUNRPC_DEBUG check in fs/lockd/svclock.c to fix build
>     failure.
> ---
>  fs/lockd/svclock.c           | 2 --
>  include/linux/sunrpc/debug.h | 6 +++---
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index 43aeba9de55cbbc5..119a0c31d30eed4f 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -55,7 +55,6 @@ static const struct rpc_call_ops nlmsvc_grant_ops;
>  static LIST_HEAD(nlm_blocked);
>  static DEFINE_SPINLOCK(nlm_blocked_lock);
>  
> -#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
>  static const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
>  {
>  	/*
> @@ -82,7 +81,6 @@ static const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
>  
>  	return buf;
>  }
> -#endif
>  
>  /*
>   * Insert a blocked lock into the global list
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index f6aeed07fe04e3d5..76539c6673f2fb15 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -67,9 +67,9 @@ do {									\
>  # define RPC_IFDEBUG(x)		x
>  #else
>  # define ifdebug(fac)		if (0)
> -# define dfprintk(fac, fmt, ...)	do {} while (0)
> -# define dfprintk_cont(fac, fmt, ...)	do {} while (0)
> -# define dfprintk_rcu(fac, fmt, ...)	do {} while (0)
> +# define dfprintk(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
> +# define dfprintk_cont(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
> +# define dfprintk_rcu(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
>  # define RPC_IFDEBUG(x)
>  #endif
>  

Acked-by: Jeff Layton <jlayton@kernel.org>
Geert Uytterhoeven Oct. 12, 2023, 10:17 a.m. UTC | #2 
On Thu, Oct 12, 2023 at 12:08 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> When building NFS with W=1 and CONFIG_WERROR=y, but
> CONFIG_SUNRPC_DEBUG=n:
>
>     fs/nfs/nfs4proc.c: In function ‘nfs4_proc_create_session’:
>     fs/nfs/nfs4proc.c:9276:19: error: variable ‘ptr’ set but not used [-Werror=unused-but-set-variable]
>      9276 |         unsigned *ptr;
>           |                   ^~~
>       CC      fs/nfs/callback.o
>     fs/nfs/callback.c: In function ‘nfs41_callback_svc’:
>     fs/nfs/callback.c:98:13: error: variable ‘error’ set but not used [-Werror=unused-but-set-variable]
>        98 |         int error;
>           |             ^~~~~
>       CC      fs/nfs/flexfilelayout/flexfilelayout.o
>     fs/nfs/flexfilelayout/flexfilelayout.c: In function ‘ff_layout_io_track_ds_error’:
>     fs/nfs/flexfilelayout/flexfilelayout.c:1230:13: error: variable ‘err’ set but not used [-Werror=unused-but-set-variable]
>      1230 |         int err;
>           |             ^~~
>       CC      fs/nfs/flexfilelayout/flexfilelayoutdev.o
>     fs/nfs/flexfilelayout/flexfilelayoutdev.c: In function ‘nfs4_ff_alloc_deviceid_node’:
>     fs/nfs/flexfilelayout/flexfilelayoutdev.c:55:16: error: variable ‘ret’ set but not used [-Werror=unused-but-set-variable]
>        55 |         int i, ret = -ENOMEM;
>           |                ^~~
>
> All these are due to variables that are set unconditionally, but are
> used only when debugging is enabled.
>
> Fix this by changing the dfprintk*() dummy macros from empty loops to
> calls to the no_printk() helper.  This informs the compiler that the
> passed debug parameters are actually used, and enables format specifier
> checking as a bonus.
>
> This requires removing the protection by CONFIG_SUNRPC_DEBUG of the
> declaration of nlmdbg_cookie2a(), as its reference is now visible to the
> compiler, but optimized away.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - s/uncontionally/unconditionally/,
>   - Drop CONFIG_SUNRPC_DEBUG check in fs/lockd/svclock.c to fix build
>     failure.

The robots pointed out a second build failure, which is not fixed by this v2:
https://lore.kernel.org/all/202310121759.0CF34DcN-lkp@intel.com/

Gr{oetje,eeting}s,

                        Geert
kernel test robot Oct. 20, 2023, 12:15 a.m. UTC | #3 
Hi Geert,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20231016]

url:    https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/sunrpc-Use-no_printk-in-dfprintk-dummies/20231017-104813
base:   next-20231016
patch link:    https://lore.kernel.org/r/a93de2e8afa826745746b00fc5f64e513df5d52f.1697104757.git.geert%2Brenesas%40glider.be
patch subject: [PATCH -next v2] sunrpc: Use no_printk() in dfprintk*() dummies
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20231020/202310200734.I6x6UCe1-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231020/202310200734.I6x6UCe1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310200734.I6x6UCe1-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:31,
                    from arch/x86/include/asm/percpu.h:27,
                    from arch/x86/include/asm/current.h:10,
                    from include/linux/sched.h:12,
                    from include/linux/sunrpc/svcauth_gss.h:12,
                    from fs/nfsd/nfsfh.c:13:
   fs/nfsd/nfsfh.c: In function 'nfsd_setuser_and_check_port':
>> fs/nfsd/nfsfh.c:111:47: error: 'buf' undeclared (first use in this function)
     111 |                         svc_print_addr(rqstp, buf, sizeof(buf)));
         |                                               ^~~
   include/linux/printk.h:427:33: note: in definition of macro 'printk_index_wrap'
     427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                                 ^~~~~~~~~~~
   include/linux/printk.h:129:17: note: in expansion of macro 'printk'
     129 |                 printk(fmt, ##__VA_ARGS__);             \
         |                 ^~~~~~
   include/linux/sunrpc/debug.h:70:41: note: in expansion of macro 'no_printk'
      70 | # define dfprintk(fac, fmt, ...)        no_printk(fmt, ##__VA_ARGS__)
         |                                         ^~~~~~~~~
   include/linux/sunrpc/debug.h:25:9: note: in expansion of macro 'dfprintk'
      25 |         dfprintk(FACILITY, fmt, ##__VA_ARGS__)
         |         ^~~~~~~~
   fs/nfsd/nfsfh.c:110:17: note: in expansion of macro 'dprintk'
     110 |                 dprintk("nfsd: request from insecure port %s!\n",
         |                 ^~~~~~~
   fs/nfsd/nfsfh.c:111:47: note: each undeclared identifier is reported only once for each function it appears in
     111 |                         svc_print_addr(rqstp, buf, sizeof(buf)));
         |                                               ^~~
   include/linux/printk.h:427:33: note: in definition of macro 'printk_index_wrap'
     427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                                 ^~~~~~~~~~~
   include/linux/printk.h:129:17: note: in expansion of macro 'printk'
     129 |                 printk(fmt, ##__VA_ARGS__);             \
         |                 ^~~~~~
   include/linux/sunrpc/debug.h:70:41: note: in expansion of macro 'no_printk'
      70 | # define dfprintk(fac, fmt, ...)        no_printk(fmt, ##__VA_ARGS__)
         |                                         ^~~~~~~~~
   include/linux/sunrpc/debug.h:25:9: note: in expansion of macro 'dfprintk'
      25 |         dfprintk(FACILITY, fmt, ##__VA_ARGS__)
         |         ^~~~~~~~
   fs/nfsd/nfsfh.c:110:17: note: in expansion of macro 'dprintk'
     110 |                 dprintk("nfsd: request from insecure port %s!\n",
         |                 ^~~~~~~


vim +/buf +111 fs/nfsd/nfsfh.c

9d7ed1355db5b0 J. Bruce Fields 2018-03-08  101  
6fa02839bf9412 J. Bruce Fields 2007-11-12  102  static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
6fa02839bf9412 J. Bruce Fields 2007-11-12  103  					  struct svc_export *exp)
6fa02839bf9412 J. Bruce Fields 2007-11-12  104  {
12045a6ee9908b J. Bruce Fields 2009-12-08  105  	int flags = nfsexp_flags(rqstp, exp);
12045a6ee9908b J. Bruce Fields 2009-12-08  106  
6fa02839bf9412 J. Bruce Fields 2007-11-12  107  	/* Check if the request originated from a secure port. */
9d7ed1355db5b0 J. Bruce Fields 2018-03-08  108  	if (!nfsd_originating_port_ok(rqstp, flags)) {
5216a8e70e25b0 Pavel Emelyanov 2008-02-21  109  		RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
a48fd0f9f77b6e Kinglong Mee    2014-05-29  110  		dprintk("nfsd: request from insecure port %s!\n",
6fa02839bf9412 J. Bruce Fields 2007-11-12 @111  		        svc_print_addr(rqstp, buf, sizeof(buf)));
6fa02839bf9412 J. Bruce Fields 2007-11-12  112  		return nfserr_perm;
6fa02839bf9412 J. Bruce Fields 2007-11-12  113  	}
6fa02839bf9412 J. Bruce Fields 2007-11-12  114  
6fa02839bf9412 J. Bruce Fields 2007-11-12  115  	/* Set user creds for this exportpoint */
6fa02839bf9412 J. Bruce Fields 2007-11-12  116  	return nfserrno(nfsd_setuser(rqstp, exp));
6fa02839bf9412 J. Bruce Fields 2007-11-12  117  }
6fa02839bf9412 J. Bruce Fields 2007-11-12  118
kernel test robot Oct. 20, 2023, 1:40 a.m. UTC | #4 
Hi Geert,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20231016]

url:    https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/sunrpc-Use-no_printk-in-dfprintk-dummies/20231017-104813
base:   next-20231016
patch link:    https://lore.kernel.org/r/a93de2e8afa826745746b00fc5f64e513df5d52f.1697104757.git.geert%2Brenesas%40glider.be
patch subject: [PATCH -next v2] sunrpc: Use no_printk() in dfprintk*() dummies
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231020/202310200948.TK9Y6iGy-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231020/202310200948.TK9Y6iGy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310200948.TK9Y6iGy-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/nfsd/nfsfh.c:111:45: error: use of undeclared identifier 'buf'
                           svc_print_addr(rqstp, buf, sizeof(buf)));
                                                             ^
   fs/nfsd/nfsfh.c:111:33: error: use of undeclared identifier 'buf'
                           svc_print_addr(rqstp, buf, sizeof(buf)));
                                                 ^
   2 errors generated.


vim +/buf +111 fs/nfsd/nfsfh.c

9d7ed1355db5b00 J. Bruce Fields 2018-03-08  101  
6fa02839bf9412e J. Bruce Fields 2007-11-12  102  static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
6fa02839bf9412e J. Bruce Fields 2007-11-12  103  					  struct svc_export *exp)
6fa02839bf9412e J. Bruce Fields 2007-11-12  104  {
12045a6ee9908b3 J. Bruce Fields 2009-12-08  105  	int flags = nfsexp_flags(rqstp, exp);
12045a6ee9908b3 J. Bruce Fields 2009-12-08  106  
6fa02839bf9412e J. Bruce Fields 2007-11-12  107  	/* Check if the request originated from a secure port. */
9d7ed1355db5b00 J. Bruce Fields 2018-03-08  108  	if (!nfsd_originating_port_ok(rqstp, flags)) {
5216a8e70e25b01 Pavel Emelyanov 2008-02-21  109  		RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
a48fd0f9f77b6e1 Kinglong Mee    2014-05-29  110  		dprintk("nfsd: request from insecure port %s!\n",
6fa02839bf9412e J. Bruce Fields 2007-11-12 @111  		        svc_print_addr(rqstp, buf, sizeof(buf)));
6fa02839bf9412e J. Bruce Fields 2007-11-12  112  		return nfserr_perm;
6fa02839bf9412e J. Bruce Fields 2007-11-12  113  	}
6fa02839bf9412e J. Bruce Fields 2007-11-12  114  
6fa02839bf9412e J. Bruce Fields 2007-11-12  115  	/* Set user creds for this exportpoint */
6fa02839bf9412e J. Bruce Fields 2007-11-12  116  	return nfserrno(nfsd_setuser(rqstp, exp));
6fa02839bf9412e J. Bruce Fields 2007-11-12  117  }
6fa02839bf9412e J. Bruce Fields 2007-11-12  118
diff mbox series

Patch

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 43aeba9de55cbbc5..119a0c31d30eed4f 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -55,7 +55,6 @@  static const struct rpc_call_ops nlmsvc_grant_ops;
 static LIST_HEAD(nlm_blocked);
 static DEFINE_SPINLOCK(nlm_blocked_lock);
 
-#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 static const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
 {
 	/*
@@ -82,7 +81,6 @@  static const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
 
 	return buf;
 }
-#endif
 
 /*
  * Insert a blocked lock into the global list
diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index f6aeed07fe04e3d5..76539c6673f2fb15 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -67,9 +67,9 @@  do {									\
 # define RPC_IFDEBUG(x)		x
 #else
 # define ifdebug(fac)		if (0)
-# define dfprintk(fac, fmt, ...)	do {} while (0)
-# define dfprintk_cont(fac, fmt, ...)	do {} while (0)
-# define dfprintk_rcu(fac, fmt, ...)	do {} while (0)
+# define dfprintk(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
+# define dfprintk_cont(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
+# define dfprintk_rcu(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
 # define RPC_IFDEBUG(x)
 #endif