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

["Chuck Lever" <cel@kernel.org>]

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