[PATCH v2] sunrpc: simplify dprintk() macros and cleanup redundant debug guards

Sean Chang posted 1 patch 1 month, 1 week ago
fs/lockd/svclock.c                       |  7 -------
fs/nfsd/nfsfh.c                          |  8 +++-----
include/linux/sunrpc/debug.h             |  8 ++------
net/sunrpc/xprtrdma/svc_rdma_transport.c | 25 +++++++++++-------------
4 files changed, 16 insertions(+), 32 deletions(-)
[PATCH v2] sunrpc: simplify dprintk() macros and cleanup redundant debug guards
Posted by Sean Chang 1 month, 1 week ago
Following David Laight's suggestion, simplify the macro definitions by
removing the unnecessary 'fmt' argument and using no_printk(__VA_ARGS__)
directly.

Verification with .lst files under -O2 confirms that the compiler
successfully performs "dead code elimination". Even when variables
(like char buf[] in nfsfh.c) or static helper functions (like
nlmdbg_cookie2a() in svclock.c) are declared without #ifdef, they are
completely optimized out (no stack allocation, no symbol references in
the final executable) as they are only referenced within no_printk().

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Sean Chang <seanwascoding@gmail.com>
---
v2:
 - Follow reversed xmas tree order for variables in svc_rdma_transport.c
   as requested by Andy Shevchenko.
 - Polish commit message: use dprintk() and remove redundant file list.
 - Correct the technical claim about dprintk() type checking.

 fs/lockd/svclock.c                       |  7 -------
 fs/nfsd/nfsfh.c                          |  8 +++-----
 include/linux/sunrpc/debug.h             |  8 ++------
 net/sunrpc/xprtrdma/svc_rdma_transport.c | 25 +++++++++++-------------
 4 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index ee23f5802af1..9b978a087b3c 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -47,7 +47,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)
 {
 	/*
@@ -74,12 +73,6 @@ static const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
 
 	return buf;
 }
-#else
-static inline const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
-{
-	return "???";
-}
-#endif
 
 /*
  * Insert a blocked lock into the global list
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 68b629fbaaeb..91514326d1b4 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -105,12 +105,10 @@ static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
 {
 	/* Check if the request originated from a secure port. */
 	if (rqstp && !nfsd_originating_port_ok(rqstp, cred, exp)) {
-		if (IS_ENABLED(CONFIG_SUNRPC_DEBUG)) {
-			char buf[RPC_MAX_ADDRBUFLEN];
+		char buf[RPC_MAX_ADDRBUFLEN];
 
-			dprintk("nfsd: request from insecure port %s!\n",
-			        svc_print_addr(rqstp, buf, sizeof(buf)));
-		}
+		dprintk("nfsd: request from insecure port %s!\n",
+			svc_print_addr(rqstp, buf, sizeof(buf)));
 		return nfserr_perm;
 	}
 
diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index ab61bed2f7af..f6f2a106eeaf 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -38,8 +38,6 @@ extern unsigned int		nlm_debug;
 do {									\
 	ifdebug(fac)							\
 		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
-	else								\
-		no_printk(fmt, ##__VA_ARGS__);				\
 } while (0)
 
 # define dfprintk_rcu(fac, fmt, ...)					\
@@ -48,15 +46,13 @@ do {									\
 		rcu_read_lock();					\
 		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
 		rcu_read_unlock();					\
-	} else {							\
-		no_printk(fmt, ##__VA_ARGS__);				\
 	}								\
 } while (0)
 
 #else
 # define ifdebug(fac)		if (0)
-# define dfprintk(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
-# define dfprintk_rcu(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
+# define dfprintk(fac, ...)		no_printk(__VA_ARGS__)
+# define dfprintk_rcu(fac, ...)	no_printk(__VA_ARGS__)
 #endif
 
 /*
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index f2d72181a6fe..0759444bda50 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -413,6 +413,7 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
 	struct rpcrdma_connect_private pmsg;
 	struct ib_qp_init_attr qp_attr;
 	struct ib_device *dev;
+	struct sockaddr *sap;
 	int ret = 0;
 
 	listen_rdma = container_of(xprt, struct svcxprt_rdma, sc_xprt);
@@ -559,20 +560,16 @@ static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
 		goto errout;
 	}
 
-	if (IS_ENABLED(CONFIG_SUNRPC_DEBUG)) {
-		struct sockaddr *sap;
-
-		dprintk("svcrdma: new connection accepted on device %s:\n", dev->name);
-		sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
-		dprintk("    local address   : %pIS:%u\n", sap, rpc_get_port(sap));
-		sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
-		dprintk("    remote address  : %pIS:%u\n", sap, rpc_get_port(sap));
-		dprintk("    max_sge         : %d\n", newxprt->sc_max_send_sges);
-		dprintk("    sq_depth        : %d\n", newxprt->sc_sq_depth);
-		dprintk("    rdma_rw_ctxs    : %d\n", ctxts);
-		dprintk("    max_requests    : %d\n", newxprt->sc_max_requests);
-		dprintk("    ord             : %d\n", conn_param.initiator_depth);
-	}
+	dprintk("svcrdma: new connection accepted on device %s:\n", dev->name);
+	sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
+	dprintk("    local address   : %pIS:%u\n", sap, rpc_get_port(sap));
+	sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
+	dprintk("    remote address  : %pIS:%u\n", sap, rpc_get_port(sap));
+	dprintk("    max_sge         : %d\n", newxprt->sc_max_send_sges);
+	dprintk("    sq_depth        : %d\n", newxprt->sc_sq_depth);
+	dprintk("    rdma_rw_ctxs    : %d\n", ctxts);
+	dprintk("    max_requests    : %d\n", newxprt->sc_max_requests);
+	dprintk("    ord             : %d\n", conn_param.initiator_depth);
 
 	return &newxprt->sc_xprt;
 
-- 
2.34.1
Re: [PATCH v2] sunrpc: simplify dprintk() macros and cleanup redundant debug guards
Posted by Chuck Lever 4 weeks ago

On Tue, Mar 3, 2026, at 9:07 AM, Sean Chang wrote:
> Following David Laight's suggestion, simplify the macro definitions by
> removing the unnecessary 'fmt' argument and using no_printk(__VA_ARGS__)
> directly.
>
> Verification with .lst files under -O2 confirms that the compiler
> successfully performs "dead code elimination". Even when variables
> (like char buf[] in nfsfh.c) or static helper functions (like
> nlmdbg_cookie2a() in svclock.c) are declared without #ifdef, they are
> completely optimized out (no stack allocation, no symbol references in
> the final executable) as they are only referenced within no_printk().
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Sean Chang <seanwascoding@gmail.com>
> ---
> v2:
>  - Follow reversed xmas tree order for variables in svc_rdma_transport.c
>    as requested by Andy Shevchenko.
>  - Polish commit message: use dprintk() and remove redundant file list.
>  - Correct the technical claim about dprintk() type checking.
>
>  fs/lockd/svclock.c                       |  7 -------
>  fs/nfsd/nfsfh.c                          |  8 +++-----
>  include/linux/sunrpc/debug.h             |  8 ++------
>  net/sunrpc/xprtrdma/svc_rdma_transport.c | 25 +++++++++++-------------
>  4 files changed, 16 insertions(+), 32 deletions(-)
>
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index ee23f5802af1..9b978a087b3c 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -47,7 +47,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)
>  {
>  	/*
> @@ -74,12 +73,6 @@ static const char *nlmdbg_cookie2a(const struct 
> nlm_cookie *cookie)
> 
>  	return buf;
>  }
> -#else
> -static inline const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
> -{
> -	return "???";
> -}
> -#endif
> 
>  /*
>   * Insert a blocked lock into the global list
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 68b629fbaaeb..91514326d1b4 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -105,12 +105,10 @@ static __be32 nfsd_setuser_and_check_port(struct 
> svc_rqst *rqstp,
>  {
>  	/* Check if the request originated from a secure port. */
>  	if (rqstp && !nfsd_originating_port_ok(rqstp, cred, exp)) {
> -		if (IS_ENABLED(CONFIG_SUNRPC_DEBUG)) {
> -			char buf[RPC_MAX_ADDRBUFLEN];
> +		char buf[RPC_MAX_ADDRBUFLEN];
> 
> -			dprintk("nfsd: request from insecure port %s!\n",
> -			        svc_print_addr(rqstp, buf, sizeof(buf)));
> -		}
> +		dprintk("nfsd: request from insecure port %s!\n",
> +			svc_print_addr(rqstp, buf, sizeof(buf)));
>  		return nfserr_perm;
>  	}
> 
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index ab61bed2f7af..f6f2a106eeaf 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -38,8 +38,6 @@ extern unsigned int		nlm_debug;
>  do {									\
>  	ifdebug(fac)							\
>  		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
> -	else								\
> -		no_printk(fmt, ##__VA_ARGS__);				\
>  } while (0)
> 
>  # define dfprintk_rcu(fac, fmt, ...)					\
> @@ -48,15 +46,13 @@ do {									\
>  		rcu_read_lock();					\
>  		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
>  		rcu_read_unlock();					\
> -	} else {							\
> -		no_printk(fmt, ##__VA_ARGS__);				\
>  	}								\
>  } while (0)
> 
>  #else
>  # define ifdebug(fac)		if (0)
> -# define dfprintk(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
> -# define dfprintk_rcu(fac, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
> +# define dfprintk(fac, ...)		no_printk(__VA_ARGS__)
> +# define dfprintk_rcu(fac, ...)	no_printk(__VA_ARGS__)
>  #endif
> 
>  /*
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c 
> b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> index f2d72181a6fe..0759444bda50 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> @@ -413,6 +413,7 @@ static struct svc_xprt *svc_rdma_accept(struct 
> svc_xprt *xprt)
>  	struct rpcrdma_connect_private pmsg;
>  	struct ib_qp_init_attr qp_attr;
>  	struct ib_device *dev;
> +	struct sockaddr *sap;
>  	int ret = 0;
> 
>  	listen_rdma = container_of(xprt, struct svcxprt_rdma, sc_xprt);
> @@ -559,20 +560,16 @@ static struct svc_xprt *svc_rdma_accept(struct 
> svc_xprt *xprt)
>  		goto errout;
>  	}
> 
> -	if (IS_ENABLED(CONFIG_SUNRPC_DEBUG)) {
> -		struct sockaddr *sap;
> -
> -		dprintk("svcrdma: new connection accepted on device %s:\n", dev->name);
> -		sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
> -		dprintk("    local address   : %pIS:%u\n", sap, rpc_get_port(sap));
> -		sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
> -		dprintk("    remote address  : %pIS:%u\n", sap, rpc_get_port(sap));
> -		dprintk("    max_sge         : %d\n", newxprt->sc_max_send_sges);
> -		dprintk("    sq_depth        : %d\n", newxprt->sc_sq_depth);
> -		dprintk("    rdma_rw_ctxs    : %d\n", ctxts);
> -		dprintk("    max_requests    : %d\n", newxprt->sc_max_requests);
> -		dprintk("    ord             : %d\n", conn_param.initiator_depth);
> -	}
> +	dprintk("svcrdma: new connection accepted on device %s:\n", dev->name);
> +	sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
> +	dprintk("    local address   : %pIS:%u\n", sap, rpc_get_port(sap));
> +	sap = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
> +	dprintk("    remote address  : %pIS:%u\n", sap, rpc_get_port(sap));
> +	dprintk("    max_sge         : %d\n", newxprt->sc_max_send_sges);
> +	dprintk("    sq_depth        : %d\n", newxprt->sc_sq_depth);
> +	dprintk("    rdma_rw_ctxs    : %d\n", ctxts);
> +	dprintk("    max_requests    : %d\n", newxprt->sc_max_requests);
> +	dprintk("    ord             : %d\n", conn_param.initiator_depth);
> 
>  	return &newxprt->sc_xprt;
> 
> -- 
> 2.34.1

Has a subsystem tree been chosen through which to merge these two patches?


-- 
Chuck Lever
Re: [PATCH v2] sunrpc: simplify dprintk() macros and cleanup redundant debug guards
Posted by Sean Chang 4 weeks ago
On Thu, Mar 12, 2026 at 5:47 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Mar 03, 2026 at 10:07:25PM +0800, Sean Chang wrote:
> > Following David Laight's suggestion, simplify the macro definitions by
> > removing the unnecessary 'fmt' argument and using no_printk(__VA_ARGS__)
> > directly.
> >
> > Verification with .lst files under -O2 confirms that the compiler
> > successfully performs "dead code elimination". Even when variables
> > (like char buf[] in nfsfh.c) or static helper functions (like
> > nlmdbg_cookie2a() in svclock.c) are declared without #ifdef, they are
> > completely optimized out (no stack allocation, no symbol references in
> > the final executable) as they are only referenced within no_printk().
>
> Does this patch fixes also 202603110038.P6d14oxa-lkp@intel.com?
>

Hi Andy,
Regarding the LKP report:
I have reproduced the Sparse warning (void vs int mismatch) locally.
To resolve this, I'll use the do-while(0) form in v3 to ensure the macro
always evaluates to void:
-# define dfprintk(fac, ...)            no_printk(__VA_ARGS__)
-# define dfprintk_rcu(fac, ...)        no_printk(__VA_ARGS__)
+# define dfprintk(fac, ...)            do { no_printk(__VA_ARGS__); } while (0)
+# define dfprintk_rcu(fac, ...)        do { no_printk(__VA_ARGS__); } while (0)


On Thu, Mar 12, 2026 at 9:20 PM Chuck Lever <cel@kernel.org> wrote:
>
> Has a subsystem tree been chosen through which to merge these two patches?
>

Hi Chuck,
To make the review and merging process easier across different
subsystems, I will
send v3 as a 3-patch series:
[PATCH v3 1/3] sunrpc: Fix dprintk type mismatch using do-while(0)
    include/linux/sunrpc/debug.h
[PATCH v3 2/3] nfsd/lockd: Remove redundant debug checks
    fs/nfsd/nfsfh.c, fs/lockd/svclock.c
[PATCH v3 3/3] sunrpc/xprtrdma: Simplify variable declarations and debug checks
    net/sunrpc/xprtrdma/svc_rdma_transport.c

Best Regards,
Sean
Re: [PATCH v2] sunrpc: simplify dprintk() macros and cleanup redundant debug guards
Posted by Andy Shevchenko 4 weeks ago
On Thu, Mar 12, 2026 at 11:54:15PM +0800, Sean Chang wrote:
> On Thu, Mar 12, 2026 at 5:47 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Tue, Mar 03, 2026 at 10:07:25PM +0800, Sean Chang wrote:
> > > Following David Laight's suggestion, simplify the macro definitions by
> > > removing the unnecessary 'fmt' argument and using no_printk(__VA_ARGS__)
> > > directly.
> > >
> > > Verification with .lst files under -O2 confirms that the compiler
> > > successfully performs "dead code elimination". Even when variables
> > > (like char buf[] in nfsfh.c) or static helper functions (like
> > > nlmdbg_cookie2a() in svclock.c) are declared without #ifdef, they are
> > > completely optimized out (no stack allocation, no symbol references in
> > > the final executable) as they are only referenced within no_printk().
> >
> > Does this patch fixes also 202603110038.P6d14oxa-lkp@intel.com?
> 
> Regarding the LKP report:
> I have reproduced the Sparse warning (void vs int mismatch) locally.
> To resolve this, I'll use the do-while(0) form in v3 to ensure the macro
> always evaluates to void:
> -# define dfprintk(fac, ...)            no_printk(__VA_ARGS__)
> -# define dfprintk_rcu(fac, ...)        no_printk(__VA_ARGS__)
> +# define dfprintk(fac, ...)            do { no_printk(__VA_ARGS__); } while (0)
> +# define dfprintk_rcu(fac, ...)        do { no_printk(__VA_ARGS__); } while (0)

Wouldn't be better to drop ({}) in nfs_error*() macros?

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v2] sunrpc: simplify dprintk() macros and cleanup redundant debug guards
Posted by Andy Shevchenko 4 weeks ago
On Tue, Mar 03, 2026 at 10:07:25PM +0800, Sean Chang wrote:
> Following David Laight's suggestion, simplify the macro definitions by
> removing the unnecessary 'fmt' argument and using no_printk(__VA_ARGS__)
> directly.
> 
> Verification with .lst files under -O2 confirms that the compiler
> successfully performs "dead code elimination". Even when variables
> (like char buf[] in nfsfh.c) or static helper functions (like
> nlmdbg_cookie2a() in svclock.c) are declared without #ifdef, they are
> completely optimized out (no stack allocation, no symbol references in
> the final executable) as they are only referenced within no_printk().

Does this patch fixes also 202603110038.P6d14oxa-lkp@intel.com?

-- 
With Best Regards,
Andy Shevchenko