Linux-arch Archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Guenter Roeck <linux@roeck-us.net>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-kselftest@vger.kernel.org,
	"David Airlie" <airlied@gmail.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Maíra Canal" <mcanal@igalia.com>,
	"Dan Carpenter" <dan.carpenter@linaro.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Daniel Diaz" <daniel.diaz@linaro.org>,
	"David Gow" <davidgow@google.com>,
	"Arthur Grillo" <arthurgrillo@riseup.net>,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"Naresh Kamboju" <naresh.kamboju@linaro.org>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	dri-devel@lists.freedesktop.org, kunit-dev@googlegroups.com,
	linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org,
	linux-sh@vger.kernel.org, loongarch@lists.linux.dev,
	netdev@lists.linux.dev
Subject: Re: [PATCH 00/14] Add support for suppressing warning backtraces
Date: Mon, 18 Mar 2024 14:24:59 +1100	[thread overview]
Message-ID: <87ttl4z0fo.fsf@mail.lhotse> (raw)
In-Reply-To: <04f34097-7788-490d-a9c2-82b44bf6af44@roeck-us.net>

Guenter Roeck <linux@roeck-us.net> writes:
> 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@roeck-us.net> 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.

Please enable it by default.

There are multiple CI systems that will benefit from it, whereas the
number of users enabling KUNIT in severely spaced constrainted
environments is surely small - perhaps just Geert ;).

cheers

      reply	other threads:[~2024-03-18  3:25 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-12 17:02 [PATCH 00/14] Add support for suppressing warning backtraces Guenter Roeck
2024-03-12 17:02 ` [PATCH 01/14] bug/kunit: Core " Guenter Roeck
2024-03-12 22:00   ` Kees Cook
2024-03-12 17:02 ` [PATCH 02/14] kunit: bug: Count suppressed " Guenter Roeck
2024-03-12 22:02   ` Kees Cook
2024-03-12 17:02 ` [PATCH 03/14] kunit: Add test cases for backtrace warning suppression Guenter Roeck
2024-03-12 22:02   ` Kees Cook
2024-03-12 17:02 ` [PATCH 04/14] kunit: Add documentation for warning backtrace suppression API Guenter Roeck
2024-03-12 22:03   ` Kees Cook
2024-03-12 17:03 ` [PATCH 05/14] drm: Suppress intentional warning backtraces in scaling unit tests Guenter Roeck
2024-03-12 17:03 ` [PATCH 06/14] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests Guenter Roeck
2024-03-12 17:03 ` [PATCH 07/14] x86: Add support for suppressing warning backtraces Guenter Roeck
2024-03-12 17:03 ` [PATCH 08/14] arm64: " Guenter Roeck
2024-03-12 17:03 ` [PATCH 09/14] loongarch: " Guenter Roeck
2024-03-12 17:03 ` [PATCH 10/14] parisc: " Guenter Roeck
2024-03-15 11:45   ` Helge Deller
2024-03-12 17:03 ` [PATCH 11/14] s390: " Guenter Roeck
2024-03-14  7:57   ` Geert Uytterhoeven
2024-03-14 13:54     ` Guenter Roeck
2024-03-12 17:03 ` [PATCH 12/14] sh: " Guenter Roeck
2024-03-12 17:03 ` [PATCH 13/14] riscv: " Guenter Roeck
2024-03-12 17:03 ` [PATCH 14/14] powerpc: " Guenter Roeck
2024-03-13  7:39 ` [PATCH 00/14] " Dan Carpenter
2024-03-14  7:19 ` Naresh Kamboju
2024-03-14 13:36 ` Geert Uytterhoeven
2024-03-14 14:37   ` Guenter Roeck
2024-03-14 15:02     ` Maxime Ripard
2024-03-14 15:29       ` Guenter Roeck
2024-03-16 16:16     ` Guenter Roeck
2024-03-18  3:24       ` Michael Ellerman [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=87ttl4z0fo.fsf@mail.lhotse \
    --to=mpe@ellerman.id.au \
    --cc=airlied@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=arthurgrillo@riseup.net \
    --cc=brendan.higgins@linux.dev \
    --cc=dan.carpenter@linaro.org \
    --cc=daniel.diaz@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=davidgow@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert@linux-m68k.org \
    --cc=keescook@chromium.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=loongarch@lists.linux.dev \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mcanal@igalia.com \
    --cc=mripard@kernel.org \
    --cc=naresh.kamboju@linaro.org \
    --cc=netdev@lists.linux.dev \
    --cc=tzimmermann@suse.de \
    --cc=ville.syrjala@linux.intel.com \
    /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).