From: Sean Chang <seanwascoding@gmail.com>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
David Laight <david.laight.linux@gmail.com>,
Anna Schumaker <anna@kernel.org>,
Andy Shevchenko <andriy.shevchenko@intel.com>,
netdev@vger.kernel.org, linux-nfs@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/5] sunrpc: Fix dprintk type mismatch using do-while(0)
Date: Wed, 25 Mar 2026 23:49:47 +0800 [thread overview]
Message-ID: <CAAb=EJV9-v4OsYeaQQH2ycDUCSx3VP2M2ji16xAzO3gT9OuzpA@mail.gmail.com> (raw)
In-Reply-To: <e3702d11-2157-46bb-b6aa-0ab60b51eecf@oracle.com>
On Sun, Mar 22, 2026 at 12:38 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On 3/21/26 10:15 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.
>
> Generally we prefer a commit message to open with an explanation of why
> the change is needed. Your first paragraph instead opens with what was
> done ("Following David Laight's suggestion, simplify ...") rather than
> why the change is necessary. The Sparse warning motivation is buried in
> the second paragraph. Consider leading with a problem statement in this
> and subsequent patches in this series.
>
>
> > To resolve a Sparse warning (void vs int mismatch) when dfprintk is used in
> > conditional statements, wrap the non-debug definition in a do-while(0) block.
> > This ensures the macro always evaluates to a void expression.
>
> The non-debug definitions in the diff below are:
>
> > -# 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__)
>
> These are not wrapped in do { ... } while (0), and no_printk()
> evaluates to int (0), not void. The do-while(0) wrapping that
> was discussed on the list and fixes the Sparse warning appears
> to be in a later patch in this series (the nfs_errorf
> refactoring), not in this one.
>
> Should the commit message second paragraph be removed or revised
> to reflect what this patch actually does?
>
Hi Chuck,
I notice that the commit messages are wrong because the current situation
is not the same as when I first sent it, so I will fix this message to the
correct one.
It may be like:
When RPC debugging is enabled, the dfprintk macros include redundant
else-branches that call no_printk(). Since no_printk() is a no-op
designed specifically for compile-time type checking, it is unnecessary
to invoke it explicitly when the ifdebug(fac) condition is not met
within a debug-enabled build.
Drop the explicit 'fmt' argument to allow the compiler to handle all
arguments directly through __VA_ARGS__. Since no_printk()
internally performs the same format string validation, passing the
entire argument list is more efficient and reduces macro complexity.
Best Regards,
Sean
next prev parent reply other threads:[~2026-03-25 15:50 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-21 14:15 [PATCH v5 0/5] sunrpc/nfs: cleanup redundant debug checks and refactor macros Sean Chang
2026-03-21 14:15 ` [PATCH v5 1/5] sunrpc: Fix dprintk type mismatch using do-while(0) Sean Chang
2026-03-21 16:37 ` Chuck Lever
2026-03-25 15:49 ` Sean Chang [this message]
2026-03-21 14:15 ` [PATCH v5 2/5] nfsd/lockd: Remove redundant debug checks Sean Chang
2026-03-21 14:15 ` [PATCH v5 3/5] svcrdma: Remove redundant IS_ENABLED(CONFIG_SUNRPC_DEBUG) guards Sean Chang
2026-03-21 14:15 ` [PATCH v5 4/5] nfs: Refactor nfs_errorf macros and remove unused ones Sean Chang
2026-03-21 16:38 ` Chuck Lever
2026-03-25 16:11 ` Sean Chang
2026-03-21 14:15 ` [PATCH v5 5/5] Revert "nfsd: Mark variable __maybe_unused to avoid W=1 build break" Sean Chang
2026-03-21 16:38 ` Chuck Lever
2026-03-25 16:39 ` Sean Chang
2026-03-23 13:25 ` [PATCH v5 0/5] sunrpc/nfs: cleanup redundant debug checks and refactor macros Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAb=EJV9-v4OsYeaQQH2ycDUCSx3VP2M2ji16xAzO3gT9OuzpA@mail.gmail.com' \
--to=seanwascoding@gmail.com \
--cc=andrew@lunn.ch \
--cc=andriy.shevchenko@intel.com \
--cc=anna@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=david.laight.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).