Re: [PATCH v2 12/14] sh: Add support for suppressing warning backtraces

From: Simon Horman <horms@kernel.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: loongarch@lists.linux.dev, linux-doc@vger.kernel.org,
	dri-devel@lists.freedesktop.org,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	linux-kselftest@vger.kernel.org, linux-riscv@lists.infradead.org,
	"David Airlie" <airlied@gmail.com>,
	"Arthur Grillo" <arthurgrillo@riseup.net>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
	"Daniel Diaz" <daniel.diaz@linaro.org>,
	linux-sh@vger.kernel.org,
	"Naresh Kamboju" <naresh.kamboju@linaro.org>,
	"Maíra Canal" <mcanal@igalia.com>,
	"Dan Carpenter" <dan.carpenter@linaro.org>,
	"Linux Kernel Functional Testing" <lkft@linaro.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"David Gow" <davidgow@google.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	linux-arm-kernel@lists.infradead.org, kunit-dev@googlegroups.com,
	linux-parisc@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v2 12/14] sh: Add support for suppressing warning backtraces
Date: Wed, 27 Mar 2024 14:44:31 +0000	[thread overview]
Message-ID: <20240327144431.GL403975@kernel.org> (raw)
In-Reply-To: <20240325175248.1499046-13-linux@roeck-us.net>

On Mon, Mar 25, 2024 at 10:52:46AM -0700, Guenter Roeck wrote:
> Add name of functions triggering warning backtraces to the __bug_table
> object section to enable support for suppressing WARNING backtraces.
> 
> To limit image size impact, the pointer to the function name is only added
> to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
> CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
> parameter is replaced with a (dummy) NULL parameter to avoid an image size
> increase due to unused __func__ entries (this is necessary because __func__
> is not a define but a virtual variable).
> 
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
> 
>  arch/sh/include/asm/bug.h | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
> index 05a485c4fabc..470ce6567d20 100644
> --- a/arch/sh/include/asm/bug.h
> +++ b/arch/sh/include/asm/bug.h
> @@ -24,21 +24,36 @@
>   * The offending file and line are encoded in the __bug_table section.
>   */
>  #ifdef CONFIG_DEBUG_BUGVERBOSE
> +
> +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> +# define HAVE_BUG_FUNCTION
> +# define __BUG_FUNC_PTR	"\t.long %O2\n"
> +#else
> +# define __BUG_FUNC_PTR
> +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> +

Hi Guenter,

a minor nit from my side: this change results in a Kernel doc warning.

     .../bug.h:29: warning: expecting prototype for _EMIT_BUG_ENTRY(). Prototype was for HAVE_BUG_FUNCTION() instead

Perhaps either the new code should be placed above the Kernel doc,
or scripts/kernel-doc should be enhanced?

>  #define _EMIT_BUG_ENTRY				\
>  	"\t.pushsection __bug_table,\"aw\"\n"	\
>  	"2:\t.long 1b, %O1\n"			\
> -	"\t.short %O2, %O3\n"			\
> -	"\t.org 2b+%O4\n"			\
> +	__BUG_FUNC_PTR				\
> +	"\t.short %O3, %O4\n"			\
> +	"\t.org 2b+%O5\n"			\
>  	"\t.popsection\n"
>  #else
>  #define _EMIT_BUG_ENTRY				\
>  	"\t.pushsection __bug_table,\"aw\"\n"	\
>  	"2:\t.long 1b\n"			\
> -	"\t.short %O3\n"			\
> -	"\t.org 2b+%O4\n"			\
> +	"\t.short %O4\n"			\
> +	"\t.org 2b+%O5\n"			\
>  	"\t.popsection\n"
>  #endif
>  
> +#ifdef HAVE_BUG_FUNCTION
> +# define __BUG_FUNC	__func__
> +#else
> +# define __BUG_FUNC	NULL
> +#endif
> +
>  #define BUG()						\
>  do {							\
>  	__asm__ __volatile__ (				\

...