Re: [PATCH v4 1/2] sunrpc: fix unused variable warnings by using no_printk
From: Sean Chang
Date: Sun Mar 01 2026 - 10:41:33 EST
On Sun, Mar 1, 2026 at 4:04 AM Andrew Lunn <andrew@xxxxxxx> 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