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.
With CONFIG_KUNIT enabled, image size increase with this series applied is
approximately 1%. The image size increase (and with it the functionality
introduced by this series) can be avoided by disabling
CONFIG_KUNIT_SUPPRESS_BACKTRACE.
This series is based on the RFC patch and subsequent discussion at
https://patchwork.kernel.org/project/linux-kselftest/patch/02546e59-1afe-4b08-ba81-d94f3b691c9a@moroto.mountain/
and offers a more comprehensive solution of the problem discussed there.
Design note:
Function pointers are only added to the __bug_table section if both
CONFIG_KUNIT_SUPPRESS_BACKTRACE and CONFIG_DEBUG_BUGVERBOSE are enabled
to avoid image size increases if CONFIG_KUNIT is disabled. 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.
Checkpatch note:
Remaining checkpatch errors and warnings were deliberately ignored.
Some are triggered by matching coding style or by comments interpreted
as code, others by assembler macros which are disliked by checkpatch.
Suggestions for improvements are welcome.
Changes since RFC:
- Introduced CONFIG_KUNIT_SUPPRESS_BACKTRACE
- Minor cleanups and bug fixes
- Added support for all affected architectures
- Added support for counting suppressed warnings
- Added unit tests using those counters
- Added patch to suppress warning backtraces in dev_addr_lists tests
Changes since v1:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
[I retained those tags since there have been no functional changes]
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option, enabled by
default.
Some unit tests intentionally trigger warning backtraces by passing
bad parameters to API functions. Such unit tests typically check the
return value from those 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. Since the new functionality
results in an image size increase of about 1% if CONFIG_KUNIT is enabled,
provide configuration option KUNIT_SUPPRESS_BACKTRACE to be able to disable
the new functionality. This option is by default enabled since almost all
systems with CONFIG_KUNIT enabled will want to benefit from it.
Cc: Dan Carpenter <redacted>
Cc: Daniel Diaz <redacted>
Cc: Naresh Kamboju <redacted>
Cc: Kees Cook <redacted>
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Reviewed-by: Kees Cook <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Added CONFIG_KUNIT_SUPPRESS_BACKTRACE configuration option,
enabled by default
include/asm-generic/bug.h | 16 +++++++++---
include/kunit/bug.h | 51 +++++++++++++++++++++++++++++++++++++++
include/kunit/test.h | 1 +
include/linux/bug.h | 13 ++++++++++
lib/bug.c | 51 ++++++++++++++++++++++++++++++++++++---
lib/kunit/Kconfig | 9 +++++++
lib/kunit/Makefile | 6 +++--
lib/kunit/bug.c | 40 ++++++++++++++++++++++++++++++
8 files changed, 178 insertions(+), 9 deletions(-)
create mode 100644 include/kunit/bug.h
create mode 100644 lib/kunit/bug.c
@@ -14,8 +14,10 @@ ifeq ($(CONFIG_KUNIT_DEBUGFS),y)kunit-objs+=debugfs.oendif-# KUnit 'hooks' are built-in even when KUnit is built as a module.-obj-y+=hooks.o+# KUnit 'hooks' and bug handling are built-in even when KUnit is built+# as a module.+obj-y+=hooks.o\+bug.oobj-$(CONFIG_KUNIT_TEST)+=kunit-test.o
Count suppressed warning backtraces to enable code which suppresses
warning backtraces to check if the expected backtrace(s) have been
observed.
Using atomics for the backtrace count resulted in build errors on some
architectures due to include file recursion, so use a plain integer
for now.
Acked-by: Dan Carpenter <redacted>
Reviewed-by: Kees Cook <redacted>
Tested-by: Linux Kernel Functional Testing <redacted>
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
include/kunit/bug.h | 7 ++++++-
lib/kunit/bug.c | 4 +++-
2 files changed, 9 insertions(+), 2 deletions(-)
Add unit tests to verify that warning backtrace suppression works.
If backtrace suppression does _not_ work, the unit tests will likely
trigger unsuppressed backtraces, which should actually help to get
the affected architectures / platforms fixed.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Reviewed-by: Kees Cook <redacted>
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
lib/kunit/Makefile | 7 +-
lib/kunit/backtrace-suppression-test.c | 104 +++++++++++++++++++++++++
2 files changed, 109 insertions(+), 2 deletions(-)
create mode 100644 lib/kunit/backtrace-suppression-test.c
@@ -16,10 +16,13 @@ endif# KUnit 'hooks' and bug handling are built-in even when KUnit is built# as a module.-obj-y+=hooks.o\-bug.o+obj-y+=hooks.o+obj-$(CONFIG_KUNIT_SUPPRESS_BACKTRACE)+=bug.oobj-$(CONFIG_KUNIT_TEST)+=kunit-test.o+ifeq ($(CCONFIG_KUNIT_SUPPRESS_BACKTRACE),y)+obj-$(CONFIG_KUNIT_TEST)+=backtrace-suppression-test.o+endif# string-stream-test compiles built-in only.ifeq ($(CONFIG_KUNIT_TEST),y)
@@ -0,0 +1,104 @@+// SPDX-License-Identifier: GPL-2.0+/*+*KUnittestforsuppressingwarningtracebacks+*+*Copyright(C)2024,GuenterRoeck+*Author:GuenterRoeck<linux@roeck-us.net>+*/++#include<kunit/test.h>+#include<linux/bug.h>++staticvoidbacktrace_suppression_test_warn_direct(structkunit*test)+{+DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);++START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);+WARN(1,"This backtrace should be suppressed");+END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);++KUNIT_EXPECT_EQ(test,SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_direct),1);+}++staticvoidtrigger_backtrace_warn(void)+{+WARN(1,"This backtrace should be suppressed");+}++staticvoidbacktrace_suppression_test_warn_indirect(structkunit*test)+{+DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);++START_SUPPRESSED_WARNING(trigger_backtrace_warn);+trigger_backtrace_warn();+END_SUPPRESSED_WARNING(trigger_backtrace_warn);++KUNIT_EXPECT_EQ(test,SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn),1);+}++staticvoidbacktrace_suppression_test_warn_multi(structkunit*test)+{+DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);+DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);++START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);+START_SUPPRESSED_WARNING(trigger_backtrace_warn);+WARN(1,"This backtrace should be suppressed");+trigger_backtrace_warn();+END_SUPPRESSED_WARNING(trigger_backtrace_warn);+END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);++KUNIT_EXPECT_EQ(test,SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_multi),1);+KUNIT_EXPECT_EQ(test,SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn),1);+}++staticvoidbacktrace_suppression_test_warn_on_direct(structkunit*test)+{+DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);++if(!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE)&&!IS_ENABLED(CONFIG_KALLSYMS))+kunit_skip(test,"requires CONFIG_DEBUG_BUGVERBOSE or CONFIG_KALLSYMS");++START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);+WARN_ON(1);+END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);++KUNIT_EXPECT_EQ(test,+SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_on_direct),1);+}++staticvoidtrigger_backtrace_warn_on(void)+{+WARN_ON(1);+}++staticvoidbacktrace_suppression_test_warn_on_indirect(structkunit*test)+{+DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn_on);++if(!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE))+kunit_skip(test,"requires CONFIG_DEBUG_BUGVERBOSE");++START_SUPPRESSED_WARNING(trigger_backtrace_warn_on);+trigger_backtrace_warn_on();+END_SUPPRESSED_WARNING(trigger_backtrace_warn_on);++KUNIT_EXPECT_EQ(test,SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn_on),1);+}++staticstructkunit_casebacktrace_suppression_test_cases[]={+KUNIT_CASE(backtrace_suppression_test_warn_direct),+KUNIT_CASE(backtrace_suppression_test_warn_indirect),+KUNIT_CASE(backtrace_suppression_test_warn_multi),+KUNIT_CASE(backtrace_suppression_test_warn_on_direct),+KUNIT_CASE(backtrace_suppression_test_warn_on_indirect),+{}+};++staticstructkunit_suitebacktrace_suppression_test_suite={+.name="backtrace-suppression-test",+.test_cases=backtrace_suppression_test_cases,+};+kunit_test_suites(&backtrace_suppression_test_suite);++MODULE_LICENSE("GPL");
@@ -157,6 +157,34 @@ Alternatively, one can take full control over the error message by using if (some_setup_function()) KUNIT_FAIL(test, "Failed to setup thing for testing");+Suppressing warning backtraces+------------------------------++Some unit tests trigger warning backtraces either intentionally or as side+effect. Such backtraces are normally undesirable since they distract from+the actual test and may result in the impression that there is a problem.++Such backtraces can be suppressed. To suppress a backtrace in some_function(),+use the following code.++..code-block:: c++ static void some_test(struct kunit *test)+ {+ DEFINE_SUPPRESSED_WARNING(some_function);++ START_SUPPRESSED_WARNING(some_function);+ trigger_backtrace();+ END_SUPPRESSED_WARNING(some_function);+ }++SUPPRESSED_WARNING_COUNT() returns the number of suppressed backtraces. If the+suppressed backtrace was triggered on purpose, this can be used to check if+the backtrace was actually triggered.++..code-block:: c++ KUNIT_EXPECT_EQ(test, SUPPRESSED_WARNING_COUNT(some_function), 1); Test Suites ~~~~~~~~~~~
@@ -857,4 +885,4 @@ For example: dev_managed_string = devm_kstrdup(fake_device, "Hello, World!"); // Everything is cleaned up automatically when the test ends.- }
The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
intentionally trigger warning backtraces by providing bad parameters to
the tested functions. What is tested is the return value, not the existence
of a warning backtrace. Suppress the backtraces to avoid clogging the
kernel log.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
drivers/gpu/drm/tests/drm_rect_test.c | 6 ++++++
1 file changed, 6 insertions(+)
dev_addr_lists_test generates lock warning noise at the end of tests
if lock debugging is enabled. There are two sets of warnings.
WARNING: CPU: 0 PID: 689 at kernel/locking/mutex.c:923 __mutex_unlock_slowpath.constprop.0+0x13c/0x368
DEBUG_LOCKS_WARN_ON(__owner_task(owner) != __get_current())
WARNING: kunit_try_catch/1336 still has locks held!
KUnit test cleanup is not guaranteed to run in the same thread as the test
itself. For this test, this means that rtnl_lock() and rtnl_unlock() may
be called from different threads. This triggers the warnings.
Suppress the warnings because they are irrelevant for the test and just
confusing.
The first warning can be suppressed by using START_SUPPRESSED_WARNING()
and END_SUPPRESSED_WARNING() around the call to rtnl_unlock(). To suppress
the second warning, it is necessary to set debug_locks_silent while the
rtnl lock is held.
Tested-by: Linux Kernel Functional Testing <redacted>
Cc: David Gow <redacted>
Cc: Jakub Kicinski <kuba@kernel.org>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
net/core/dev_addr_lists_test.c | 6 ++++++
1 file changed, 6 insertions(+)
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 <redacted>
Acked-by: Dan Carpenter <redacted>
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/x86/include/asm/bug.h | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
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 <redacted>
Acked-by: Dan Carpenter <redacted>
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/arm64/include/asm/asm-bug.h | 29 +++++++++++++++++++----------
arch/arm64/include/asm/bug.h | 8 +++++++-
2 files changed, 26 insertions(+), 11 deletions(-)
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 <redacted>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1; resolved context conflict
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
arch/loongarch/include/asm/bug.h | 38 +++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 11 deletions(-)
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).
While at it, declare assembler parameters as constants where possible.
Refine .blockz instructions to calculate the necessary padding instead
of using fixed values.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Acked-by: Helge Deller <deller@gmx.de>
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/parisc/include/asm/bug.h | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
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 <redacted>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1 (simplified assembler changes after upstream commit
3938490e78f4 ("s390/bug: remove entry size from __bug_table section")
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
arch/s390/include/asm/bug.h | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
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 <redacted>
Acked-by: Dan Carpenter <redacted>
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(-)
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).
To simplify the implementation, unify the __BUG_ENTRY_ADDR and
__BUG_ENTRY_FILE macros into a single macro named __BUG_REL() which takes
the address, file, or function reference as parameter.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
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/riscv/include/asm/bug.h | 38 ++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
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 <redacted>
Acked-by: Dan Carpenter <redacted>
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/powerpc/include/asm/bug.h | 37 +++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
Hi Guenter,
On 3/25/24 14:52, Guenter Roeck wrote:
quoted hunk
The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
intentionally trigger warning backtraces by providing bad parameters to
the tested functions. What is tested is the return value, not the existence
of a warning backtrace. Suppress the backtraces to avoid clogging the
kernel log.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
drivers/gpu/drm/tests/drm_rect_test.c | 6 ++++++
1 file changed, 6 insertions(+)
I'm not sure if it is not that obvious only to me, but it would be nice
to have a comment here, remembering that we provide bad parameters in
some test cases.
Best Regards,
- Maíra
Hi,
On Mon, Mar 25, 2024 at 04:05:06PM -0300, Maíra Canal wrote:
Hi Guenter,
On 3/25/24 14:52, Guenter Roeck wrote:
quoted
The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
intentionally trigger warning backtraces by providing bad parameters to
the tested functions. What is tested is the return value, not the existence
of a warning backtrace. Suppress the backtraces to avoid clogging the
kernel log.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
drivers/gpu/drm/tests/drm_rect_test.c | 6 ++++++
1 file changed, 6 insertions(+)
I'm not sure if it is not that obvious only to me, but it would be nice
to have a comment here, remembering that we provide bad parameters in
some test cases.
Sure. Something like this ?
/*
* drm_rect_calc_hscale() generates a warning backtrace whenever bad
* parameters are passed to it. This affects all unit tests with an
* error code in expected_scaling_factor.
*/
Thanks,
Guenter
Hi,
On Mon, Mar 25, 2024 at 04:05:06PM -0300, Maíra Canal wrote:
quoted
Hi Guenter,
On 3/25/24 14:52, Guenter Roeck wrote:
quoted
The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
intentionally trigger warning backtraces by providing bad parameters to
the tested functions. What is tested is the return value, not the existence
of a warning backtrace. Suppress the backtraces to avoid clogging the
kernel log.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
drivers/gpu/drm/tests/drm_rect_test.c | 6 ++++++
1 file changed, 6 insertions(+)
I'm not sure if it is not that obvious only to me, but it would be nice
to have a comment here, remembering that we provide bad parameters in
some test cases.
Sure. Something like this ?
/*
* drm_rect_calc_hscale() generates a warning backtrace whenever bad
* parameters are passed to it. This affects all unit tests with an
* error code in expected_scaling_factor.
*/
Yeah, perfect. With that, feel free to add my
Acked-by: Maíra Canal <mcanal@igalia.com>
Best Regards,
- Maíra
Hi,
On Mon, Mar 25, 2024 at 04:05:06PM -0300, Maíra Canal wrote:
quoted
Hi Guenter,
On 3/25/24 14:52, Guenter Roeck wrote:
quoted
The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
intentionally trigger warning backtraces by providing bad parameters to
the tested functions. What is tested is the return value, not the existence
of a warning backtrace. Suppress the backtraces to avoid clogging the
kernel log.
Tested-by: Linux Kernel Functional Testing <redacted>
Acked-by: Dan Carpenter <redacted>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
drivers/gpu/drm/tests/drm_rect_test.c | 6 ++++++
1 file changed, 6 insertions(+)
I'm not sure if it is not that obvious only to me, but it would be nice
to have a comment here, remembering that we provide bad parameters in
some test cases.
Sure. Something like this ?
/*
* drm_rect_calc_hscale() generates a warning backtrace whenever bad
* parameters are passed to it. This affects all unit tests with an
* error code in expected_scaling_factor.
*/
Yeah, perfect. With that, feel free to add my
Acked-by: Maíra Canal <mcanal@igalia.com>
From: Simon Horman <horms@kernel.org> Date: 2024-03-27 14:44:39
On Mon, Mar 25, 2024 at 10:52:46AM -0700, Guenter Roeck wrote:
quoted hunk
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 <redacted>
Acked-by: Dan Carpenter <redacted>
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(-)
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?
On Mon, Mar 25, 2024 at 10:52:46AM -0700, Guenter Roeck wrote:
quoted
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 <redacted>
Acked-by: Dan Carpenter <redacted>
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(-)
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
From: Simon Horman <horms@kernel.org> Date: 2024-03-27 19:39:28
On Wed, Mar 27, 2024 at 08:10:51AM -0700, Guenter Roeck wrote:
On 3/27/24 07:44, Simon Horman wrote:
quoted
On Mon, Mar 25, 2024 at 10:52:46AM -0700, Guenter Roeck wrote:
quoted
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 <redacted>
Acked-by: Dan Carpenter <redacted>
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(-)
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
On Wed, Mar 27, 2024 at 07:39:20PM +0000, Simon Horman wrote:
[ ... ]
quoted
quoted
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