Linux NFS development
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: Jeff Layton <jlayton@kernel.org>,
	Trond Myklebust <trondmy@kernel.org>,
	Anna Schumaker <anna@kernel.org>, NeilBrown <neil@brown.name>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH v2 2/2] sunrpc: add a Kconfig option to redirect dfprintk() output to trace buffer
Date: Wed, 27 Aug 2025 10:43:53 -0400	[thread overview]
Message-ID: <19c94d41-bf0f-47b2-8bbc-36911f5de656@oracle.com> (raw)
In-Reply-To: <20250822-nfs-testing-v2-2-5f6034b16e46@kernel.org>

On 8/22/25 9:19 AM, Jeff Layton wrote:
> We have a lot of old dprintk() call sites that aren't going anywhere
> anytime soon. At the same time, turning them up is a serious burden on
> the host due to the console locking overhead.
> 
> Add a new Kconfig option that redirects dfprintk() output to the trace
> buffer. This is more efficient than logging to the console and allows
> for proper interleaving of dprintk and static tracepoint events.
> 
> Since using trace_printk() causes scary warnings to pop at boot time,
> this new option defaults to "n".
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  include/linux/sunrpc/debug.h | 10 ++++++++--
>  net/sunrpc/Kconfig           | 14 ++++++++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index 99a6fa4a1d6af0b275546a53957f07c9a509f2ac..891f6173c951a6644018237017c845d81b42aa76 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -30,17 +30,23 @@ extern unsigned int		nlm_debug;
>  #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
>  # define ifdebug(fac)		if (unlikely(rpc_debug & RPCDBG_##fac))
>  
> +# if IS_ENABLED(CONFIG_SUNRPC_DEBUG_TRACE)
> +#  define __sunrpc_printk(fmt, ...)	trace_printk(fmt, ##__VA_ARGS__)
> +# else
> +#  define __sunrpc_printk(fmt, ...)	printk(KERN_DEFAULT fmt, ##__VA_ARGS__)
> +# endif
> +
>  # define dfprintk(fac, fmt, ...)					\
>  do {									\
>  	ifdebug(fac)							\
> -		printk(KERN_DEFAULT fmt, ##__VA_ARGS__);				\
> +		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
>  } while (0)
>  
>  # define dfprintk_rcu(fac, fmt, ...)					\
>  do {									\
>  	ifdebug(fac) {							\
>  		rcu_read_lock();					\
> -		printk(KERN_DEFAULT fmt, ##__VA_ARGS__);				\
> +		__sunrpc_printk(fmt, ##__VA_ARGS__);			\
>  		rcu_read_unlock();					\
>  	}								\
>  } while (0)
> diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig
> index 2d8b67dac7b5b58a8a86c3022dd573746fb22547..a570e7adf270fb8976f751266bbffe39ef696c6a 100644
> --- a/net/sunrpc/Kconfig
> +++ b/net/sunrpc/Kconfig
> @@ -101,6 +101,20 @@ config SUNRPC_DEBUG
>  
>  	  If unsure, say Y.
>  
> +config SUNRPC_DEBUG_TRACE
> +	bool "RPC: Send dfprintk() output to the trace buffer"
> +	depends on SUNRPC_DEBUG && TRACING
> +	default n
> +	help
> +          dprintk() output can be voluminous, which can overwhelm the
> +          kernel's logging facility as it must be sent to the console.
> +          This option causes dprintk() output to go to the trace buffer
> +          instead of the kernel log.
> +
> +          This will cause warnings about trace_printk() being used to be
> +          logged at boot time, so say N unless you are debugging a problem
> +          with sunrpc-based clients or services.
> +
>  config SUNRPC_XPRT_RDMA
>  	tristate "RPC-over-RDMA transport"
>  	depends on SUNRPC && INFINIBAND && INFINIBAND_ADDR_TRANS
> 

Nice and surgical. But I'm on the fence about whether this is a
good long-term strategy. No real objections, though.

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>


-- 
Chuck Lever

      parent reply	other threads:[~2025-08-27 14:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22 13:19 [PATCH v2 0/2] sunrpc: allow dprintk() to go to the trace buffer instead of console Jeff Layton
2025-08-22 13:19 ` [PATCH v2 1/2] sunrpc: remove dfprintk_cont() and dfprintk_rcu_cont() Jeff Layton
2025-08-27 14:23   ` Simon Horman
2025-08-27 14:41   ` Chuck Lever
2025-08-22 13:19 ` [PATCH v2 2/2] sunrpc: add a Kconfig option to redirect dfprintk() output to trace buffer Jeff Layton
2025-08-27 14:24   ` Simon Horman
2025-08-27 14:43   ` Chuck Lever [this message]

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=19c94d41-bf0f-47b2-8bbc-36911f5de656@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=anna@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=netdev@vger.kernel.org \
    --cc=okorniev@redhat.com \
    --cc=pabeni@redhat.com \
    --cc=tom@talpey.com \
    --cc=trondmy@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