Message ID | 20240325175248.1499046-13-linux@roeck-us.net (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Add support for suppressing warning backtraces | expand |
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__ ( \ ...
On 3/27/24 07:44, Simon Horman wrote: > 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? > Thanks a lot for the feedback. The definition block needs to be inside CONFIG_DEBUG_BUGVERBOSE, so it would be a bit odd to move it above the documentation just to make kerneldoc happy. I am not really sure that to do about it. I'll wait for comments from others before making any changes. Thanks, Guenter >> #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__ ( \ > > ...
On Wed, Mar 27, 2024 at 08:10:51AM -0700, Guenter Roeck wrote: > On 3/27/24 07:44, Simon Horman wrote: > > 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? > > > > Thanks a lot for the feedback. > > The definition block needs to be inside CONFIG_DEBUG_BUGVERBOSE, > so it would be a bit odd to move it above the documentation > just to make kerneldoc happy. I am not really sure that to do > about it. FWIIW, I agree that would be odd. But perhaps the #ifdef could also move above the Kernel doc? Maybe not a great idea, but the best one I've had so far. > I'll wait for comments from others before making any changes. > > Thanks, > Guenter > > > > #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__ ( \ > > > > ... >
On Wed, Mar 27, 2024 at 07:39:20PM +0000, Simon Horman wrote: [ ... ] > > > > > > 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? > > > > > > > Thanks a lot for the feedback. > > > > The definition block needs to be inside CONFIG_DEBUG_BUGVERBOSE, > > so it would be a bit odd to move it above the documentation > > just to make kerneldoc happy. I am not really sure that to do > > about it. > > FWIIW, I agree that would be odd. > But perhaps the #ifdef could also move above the Kernel doc? > Maybe not a great idea, but the best one I've had so far. > I did that for the next version of the patch series. It is a bit more clumsy, so I left it as separate patch on top of this patch. I'd still like to get input from others before making the change final. Thanks, Guenter
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 */ + #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__ ( \ @@ -47,6 +62,7 @@ do { \ : \ : "n" (TRAPA_BUG_OPCODE), \ "i" (__FILE__), \ + "i" (__BUG_FUNC), \ "i" (__LINE__), "i" (0), \ "i" (sizeof(struct bug_entry))); \ unreachable(); \ @@ -60,6 +76,7 @@ do { \ : \ : "n" (TRAPA_BUG_OPCODE), \ "i" (__FILE__), \ + "i" (__BUG_FUNC), \ "i" (__LINE__), \ "i" (BUGFLAG_WARNING|(flags)), \ "i" (sizeof(struct bug_entry))); \ @@ -85,6 +102,7 @@ do { \ : \ : "n" (TRAPA_BUG_OPCODE), \ "i" (__FILE__), \ + "i" (__BUG_FUNC), \ "i" (__LINE__), \ "i" (BUGFLAG_UNWINDER), \ "i" (sizeof(struct bug_entry))); \