Re: [PATCH 00/14] Add support for suppressing warning backtraces

From: Guenter Roeck
Date: Sat Mar 16 2024 - 12:17:12 EST


On 3/14/24 07:37, Guenter Roeck wrote:
On 3/14/24 06:36, Geert Uytterhoeven wrote:
Hi Günter,

On Tue, Mar 12, 2024 at 6:03 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
Some unit tests intentionally trigger warning backtraces by passing bad
parameters to kernel API functions. Such unit tests typically check the
return value from such calls, not the existence of the warning backtrace.

Such intentionally generated warning backtraces are neither desirable
nor useful for a number of reasons.
- They can result in overlooked real problems.
- A warning that suddenly starts to show up in unit tests needs to be
   investigated and has to be marked to be ignored, for example by
   adjusting filter scripts. Such filters are ad-hoc because there is
   no real standard format for warnings. On top of that, such filter
   scripts would require constant maintenance.

One option to address problem would be to add messages such as "expected
warning backtraces start / end here" to the kernel log.  However, that
would again require filter scripts, it might result in missing real
problematic warning backtraces triggered while the test is running, and
the irrelevant backtrace(s) would still clog the kernel log.

Solve the problem by providing a means to identify and suppress specific
warning backtraces while executing test code. Support suppressing multiple
backtraces while at the same time limiting changes to generic code to the
absolute minimum. Architecture specific changes are kept at minimum by
retaining function names only if both CONFIG_DEBUG_BUGVERBOSE and
CONFIG_KUNIT are enabled.

The first patch of the series introduces the necessary infrastructure.
The second patch introduces support for counting suppressed backtraces.
This capability is used in patch three to implement unit tests.
Patch four documents the new API.
The next two patches add support for suppressing backtraces in drm_rect
and dev_addr_lists unit tests. These patches are intended to serve as
examples for the use of the functionality introduced with this series.
The remaining patches implement the necessary changes for all
architectures with GENERIC_BUG support.

Thanks for your series!

I gave it a try on m68k, just running backtrace-suppression-test,
and that seems to work fine.

Design note:
   Function pointers are only added to the __bug_table section if both
   CONFIG_KUNIT and CONFIG_DEBUG_BUGVERBOSE are enabled to avoid image
   size increases if CONFIG_KUNIT=n. There would be some benefits to
   adding those pointers all the time (reduced complexity, ability to
   display function names in BUG/WARNING messages). That change, if
   desired, can be made later.

Unfortunately this also increases kernel size in the CONFIG_KUNIT=m
case (ca. 80 KiB for atari_defconfig), making it less attractive to have
kunit and all tests enabled as modules in my standard kernel.


Good point. Indeed, it does. I wanted to avoid adding a configuration option,
but maybe I should add it after all. How about something like this ?

+config KUNIT_SUPPRESS_BACKTRACE
+       bool "KUnit - Enable backtrace suppression"
+       default y
+       help
+         Enable backtrace suppression for KUnit. If enabled, backtraces
+         generated intentionally by KUnit tests can be suppressed. Disable
+         to reduce kernel image size if image size is more important than
+         suppression of backtraces generated by KUnit tests.
+


Any more comments / feedback on this ? Otherwise I'll introduce the
above configuration option in v2 of the series.

In this context, any suggestions if it should be enabled or disabled by
default ? I personally think it would be more important to be able to
suppress backtraces, but I understand that others may not be willing to
accept a ~1% image size increase with CONFIG_KUNIT=m unless
KUNIT_SUPPRESS_BACKTRACE is explicitly disabled.

Thanks,
Guenter