When CONFIG_SUNRPC_DEBUG is disabled, the dfprintk() macros currently
expand to empty do-while loops. This causes variables used solely
within these calls to appear unused, triggering -Wunused-variable
warnings.
Instead of marking every affected variable with __maybe_unused,
update the dfprintk and dfprintk_rcu stubs to use no_printk().
This allows the compiler to see the variables and perform type
checking without emitting any code, thus silencing the warnings
globally for these macros.
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Sean Chang <seanwascoding@gmail.com>
---
include/linux/sunrpc/debug.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index eb4bd62df319..55c54df8bc7d 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -52,8 +52,8 @@ do { \
# define RPC_IFDEBUG(x) x
#else
# define ifdebug(fac) if (0)
-# define dfprintk(fac, fmt, ...) do {} while (0)
-# define dfprintk_rcu(fac, fmt, ...) do {} while (0)
+# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
+# define dfprintk_rcu(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
# define RPC_IFDEBUG(x)
#endif
--
2.34.1On Fri, 27 Feb 2026 23:26:23 +0800
Sean Chang <seanwascoding@gmail.com> wrote:
> When CONFIG_SUNRPC_DEBUG is disabled, the dfprintk() macros currently
> expand to empty do-while loops. This causes variables used solely
> within these calls to appear unused, triggering -Wunused-variable
> warnings.
>
> Instead of marking every affected variable with __maybe_unused,
> update the dfprintk and dfprintk_rcu stubs to use no_printk().
> This allows the compiler to see the variables and perform type
> checking without emitting any code, thus silencing the warnings
> globally for these macros.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Sean Chang <seanwascoding@gmail.com>
> ---
> include/linux/sunrpc/debug.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index eb4bd62df319..55c54df8bc7d 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -52,8 +52,8 @@ do { \
> # define RPC_IFDEBUG(x) x
> #else
> # define ifdebug(fac) if (0)
> -# define dfprintk(fac, fmt, ...) do {} while (0)
> -# define dfprintk_rcu(fac, fmt, ...) do {} while (0)
> +# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> +# define dfprintk_rcu(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
You can omit fmt, then you don't need the ##
#define dfprintk(fac, ...) no_printk(__VA_ARGS__)
David
> # define RPC_IFDEBUG(x)
> #endif
>
> > # define ifdebug(fac) if (0)
> > -# define dfprintk(fac, fmt, ...) do {} while (0)
> > -# define dfprintk_rcu(fac, fmt, ...) do {} while (0)
> > +# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> > +# define dfprintk_rcu(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
>
> You can omit fmt, then you don't need the ##
> #define dfprintk(fac, ...) no_printk(__VA_ARGS__)
/*
* Dummy printk for disabled debugging statements to use whilst maintaining
* gcc's format checking.
*/
#define no_printk(fmt, ...) \
({ \
if (0) \
_printk(fmt, ##__VA_ARGS__); \
0; \
})
Without fmt, gcc cannot do format checking. Or worse, it takes the
first member of __VA_ARGS__ as the format, and gives spurious errors?
Andrew
On Fri, 27 Feb 2026 18:57:33 +0100
Andrew Lunn <andrew@lunn.ch> wrote:
> > > # define ifdebug(fac) if (0)
> > > -# define dfprintk(fac, fmt, ...) do {} while (0)
> > > -# define dfprintk_rcu(fac, fmt, ...) do {} while (0)
> > > +# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> > > +# define dfprintk_rcu(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> >
> > You can omit fmt, then you don't need the ##
> > #define dfprintk(fac, ...) no_printk(__VA_ARGS__)
>
> /*
> * Dummy printk for disabled debugging statements to use whilst maintaining
> * gcc's format checking.
> */
> #define no_printk(fmt, ...) \
> ({ \
> if (0) \
> _printk(fmt, ##__VA_ARGS__); \
> 0; \
> })
>
> Without fmt, gcc cannot do format checking. Or worse, it takes the
> first member of __VA_ARGS__ as the format, and gives spurious errors?
By the time the compiler looks at it the pre-processor has expanded
__VA_ARGS__.
So it doesn't matter that the format string is in the __VA_ARGS__
list rather than preceding it.
David
>
> Andrew
On Sat, Feb 28, 2026 at 2:15 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Fri, 27 Feb 2026 18:57:33 +0100
> Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > # define ifdebug(fac) if (0)
> > > > -# define dfprintk(fac, fmt, ...) do {} while (0)
> > > > -# define dfprintk_rcu(fac, fmt, ...) do {} while (0)
> > > > +# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> > > > +# define dfprintk_rcu(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> > >
> > > You can omit fmt, then you don't need the ##
> > > #define dfprintk(fac, ...) no_printk(__VA_ARGS__)
> >
> > /*
> > * Dummy printk for disabled debugging statements to use whilst maintaining
> > * gcc's format checking.
> > */
> > #define no_printk(fmt, ...) \
> > ({ \
> > if (0) \
> > _printk(fmt, ##__VA_ARGS__); \
> > 0; \
> > })
> >
> > Without fmt, gcc cannot do format checking. Or worse, it takes the
> > first member of __VA_ARGS__ as the format, and gives spurious errors?
>
> By the time the compiler looks at it the pre-processor has expanded
> __VA_ARGS__.
> So it doesn't matter that the format string is in the __VA_ARGS__
> list rather than preceding it.
>
Hi David,
Thanks for the explanation. I agree that since __VA_ARGS__ will
always contain the format string in this context, the ## and explicit
fmt are indeed redundant. Since no_printk itself already handles
the ##__VA_ARGS__ logic internally, there's no need to duplicate
it in the dfprintk definition.
I'll switch to the simpler in v5:
-# 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__)
Best regards,
Sean
On Sat, Feb 28, 2026 at 10:56 PM Sean Chang <seanwascoding@gmail.com> wrote:
>
> On Sat, Feb 28, 2026 at 2:15 AM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Fri, 27 Feb 2026 18:57:33 +0100
> > Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > > # define ifdebug(fac) if (0)
> > > > > -# define dfprintk(fac, fmt, ...) do {} while (0)
> > > > > -# define dfprintk_rcu(fac, fmt, ...) do {} while (0)
> > > > > +# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> > > > > +# define dfprintk_rcu(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> > > >
> > > > You can omit fmt, then you don't need the ##
> > > > #define dfprintk(fac, ...) no_printk(__VA_ARGS__)
> > >
> > > /*
> > > * Dummy printk for disabled debugging statements to use whilst maintaining
> > > * gcc's format checking.
> > > */
> > > #define no_printk(fmt, ...) \
> > > ({ \
> > > if (0) \
> > > _printk(fmt, ##__VA_ARGS__); \
> > > 0; \
> > > })
> > >
> > > Without fmt, gcc cannot do format checking. Or worse, it takes the
> > > first member of __VA_ARGS__ as the format, and gives spurious errors?
> >
> > By the time the compiler looks at it the pre-processor has expanded
> > __VA_ARGS__.
> > So it doesn't matter that the format string is in the __VA_ARGS__
> > list rather than preceding it.
> >
>
> Hi David,
> Thanks for the explanation. I agree that since __VA_ARGS__ will
> always contain the format string in this context, the ## and explicit
> fmt are indeed redundant. Since no_printk itself already handles
> the ##__VA_ARGS__ logic internally, there's no need to duplicate
> it in the dfprintk definition.
>
> I'll switch to the simpler in v5:
> -# 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__)
>
HI all,
I've also identified the cause of the build error in fs/nfsd/nfsfh.c
reported by syzbot [1]. The "use of undeclared identifier 'buf'" occurs
because buf is only declared within the RPC_IFDEBUG macro,
which is removed when CONFIG_SUNRPC_DEBUG is disabled.
To fix this, I will follow the pattern used in net/sunrpc/xprtrdma/
svc_rdma_transport.c by wrapping the dprintk call that references
buf within an #if IS_ENABLED(CONFIG_SUNRPC_DEBUG) block.
static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
{
....
RPC_IFDEBUG(struct sockaddr *sap);
...
#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
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);
#endif
...
}
The refactor in fs/nfsd/nfsfh.c will look like this:
static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
struct svc_cred *cred,
struct svc_export *exp)
{
/* Check if the request originated from a secure port. */
if (rqstp && !nfsd_originating_port_ok(rqstp, cred, exp)) {
RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
+#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
dprintk("nfsd: request from insecure port %s!\n",
svc_print_addr(rqstp, buf, sizeof(buf)));
+#endif
return nfserr_perm;
}
/* Set user creds for this exportpoint */
return nfserrno(nfsd_setuser(cred, exp));
}
This fix, along with the macro simplification suggested by David, will
be included in v5.
[1] https://lore.kernel.org/all/69a2e269.050a0220.3a55be.003e.GAE@google.com/
Best regards,
Sean
> HI all,
> I've also identified the cause of the build error in fs/nfsd/nfsfh.c
> reported by syzbot [1]. The "use of undeclared identifier 'buf'" occurs
> because buf is only declared within the RPC_IFDEBUG macro,
> which is removed when CONFIG_SUNRPC_DEBUG is disabled.
> To fix this, I will follow the pattern used in net/sunrpc/xprtrdma/
> svc_rdma_transport.c by wrapping the dprintk call that references
> buf within an #if IS_ENABLED(CONFIG_SUNRPC_DEBUG) block.
Please try to avoid adding such #if code. Compile testing does not
work as well if there are millions of #if def combinations. Ideally we
want the stub functions to allow as much as possible to be compiled,
and then let the optimizer throw it out because it is unreachable.
>
> static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
> {
> ....
> RPC_IFDEBUG(struct sockaddr *sap);
> ...
> #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> 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);
> #endif
> ...
> }
>
> The refactor in fs/nfsd/nfsfh.c will look like this:
>
> static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
> struct svc_cred *cred,
> struct svc_export *exp)
> {
> /* Check if the request originated from a secure port. */
> if (rqstp && !nfsd_originating_port_ok(rqstp, cred, exp)) {
> RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
> +#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> dprintk("nfsd: request from insecure port %s!\n",
> svc_print_addr(rqstp, buf, sizeof(buf)));
> +#endif
In this case, now dprintk() uses it arguments, i think you can drop
the RPC_IFDEBUG() and always have buf. This code then gets
compiled. Ask make to produce fs/nfsd/nfsfh.lst and see if it
generated any code for this, or has it optimized it out.
Andrew
On Sun, Mar 1, 2026 at 4:04 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> Please try to avoid adding such #if code. Compile testing does not
> work as well if there are millions of #if def combinations. Ideally we
> want the stub functions to allow as much as possible to be compiled,
> and then let the optimizer throw it out because it is unreachable.
>
> >
> > static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt)
> > {
> > ....
> > RPC_IFDEBUG(struct sockaddr *sap);
> > ...
> > #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> > 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);
> > #endif
> > ...
> > }
> >
> > The refactor in fs/nfsd/nfsfh.c will look like this:
> >
> > static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
> > struct svc_cred *cred,
> > struct svc_export *exp)
> > {
> > /* Check if the request originated from a secure port. */
> > if (rqstp && !nfsd_originating_port_ok(rqstp, cred, exp)) {
> > RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
> > +#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> > dprintk("nfsd: request from insecure port %s!\n",
> > svc_print_addr(rqstp, buf, sizeof(buf)));
> > +#endif
>
> In this case, now dprintk() uses it arguments, i think you can drop
> the RPC_IFDEBUG() and always have buf. This code then gets
> compiled. Ask make to produce fs/nfsd/nfsfh.lst and see if it
> generated any code for this, or has it optimized it out.
>
Hi Andrew,
You are absolutely right. I have verified the generated code with
fs/nfsd/nfsfh.lst and net/sunrpc/xprtrdma/svc_rdma_transport.lst
under -O2 optimization.
Even after dropping the #ifdef and always declaring the char buf[],
the compiler successfully optimizes everything out when
CONFIG_SUNRPC_DEBUG is disabled. Specifically:
- There is no stack allocation (no sub %rsp) for the buffer.
- The logic flows directly as if the dprintk lines were never there.
Given this, I agree it is the perfect time to also remove the
RPC_IFDEBUG(x) macro entirely, as it is only used in these
two files and is now redundant with the new no_printk approach.
I will send out v6 shortly with these cleanups.
Best Regards,
Sean
On Fri, Feb 27, 2026 at 11:26:23PM +0800, Sean Chang wrote:
> When CONFIG_SUNRPC_DEBUG is disabled, the dfprintk() macros currently
> expand to empty do-while loops. This causes variables used solely
> within these calls to appear unused, triggering -Wunused-variable
> warnings.
>
> Instead of marking every affected variable with __maybe_unused,
> update the dfprintk and dfprintk_rcu stubs to use no_printk().
> This allows the compiler to see the variables and perform type
> checking without emitting any code, thus silencing the warnings
> globally for these macros.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Sean Chang <seanwascoding@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
On Fri, Feb 27, 2026, at 10:26 AM, Sean Chang wrote:
> When CONFIG_SUNRPC_DEBUG is disabled, the dfprintk() macros currently
> expand to empty do-while loops. This causes variables used solely
> within these calls to appear unused, triggering -Wunused-variable
> warnings.
>
> Instead of marking every affected variable with __maybe_unused,
> update the dfprintk and dfprintk_rcu stubs to use no_printk().
> This allows the compiler to see the variables and perform type
> checking without emitting any code, thus silencing the warnings
> globally for these macros.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Sean Chang <seanwascoding@gmail.com>
> ---
> include/linux/sunrpc/debug.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index eb4bd62df319..55c54df8bc7d 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -52,8 +52,8 @@ do { \
> # define RPC_IFDEBUG(x) x
> #else
> # define ifdebug(fac) if (0)
> -# define dfprintk(fac, fmt, ...) do {} while (0)
> -# define dfprintk_rcu(fac, fmt, ...) do {} while (0)
> +# define dfprintk(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> +# define dfprintk_rcu(fac, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> # define RPC_IFDEBUG(x)
> #endif
>
> --
> 2.34.1
Seems obviously good to me.
Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
--
Chuck Lever
© 2016 - 2026 Red Hat, Inc.