[PATCH v3] drm/nouveau/nvif: Avoid build error due to potential integer overflows
Trying to build parisc:allmodconfig with gcc 12.x or later results
in the following build error.
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd':
drivers/gpu/drm/nouveau/nvif/object.c:161:9: error:
'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32
overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict]
161 | memcpy(data, args->mthd.data, size);
| ^~~
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor':
drivers/gpu/drm/nouveau/nvif/object.c:298:17: error:
'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56
overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict]
298 | memcpy(data, args->new.data, size);
gcc assumes that 'sizeof(*args) + size' can overflow, which would result
in the problem.
The problem is not new, only it is now no longer a warning but an error
since W=1 has been enabled for the drm subsystem and since Werror is
enabled for test builds.
Rearrange arithmetic and use check_add_overflow() for validating the
allocation size to avoid the overflow. While at it, split assignments
out of if conditions.
Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the
subsystem")
Cc: Javier Martinez Canillas
Cc: Jani Nikula
Cc: Thomas Zimmermann
Cc: Danilo Krummrich
Cc: Maxime Ripard
Cc: Kees Cook
Cc: Christophe JAILLET
Cc: Joe Perches
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
v3: Split assignments from if conditions.
v2: Use check_add_overflow() to calculate the allocation size and to check
for overflows.
drivers/gpu/drm/nouveau/nvif/object.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
b/drivers/gpu/drm/nouveau/nvif/object.c
index 4d1aaee8fe15..1d19c87eaec1 100644
--- a/drivers/gpu/drm/nouveau/nvif/object.c
+++ b/drivers/gpu/drm/nouveau/nvif/object.c
@@ -142,11 +142,16 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd,
void *data, u32 size)
struct nvif_ioctl_v0 ioctl;
struct nvif_ioctl_mthd_v0 mthd;
} *args;
+ u32 args_size;
u8 stack[128];
int ret;
- if (sizeof(*args) + size > sizeof(stack)) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
+ if (check_add_overflow(sizeof(*args), size, _size))
+ return -ENOMEM;
+
+ if (args_size > sizeof(stack)) {
+ args = kmalloc(args_size, GFP_KERNEL);
+ if (!args)
return -ENOMEM;
} else {
args = (void *)stack;
@@ -157,7 +162,7 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd, void
*data, u32 size)
args->mthd.method = mthd;
memcpy(args->mthd.data, data, size);
- ret = nvif_object_ioctl(object, args, sizeof(*args) + size, NULL);
+ ret = nvif_object_ioctl(object, args, args_size, NULL);
memcpy(data, args->mthd.data, size);
if (args != (void *)stack)
kfree(args);
@@ -276,7 +281,15 @@ nvif_object_ctor(struct nvif_object *parent, const char
*name, u32 handle,
object->map.size = 0;
if (parent) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) {
+ u32 args_size;
+
+ if (check_add_overflow(sizeof(*args), size, _size)) {
+ nvif_object_dtor(object);
+ return -ENOMEM;
+ }
+
+ args = kmalloc(args_size, GFP_KERNEL);
+ if (!args) {
nvif_object_dtor(object);
return -ENOMEM;
}
@@ -293,8 +306,7 @@ nvif_object_ctor(struct nvif_object *parent, const char
*name, u32 handle,
args->new.oclass = oclass;
memcpy(args->new.data, data, size);
- ret = nvif_object_ioctl(parent, args, sizeof(*args) + size,
- >priv);
+ ret = nvif_object_ioctl(parent, args, args_size, >priv);
memcpy(data, args->new.data, size);
kfree(args);
if (ret == 0)
--
2.39.2
Re: [PATCH] drm/nouveau/nvif: Avoid build error due to potential integer overflows
On 5/18/24 18:19, Joe Perches wrote:
On Sat, 2024-05-18 at 11:23 -0700, Guenter Roeck wrote:
On 5/18/24 10:32, Kees Cook wrote:
[]
I think the INT_MAX test is actually better in this case because
nvif_object_ioctl()'s size argument is u32:
ret = nvif_object_ioctl(object, args, sizeof(*args) + size, NULL);
So that could wrap around, even though the allocation may not.
Better yet, since "sizeof(*args) + size" is repeated 3 times in the
function, I'd recommend:
...
u32 args_size;
if (check_add_overflow(sizeof(*args), size, _size))
return -ENOMEM;
if (args_size > sizeof(stack)) {
if (!(args = kmalloc(args_size, GFP_KERNEL)))
trivia:
More typical kernel style would use separate alloc and test
args = kmalloc(args_size, GFP_KERNEL);
if (!args)
Sure, I can do that as well. I'll wait a couple of days though before
sending v3 in case there are more change requests.
Guenter
[PATCH v2] drm/nouveau/nvif: Avoid build error due to potential integer overflows
Trying to build parisc:allmodconfig with gcc 12.x or later results
in the following build error.
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd':
drivers/gpu/drm/nouveau/nvif/object.c:161:9: error:
'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32
overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict]
161 | memcpy(data, args->mthd.data, size);
| ^~~
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor':
drivers/gpu/drm/nouveau/nvif/object.c:298:17: error:
'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56
overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict]
298 | memcpy(data, args->new.data, size);
gcc assumes that 'sizeof(*args) + size' can overflow, which would result
in the problem.
The problem is not new, only it is now no longer a warning but an error
since W=1 has been enabled for the drm subsystem and since Werror is
enabled for test builds.
Rearrange arithmetic and use check_add_overflow() for validating the
allocation size to avoid the overflow.
Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the
subsystem")
Cc: Javier Martinez Canillas
Cc: Jani Nikula
Cc: Thomas Zimmermann
Cc: Danilo Krummrich
Cc: Maxime Ripard
Cc: Kees Cook
Cc: Christophe JAILLET
Signed-off-by: Guenter Roeck
---
v2: Use check_add_overflow() to calculate the allocation size and to check
for overflows.
drivers/gpu/drm/nouveau/nvif/object.c | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
b/drivers/gpu/drm/nouveau/nvif/object.c
index 4d1aaee8fe15..89a812f812af 100644
--- a/drivers/gpu/drm/nouveau/nvif/object.c
+++ b/drivers/gpu/drm/nouveau/nvif/object.c
@@ -142,11 +142,15 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd,
void *data, u32 size)
struct nvif_ioctl_v0 ioctl;
struct nvif_ioctl_mthd_v0 mthd;
} *args;
+ u32 args_size;
u8 stack[128];
int ret;
- if (sizeof(*args) + size > sizeof(stack)) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
+ if (check_add_overflow(sizeof(*args), size, _size))
+ return -ENOMEM;
+
+ if (args_size > sizeof(stack)) {
+ if (!(args = kmalloc(args_size, GFP_KERNEL)))
return -ENOMEM;
} else {
args = (void *)stack;
@@ -157,7 +161,7 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd, void
*data, u32 size)
args->mthd.method = mthd;
memcpy(args->mthd.data, data, size);
- ret = nvif_object_ioctl(object, args, sizeof(*args) + size, NULL);
+ ret = nvif_object_ioctl(object, args, args_size, NULL);
memcpy(data, args->mthd.data, size);
if (args != (void *)stack)
kfree(args);
@@ -276,7 +280,14 @@ nvif_object_ctor(struct nvif_object *parent, const char
*name, u32 handle,
object->map.size = 0;
if (parent) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) {
+ u32 args_size;
+
+ if (check_add_overflow(sizeof(*args), size, _size)) {
+ nvif_object_dtor(object);
+ return -ENOMEM;
+ }
+
+ if (!(args = kmalloc(args_size, GFP_KERNEL))) {
nvif_object_dtor(object);
return -ENOMEM;
}
@@ -293,8 +304,7 @@ nvif_object_ctor(struct nvif_object *parent, const char
*name, u32 handle,
args->new.oclass = oclass;
memcpy(args->new.data, data, size);
- ret = nvif_object_ioctl(parent, args, sizeof(*args) + size,
- >priv);
+ ret = nvif_object_ioctl(parent, args, args_size, >priv);
memcpy(data, args->new.data, size);
kfree(args);
if (ret == 0)
--
2.39.2
Re: [PATCH] drm/nouveau/nvif: Avoid build error due to potential integer overflows
On 5/18/24 10:32, Kees Cook wrote:
On Sat, May 18, 2024 at 06:54:36PM +0200, Christophe JAILLET wrote:
(adding linux-harden...@vger.kernel.org)
Le 18/05/2024 à 16:37, Guenter Roeck a écrit :
Trying to build parisc:allmodconfig with gcc 12.x or later results
in the following build error.
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd':
drivers/gpu/drm/nouveau/nvif/object.c:161:9: error:
'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32
overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict]
161 | memcpy(data, args->mthd.data, size);
| ^~~
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor':
drivers/gpu/drm/nouveau/nvif/object.c:298:17: error:
'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56
overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict]
298 | memcpy(data, args->new.data, size);
gcc assumes that 'sizeof(*args) + size' can overflow, which would result
in the problem.
The problem is not new, only it is now no longer a warning but an error since
W=1
has been enabled for the drm subsystem and since Werror is enabled for test
builds.
Rearrange arithmetic and add extra size checks to avoid the overflow.
Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the
subsystem")
Cc: Javier Martinez Canillas
Cc: Jani Nikula
Cc: Thomas Zimmermann
Cc: Danilo Krummrich
Cc: Maxime Ripard
Signed-off-by: Guenter Roeck
---
checkpatch complains about the line length in the description and the
(pre-existing)
assignlemts in if conditions, but I did not want to split lines in the
description
or rearrange the code further.
I don't know why I only see the problem with parisc builds (at least so far).
drivers/gpu/drm/nouveau/nvif/object.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
b/drivers/gpu/drm/nouveau/nvif/object.c
index 4d1aaee8fe15..baf623a48874 100644
--- a/drivers/gpu/drm/nouveau/nvif/object.c
+++ b/drivers/gpu/drm/nouveau/nvif/object.c
@@ -145,8 +145,9 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd, void
*data, u32 size)
u8 stack[128];
int ret;
- if (sizeof(*args) + size > sizeof(stack)) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
+ if (size > sizeof(stack) - sizeof(*args)) {
+ if (size > INT_MAX ||
+ !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
Hi,
Would it be cleaner or better to use size_add(sizeof(*args), size)?
I think the INT_MAX test is actually better in this case because
nvif_object_ioctl()'s size argument is u32:
ret = nvif_object_ioctl(object, args, sizeof(*args) + size, NULL);
So that could wrap around, even though the allocation may not.
Better yet, since "sizeof(*args) + size" is repeated 3 times in the
function, I'd recommend:
...
u32 args_size;
if (check_add_overflow(sizeof(*args), size, _size))
return -ENOMEM;
if (args_size > sizeof(stack)) {
if (!(args = kmalloc(args_size, GFP_KERNEL)))
return -ENOMEM;
} else {
args = (void *)stack;
}
...
ret = nvif_object_ioctl(object, args, args_size, NULL);
This will catch the u32 overflow to nvif_object_ioctl(), catch an
allocation underflow on 32-bits systems, and make the code more
readable. :)
Makes sense. I'll change that and send v2.
Thanks,
Guenter
[PATCH] drm/nouveau/nvif: Avoid build error due to potential integer overflows
Trying to build parisc:allmodconfig with gcc 12.x or later results
in the following build error.
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd':
drivers/gpu/drm/nouveau/nvif/object.c:161:9: error:
'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32
overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict]
161 | memcpy(data, args->mthd.data, size);
| ^~~
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor':
drivers/gpu/drm/nouveau/nvif/object.c:298:17: error:
'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56
overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict]
298 | memcpy(data, args->new.data, size);
gcc assumes that 'sizeof(*args) + size' can overflow, which would result
in the problem.
The problem is not new, only it is now no longer a warning but an error since
W=1
has been enabled for the drm subsystem and since Werror is enabled for test
builds.
Rearrange arithmetic and add extra size checks to avoid the overflow.
Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the
subsystem")
Cc: Javier Martinez Canillas
Cc: Jani Nikula
Cc: Thomas Zimmermann
Cc: Danilo Krummrich
Cc: Maxime Ripard
Signed-off-by: Guenter Roeck
---
checkpatch complains about the line length in the description and the
(pre-existing)
assignlemts in if conditions, but I did not want to split lines in the
description
or rearrange the code further.
I don't know why I only see the problem with parisc builds (at least so far).
drivers/gpu/drm/nouveau/nvif/object.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
b/drivers/gpu/drm/nouveau/nvif/object.c
index 4d1aaee8fe15..baf623a48874 100644
--- a/drivers/gpu/drm/nouveau/nvif/object.c
+++ b/drivers/gpu/drm/nouveau/nvif/object.c
@@ -145,8 +145,9 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd, void
*data, u32 size)
u8 stack[128];
int ret;
- if (sizeof(*args) + size > sizeof(stack)) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
+ if (size > sizeof(stack) - sizeof(*args)) {
+ if (size > INT_MAX ||
+ !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
return -ENOMEM;
} else {
args = (void *)stack;
@@ -276,7 +277,8 @@ nvif_object_ctor(struct nvif_object *parent, const char
*name, u32 handle,
object->map.size = 0;
if (parent) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) {
+ if (size > INT_MAX ||
+ !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) {
nvif_object_dtor(object);
return -ENOMEM;
}
--
2.39.2
Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()
On 5/17/24 11:00, Guenter Roeck wrote:
On 5/17/24 10:48, Steven Rostedt wrote:
On Fri, 17 May 2024 10:36:37 -0700
Guenter Roeck wrote:
Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
from include/trace/define_trace.h:102,
from drivers/cxl/core/trace.h:737,
from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2
arguments, but takes just 1
This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.
Looks like I made this patch just before this commit was pulled into
Linus's tree.
Which is why I'll apply and rerun the above again probably on Tuesday of
next week against Linus's latest.
This patch made it through both an allyesconfig and an allmodconfig, but on
the commit I had applied it to, which was:
1b294a1f3561 ("Merge tag 'net-next-6.10' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next")
I'll be compiling those two builds after I update it then.
I am currently repeating my test builds with the above errors fixed.
That should take a couple of hours. I'll let you know how it goes.
There are no more build failures caused by this patch after fixing the above
errors.
Tested-by: Guenter Roeck
Guenter
Re: [RESEND v3 1/2] drm: enable (most) W=1 warnings by default across the subsystem
Hi, On Tue, Mar 05, 2024 at 11:07:35AM +0200, Jani Nikula wrote: > At least the i915 and amd drivers enable a bunch more compiler warnings > than the kernel defaults. > > Extend most of the W=1 warnings to the entire drm subsystem by > default. Use the copy-pasted warnings from scripts/Makefile.extrawarn > with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and > keep up with them in the future. > > This is similar to the approach currently used in i915. > > Some of the -Wextra warnings do need to be disabled, just like in > Makefile.extrawarn, but take care to not disable them for W=2 or W=3 > builds, depending on the warning. > > There are too many -Wformat-truncation warnings to cleanly fix up front; > leave that warning disabled for now. > With this patch in the mainline kernel, I get the following build error when trying to build parisc:allmodconfig. Error log: drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd': drivers/gpu/drm/nouveau/nvif/object.c:161:9: error: 'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32 overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict] 161 | memcpy(data, args->mthd.data, size); | ^~~ drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor': drivers/gpu/drm/nouveau/nvif/object.c:298:17: error: 'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56 overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict] 298 | memcpy(data, args->new.data, size); The problem is also seen with v6.9 when trying to build an image with W=1, so it is not triggered by a code change. I don't know if other architectures are affected. The problem is not seen with gcc 11.4, but it is seen with gcc 12.3 and 13.2. I did not try with older versions of gcc. Bisect log is attached for reference. The odd error makes me wonder if I should revert to testing with gcc 11.4 and no longer bother with later versions of gcc, at least for any affected architectures. Any recommendations ? Thanks, Guenter --- # bad: [7ee332c9f12bc5b380e36919cd7d056592a7073f] Merge tag 'parisc-for-6.10-1' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux # good: [a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6] Linux 6.9 git bisect start 'HEAD' 'v6.9' # good: [1b294a1f35616977caddaddf3e9d28e576a1adbc] Merge tag 'net-next-6.10' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next git bisect good 1b294a1f35616977caddaddf3e9d28e576a1adbc # bad: [d34672777da3ea919e8adb0670ab91ddadf7dea0] Merge tag 'fbdev-for-6.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev git bisect bad d34672777da3ea919e8adb0670ab91ddadf7dea0 # bad: [2871ec40994912ce4f2e2d5072a428eb84c77d3c] Merge tag 'drm-misc-next-2024-04-19' of https://gitlab.freedesktop.org/drm/misc/kernel into drm-next git bisect bad 2871ec40994912ce4f2e2d5072a428eb84c77d3c # bad: [34633158b8eb8fca145c9a73f8fe4f98c7275b06] Merge tag 'amd-drm-next-6.10-2024-04-13' of https://gitlab.freedesktop.org/agd5f/linux into drm-next git bisect bad 34633158b8eb8fca145c9a73f8fe4f98c7275b06 # good: [4b0cb230bdb71c23981acfa5e7b367c7dde02a41] drm/amdgpu: retire UMC v12 mca_addr_to_pa git bisect good 4b0cb230bdb71c23981acfa5e7b367c7dde02a41 # bad: [6376eb8b911534735fec104c1a0d780e4cf3116a] drm/dp: Clarify that wait_hpd_asserted() is not optional for panels git bisect bad 6376eb8b911534735fec104c1a0d780e4cf3116a # bad: [9c86b03863844ce69f99aa66404c79492ec9e208] drm/panthor: Fix panthor_devfreq kerneldoc git bisect bad 9c86b03863844ce69f99aa66404c79492ec9e208 # bad: [b5d7cb76f2674c9d01b611141702723a95d12553] drm: add missing header guards to drm_internal.h git bisect bad b5d7cb76f2674c9d01b611141702723a95d12553 # good: [4bdca11507928a4c9174e9b7240e9d058c12a71d] drm/panthor: Add the driver frontend block git bisect good 4bdca11507928a4c9174e9b7240e9d058c12a71d # good: [b2ec429b69280001d85029dc50b5427af41eb641] drm/tidss: Use dev_err_probe() over dev_dbg() when failing to probe the port git bisect good b2ec429b69280001d85029dc50b5427af41eb641 # bad: [a61ddb4393ad1be61d2ffd92576d42707b05be17] drm: enable (most) W=1 warnings by default across the subsystem git bisect bad a61ddb4393ad1be61d2ffd92576d42707b05be17 # good: [113cc3ad8566e06d6c8ef4fc0075a938dedefab5] drm/bridge: Document bridge init order with pre_enable_prev_first git bisect good 113cc3ad8566e06d6c8ef4fc0075a938dedefab5 # good: [460be1d527a8e296d85301e8b14923299508d4fc] drm/nouveau: move more missing UAPI bits git bisect good 460be1d527a8e296d85301e8b14923299508d4fc # first bad commit: [a61ddb4393ad1be61d2ffd92576d42707b05be17] drm: enable (most) W=1 warnings by default across the subsystem
Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()
On 5/17/24 10:48, Steven Rostedt wrote:
On Fri, 17 May 2024 10:36:37 -0700
Guenter Roeck wrote:
Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
from include/trace/define_trace.h:102,
from drivers/cxl/core/trace.h:737,
from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2
arguments, but takes just 1
This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.
Looks like I made this patch just before this commit was pulled into
Linus's tree.
Which is why I'll apply and rerun the above again probably on Tuesday of
next week against Linus's latest.
This patch made it through both an allyesconfig and an allmodconfig, but on
the commit I had applied it to, which was:
1b294a1f3561 ("Merge tag 'net-next-6.10' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next")
I'll be compiling those two builds after I update it then.
I am currently repeating my test builds with the above errors fixed.
That should take a couple of hours. I'll let you know how it goes.
Guenter
Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Thu, May 16, 2024 at 01:34:54PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)"
>
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
>
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
>
> This means that with:
>
> __string(field, mystring)
>
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
>
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
>
> git grep -l __assign_str | while read a ; do
> sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
> mv /tmp/test-file $a;
> done
>
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
>
Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
from include/trace/define_trace.h:102,
from drivers/cxl/core/trace.h:737,
from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2
arguments, but takes just 1
This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.
Guenter
Re: [PATCH v3 03/15] kunit: Add test cases for backtrace warning suppression
On Tue, Apr 09, 2024 at 04:29:42PM +0800, David Gow wrote: > > +ifeq ($(CCONFIG_KUNIT_SUPPRESS_BACKTRACE),y) > > s/CCONFIG_/CONFIG_/ ? > > Odd, I know I tested this (and it still works ;-). The additional "C" must have slipped in at some point. Thanks for noticing! Guenter
Re: [PATCH v3 06/15] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests
On Wed, Apr 03, 2024 at 06:34:12PM -0700, Jakub Kicinski wrote: > On Wed, 3 Apr 2024 06:19:27 -0700 Guenter Roeck wrote: > > 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 and distracting. > > > > 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. > > Is it okay if I move the locking into the tests, instead? > It's only 4 lines more and no magic required, seems to work fine. I don't think it is that simple. Unit tests run in a kernel thread and exit immediately if a test fails. While the unit test code _looks_ sequential, that isn't really the case. Every instance of KUNIT_ASSERT_x or KUNIT_FAIL() results in immediate kernel thread termination. If that happens, any rtnl_unlock() in the failed function would not be executed. I am not aware of an equivalent of atexit() for kernel threads which would fix the problem. My understanding is that the kunit system doesn't support an equivalent either, but at least sometimes executes the exit function in a different thread context. Guenter
[PATCH v3 13/15] sh: Move defines needed for suppressing warning backtraces
Declaring the defines needed for suppressing warning inside '#ifdef CONFIG_DEBUG_BUGVERBOSE' results in a kerneldoc warning. .../bug.h:29: warning: expecting prototype for _EMIT_BUG_ENTRY(). Prototype was for HAVE_BUG_FUNCTION() instead Move the defines above the kerneldoc entry for _EMIT_BUG_ENTRY to make kerneldoc happy. Reported-by: Simon Horman Cc: Simon Horman Cc: Yoshinori Sato Cc: Rich Felker Cc: John Paul Adrian Glaubitz Signed-off-by: Guenter Roeck --- v3: Added patch. Possibly squash into previous patch. arch/sh/include/asm/bug.h | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h index 470ce6567d20..bf4947d51d69 100644 --- a/arch/sh/include/asm/bug.h +++ b/arch/sh/include/asm/bug.h @@ -11,6 +11,15 @@ #define HAVE_ARCH_BUG #define HAVE_ARCH_WARN_ON +#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 */ +#endif /* CONFIG_DEBUG_BUGVERBOSE */ + /** * _EMIT_BUG_ENTRY * %1 - __FILE__ @@ -25,13 +34,6 @@ */ #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" \ -- 2.39.2
[PATCH v3 15/15] powerpc: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Cc: Michael Ellerman
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/powerpc/include/asm/bug.h | 37 +-
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 1db485aacbd9..5b06745d20aa 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -14,6 +14,9 @@
.section __bug_table,"aw"
5001: .4byte \addr - .
.4byte 5002f - .
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+.4byte 0
+#endif
.short \line, \flags
.org 5001b+BUG_ENTRY_SIZE
.previous
@@ -32,30 +35,46 @@
#endif /* verbose */
#else /* !__ASSEMBLY__ */
-/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
- sizeof(struct bug_entry), respectively */
+/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3,%4 to be FILE, __func__, LINE,
flags
+ and sizeof(struct bug_entry), respectively */
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .4byte %1 - .\n"
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte %0 - .\n"\
- " .short %1, %2\n"\
- ".org 2b+%3\n" \
+ __BUG_FUNC_PTR \
+ " .short %2, %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#else
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
- " .short %2\n"\
- ".org 2b+%3\n" \
+ " .short %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#endif
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define BUG_ENTRY(insn, flags, ...)\
__asm__ __volatile__( \
"1: " insn "\n" \
_EMIT_BUG_ENTRY \
- : : "i" (__FILE__), "i" (__LINE__), \
+ : : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
##__VA_ARGS__)
@@ -80,7 +99,7 @@
if (x) \
BUG(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "r" ((__force long)(x)));
\
+ BUG_ENTRY(PPC_TLNEI " %5, 0", 0, "r" ((__force long)(x)));
\
} \
} while (0)
@@ -90,7 +109,7 @@
if (__ret_warn_on) \
__WARN(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", \
+ BUG_ENTRY(PPC_TLNEI " %5, 0", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
"r" (__ret_warn_on)); \
} \
--
2.39.2
[PATCH v3 14/15] riscv: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Cc: Paul Walmsley
Cc: Palmer Dabbelt
Cc: Albert Ou
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/riscv/include/asm/bug.h | 38
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
index 1aaea81fb141..79f360af4ad8 100644
--- a/arch/riscv/include/asm/bug.h
+++ b/arch/riscv/include/asm/bug.h
@@ -30,26 +30,39 @@
typedef u32 bug_insn_t;
#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
-#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ."
-#define __BUG_ENTRY_FILE RISCV_INT " %0 - ."
+#define __BUG_REL(val) RISCV_INT " " __stringify(val) " - ."
#else
-#define __BUG_ENTRY_ADDR RISCV_PTR " 1b"
-#define __BUG_ENTRY_FILE RISCV_PTR " %0"
+#define __BUG_REL(val) RISCV_PTR " " __stringify(val)
#endif
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%1)
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- __BUG_ENTRY_FILE "\n\t" \
- RISCV_SHORT " %1\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ __BUG_REL(%0) "\n\t"\
+ __BUG_FUNC_PTR "\n\t" \
+ RISCV_SHORT " %2\n\t" \
+ RISCV_SHORT " %3"
#else
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ RISCV_SHORT " %3"
#endif
#ifdef CONFIG_GENERIC_BUG
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
do { \
__asm__ __volatile__ ( \
@@ -58,10 +71,11 @@ do {
\
".pushsection __bug_table,\"aw\"\n\t" \
"2:\n\t"\
__BUG_ENTRY "\n\t" \
- ".org 2b + %3\n\t" \
+ ".org 2b + %4\n\t" \
".popsection" \
: \
- : "i" (__FILE__), "i" (__LINE__), \
+ : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
--
2.39.2
[PATCH v3 12/15] sh: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Cc: Yoshinori Sato
Cc: Rich Felker
Cc: John Paul Adrian Glaubitz
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
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 */
+
#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_FUNCNULL
+#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))); \
--
2.39.2
[PATCH v3 11/15] s390: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Cc: Heiko Carstens
Cc: Vasily Gorbik
Cc: Alexander Gordeev
Signed-off-by: Guenter Roeck
---
v2:
- 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
v3:
- Rebased to v6.9-rc2
arch/s390/include/asm/bug.h | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index c500d45fb465..44d4e9f24ae0 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -8,6 +8,15 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .long %0-.\n"
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define __EMIT_BUG(x) do { \
asm_inline volatile(\
"0: mc 0,0\n" \
@@ -17,10 +26,12 @@
".section __bug_table,\"aw\"\n" \
"2: .long 0b-.\n" \
" .long 1b-.\n" \
- " .short %0,%1\n"\
- " .org2b+%2\n"\
+ __BUG_FUNC_PTR \
+ " .short %1,%2\n"\
+ " .org2b+%3\n"\
".previous\n" \
- : : "i" (__LINE__), \
+ : : "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (x),\
"i" (sizeof(struct bug_entry)));\
} while (0)
--
2.39.2
[PATCH v3 09/15] loongarch: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Cc: Huacai Chen
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1; resolved context conflict
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2; resolved context conflict
arch/loongarch/include/asm/bug.h | 38 +++-
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index 08388876ade4..193f396d81a0 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -3,47 +3,63 @@
#define __ASM_BUG_H
#include
+#include
#include
#ifndef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#else
-#define __BUGVERBOSE_LOCATION(file, line) \
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str, "aMS", @progbits, 1; \
10002: .string file; \
.popsection;\
\
.long 10002b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
#endif
#ifndef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#else
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table, "aw"; \
.align 2; \
1: .long 10001f - .; \
- _BUGVERBOSE_LOCATION(__FILE__, __LINE__)\
+ _BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
.short flags; \
.popsection;\
10001:
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
break BRK_BUG
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
+
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
#define __BUG_FLAGS(flags) \
- asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags, %0)) : : "i"
(__BUG_FUNC));
#define __WARN_FLAGS(flags)\
do { \
instrumentation_begin();\
- __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
annotate_reachable(); \
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH v3 10/15] parisc: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Acked-by: Helge Deller
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/parisc/include/asm/bug.h | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 833555f74ffa..b59c3f7380bf 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -23,8 +23,17 @@
# define __BUG_REL(val) ".word " __stringify(val)
#endif
-
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define BUG() \
do {\
asm volatile("\n" \
@@ -33,10 +42,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %c3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (0), "i" (sizeof(struct bug_entry)) ); \
unreachable(); \
} while(0)
@@ -58,10 +69,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
} while(0)
@@ -74,7 +87,7 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t.short %0\n"\
-"\t.blockz %1-4-2\n" \
+"\t.blockz %c1-(.-2b)\n" \
"\t.popsection"\
: : "i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
--
2.39.2
[PATCH v3 03/15] kunit: Add test cases for backtrace warning suppression
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
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
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
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 545b57c3be48..3eee1bd0ce5e 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -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.o
obj-$(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)
diff --git a/lib/kunit/backtrace-suppression-test.c
b/lib/kunit/backtrace-suppression-test.c
new file mode 100644
index ..47c619283802
--- /dev/null
+++ b/lib/kunit/backtrace-suppression-test.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for suppressing warning tracebacks
+ *
+ * Copyright (C) 2024, Guenter Roeck
+ * Author: Guenter Roeck
+ */
+
+#include
+#include
+
+static void backtrace_suppression_test_warn_direct(struct kunit *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);
+}
+
+static void trigger_backtrace_warn(void)
+{
+ WARN(1, "This backtrace should be suppressed");
+}
+
+static void backtrace_suppression_test_warn_indirect(struct kunit *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);
+}
+
+static void backtrace_suppression_test_warn_multi(struct kunit *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);
+}
+
+static void backtrace_suppression_test_warn_on_direct(struct kunit *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);
+}
+
+static void trigger_backtrace_warn_on(void)
+{
+ WARN_ON(1);
+}
+
+static void backtrace_suppression_test_warn_on_indirect(struct kunit *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);
+}
+
+static struct kunit_case backtrace_suppression_test_cases[] = {
+ KUNIT_CASE(b
[PATCH v3 08/15] arm64: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Cc: Catalin Marinas
Cc: Will Deacon
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/arm64/include/asm/asm-bug.h | 29 +++--
arch/arm64/include/asm/bug.h | 8 +++-
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index c762038ba400..c6d22e3cd840 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -8,36 +8,45 @@
#include
#ifdef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
-#define __BUGVERBOSE_LOCATION(file, line) \
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str,"aMS",@progbits,1; \
14472: .string file; \
.popsection;\
\
.long 14472b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
#else
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#endif
#ifdef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table,"aw"; \
.align 2; \
14470: .long 14471f - .; \
-_BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
- .short flags; \
+_BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
+ .short flags; \
.popsection;\
14471:
#else
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
brk BUG_BRK_IMM
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
#endif /* __ASM_ASM_BUG_H */
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 28be048db3f6..044c5e24a17d 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -11,8 +11,14 @@
#include
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
- asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i"
(__BUG_FUNC));
#define BUG() do { \
__BUG_FLAGS(0); \
--
2.39.2
[PATCH v3 07/15] x86: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: Dave Hansen
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/x86/include/asm/bug.h | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index a3ec87d198ac..7698dfa74c98 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -23,18 +23,28 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _BUG_FLAGS(ins, flags, extra) \
do { \
asm_inline volatile("1:\t" ins "\n" \
".pushsection __bug_table,\"aw\"\n"\
"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
"\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
-"\t.word %c1""\t# bug_entry::line\n" \
-"\t.word %c2""\t# bug_entry::flags\n" \
-"\t.org 2b+%c3\n" \
+"\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
+"\t.word %c2""\t# bug_entry::line\n" \
+"\t.word %c3""\t# bug_entry::flags\n" \
+"\t.org 2b+%c4\n" \
".popsection\n"\
extra \
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
@@ -80,7 +90,8 @@ do {
\
do { \
__auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin();\
- _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH v3 06/15] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests
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 and distracting.
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
Cc: David Gow
Cc: Jakub Kicinski
Cc: Eric Dumazet
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
v3:
- Rebased to v6.9-rc2
net/core/dev_addr_lists_test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index 4dbd0dc6aea2..b427dd1a3c93 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#include
+#include
#include
#include
#include
@@ -49,6 +50,7 @@ static int dev_addr_test_init(struct kunit *test)
KUNIT_FAIL(test, "Can't register netdev %d", err);
}
+ debug_locks_silent = 1;
rtnl_lock();
return 0;
}
@@ -56,8 +58,12 @@ static int dev_addr_test_init(struct kunit *test)
static void dev_addr_test_exit(struct kunit *test)
{
struct net_device *netdev = test->priv;
+ DEFINE_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ START_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
rtnl_unlock();
+ END_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ debug_locks_silent = 0;
unregister_netdev(netdev);
free_netdev(netdev);
}
--
2.39.2
[PATCH v3 05/15] drm: Suppress intentional warning backtraces in scaling unit tests
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 and distraction from real problems.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Acked-by: Maíra Canal
Cc: Maarten Lankhorst
Cc: David Airlie
Cc: Daniel Vetter
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
v3:
- Rebased to v6.9-rc2
drivers/gpu/drm/tests/drm_rect_test.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
b/drivers/gpu/drm/tests/drm_rect_test.c
index 76332cd2ead8..66851769ee32 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -406,22 +406,38 @@ KUNIT_ARRAY_PARAM(drm_rect_scale, drm_rect_scale_cases,
drm_rect_scale_case_desc
static void drm_test_rect_calc_hscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ /*
+* 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.
+*/
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_hscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
static void drm_test_rect_calc_vscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ /*
+* drm_rect_calc_vscale() generates a warning backtrace whenever bad
+* parameters are passed to it. This affects all unit tests with an
+* error code in expected_scaling_factor.
+*/
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_vscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
--
2.39.2
[PATCH v3 04/15] kunit: Add documentation for warning backtrace suppression API
Document API functions for suppressing warning backtraces.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
v3:
- Rebased to v6.9-rc2
Documentation/dev-tools/kunit/usage.rst | 30 -
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst
b/Documentation/dev-tools/kunit/usage.rst
index 22955d56b379..8d3d36d4103d 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -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.
- }
\ No newline at end of file
+ }
--
2.39.2
[PATCH v3 01/15] bug/kunit: Core support for suppressing warning backtraces
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
Cc: Daniel Diaz
Cc: Naresh Kamboju
Cc: Kees Cook
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
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
v3:
- Rebased to v6.9-rc2
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
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 6e794420bd39..c170b6477689 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -18,6 +18,7 @@
#endif
#ifndef __ASSEMBLY__
+#include
#include
#include
@@ -39,8 +40,14 @@ struct bug_entry {
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
const char *file;
+#ifdef HAVE_BUG_FUNCTION
+ const char *function;
+#endif
#else
signed int file_disp;
+#ifdef HAVE_BUG_FUNCTION
+ signed int function_disp;
+#endif
#endif
unsigned short line;
#endif
@@ -96,15 +103,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt,
...);
#define __WARN() __WARN_printf(TAINT_WARN, NULL)
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- warn_slowpath_fmt(__FILE__, __LINE__, taint, arg); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);\
instrumentation_end(); \
} while (0)
#else
#define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- __warn_printk(arg); \
- __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+ if (!IS_SUPPRESSED_WARNING(__func__)) { \
+ __warn_printk(arg); \
+ __WARN_FLAGS(BUGFLAG_NO_CUT_HERE |
BUGFLAG_TAINT(taint));\
+ } \
instrumentation_end(); \
} while (0)
#define WARN_ON_ONCE(condition) ({ \
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
new file mode 100644
index ..bd0fe047572b
--- /dev/null
+++ b/include/kunit/bug.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck
+ */
+
+#ifndef _KUNIT_BUG_H
+#define _KUNIT_BUG_H
+
+#ifndef __ASSEMBLY__
+
+#include
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+
+#include
+#include
+
+struct __suppressed_warning {
+ struct list
[PATCH v3 00/15] Add support for suppressing warning backtraces
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. Changes since v2: - Rebased to v6.9-rc2 - Added comments to drm warning suppression explaining why it is needed. - Added patch to move conditional code in arch/sh/include/asm/bug.h to avoid kerneldoc warning - Added architecture maintainers to Cc: for architecture specific patches - No functional changes -------- Guenter Roeck (15): bug/kunit: Core support for suppressing warning backtraces kunit: bug: Count suppressed warning backtraces kunit: Add test cases for backtrace warning suppression kunit: Add documentation for warning backtrace suppression API drm: Suppress intentional warning backtraces in scaling unit tests net: kunit: Suppress lock warning noise at end of dev_addr_lists tests x86: Add support for suppressing warning backtraces arm64: Add support for suppressing warning backtraces loongarch: Add support for suppressing warning backtraces parisc: Add support for suppressing warning backtraces s390: Add support for suppressing warning backtraces sh: Add support for suppressing warning backtraces sh: Move defines needed for suppressing warning backtraces riscv: Add support for suppressing warning
[PATCH v3 02/15] kunit: bug: Count suppressed warning backtraces
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
Reviewed-by: Kees Cook
Tested-by: Linux Kernel Functional Testing
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
include/kunit/bug.h | 7 ++-
lib/kunit/bug.c | 4 +++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
index bd0fe047572b..72e9fb23bbd5 100644
--- a/include/kunit/bug.h
+++ b/include/kunit/bug.h
@@ -20,6 +20,7 @@
struct __suppressed_warning {
struct list_head node;
const char *function;
+ int counter;
};
void __start_suppress_warning(struct __suppressed_warning *warning);
@@ -28,7 +29,7 @@ bool __is_suppressed_warning(const char *function);
#define DEFINE_SUPPRESSED_WARNING(func)\
struct __suppressed_warning __kunit_suppress_##func = \
- { .function = __stringify(func) }
+ { .function = __stringify(func), .counter = 0 }
#define START_SUPPRESSED_WARNING(func) \
__start_suppress_warning(&__kunit_suppress_##func)
@@ -39,12 +40,16 @@ bool __is_suppressed_warning(const char *function);
#define IS_SUPPRESSED_WARNING(func) \
__is_suppressed_warning(func)
+#define SUPPRESSED_WARNING_COUNT(func) \
+ (__kunit_suppress_##func.counter)
+
#else /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#define DEFINE_SUPPRESSED_WARNING(func)
#define START_SUPPRESSED_WARNING(func)
#define END_SUPPRESSED_WARNING(func)
#define IS_SUPPRESSED_WARNING(func) (false)
+#define SUPPRESSED_WARNING_COUNT(func) (0)
#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#endif /* __ASSEMBLY__ */
diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
index f93544d7a9d1..13b3d896c114 100644
--- a/lib/kunit/bug.c
+++ b/lib/kunit/bug.c
@@ -32,8 +32,10 @@ bool __is_suppressed_warning(const char *function)
return false;
list_for_each_entry(warning, _warnings, node) {
- if (!strcmp(function, warning->function))
+ if (!strcmp(function, warning->function)) {
+ warning->counter++;
return true;
+ }
}
return false;
}
--
2.39.2
Re: [PATCH v2 12/14] sh: Add support for suppressing warning backtraces
On Wed, Mar 27, 2024 at 07:39:20PM +, 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
Re: [PATCH v2 12/14] sh: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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_FUNCNULL
+#endif
+
#define BUG() \
do { \
__asm__ __volatile__ ( \
...
Re: [PATCH v2 05/14] drm: Suppress intentional warning backtraces in scaling unit tests
On 3/25/24 18:09, Maíra Canal wrote:
On 3/25/24 16:24, Guenter Roeck wrote:
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:
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(+)
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
b/drivers/gpu/drm/tests/drm_rect_test.c
index 76332cd2ead8..75614cb4deb5 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -406,22 +406,28 @@ KUNIT_ARRAY_PARAM(drm_rect_scale, drm_rect_scale_cases,
drm_rect_scale_case_desc
static void drm_test_rect_calc_hscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ START_SUPPRESSED_WARNING(drm_calc_scale);
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
Thanks!
Guenter
Re: [PATCH v2 05/14] drm: Suppress intentional warning backtraces in scaling unit tests
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:
> > 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
> > Acked-by: Dan Carpenter
> > Signed-off-by: Guenter Roeck
> > ---
> > - 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(+)
> >
> > diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
> > b/drivers/gpu/drm/tests/drm_rect_test.c
> > index 76332cd2ead8..75614cb4deb5 100644
> > --- a/drivers/gpu/drm/tests/drm_rect_test.c
> > +++ b/drivers/gpu/drm/tests/drm_rect_test.c
> > @@ -406,22 +406,28 @@ KUNIT_ARRAY_PARAM(drm_rect_scale,
> > drm_rect_scale_cases, drm_rect_scale_case_desc
> > static void drm_test_rect_calc_hscale(struct kunit *test)
> > {
> > + DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
> > const struct drm_rect_scale_case *params = test->param_value;
> > int scaling_factor;
> > + START_SUPPRESSED_WARNING(drm_calc_scale);
>
> 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
[PATCH v2 09/14] loongarch: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index 08388876ade4..193f396d81a0 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -3,47 +3,63 @@
#define __ASM_BUG_H
#include
+#include
#include
#ifndef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#else
-#define __BUGVERBOSE_LOCATION(file, line) \
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str, "aMS", @progbits, 1; \
10002: .string file; \
.popsection;\
\
.long 10002b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
#endif
#ifndef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#else
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table, "aw"; \
.align 2; \
1: .long 10001f - .; \
- _BUGVERBOSE_LOCATION(__FILE__, __LINE__)\
+ _BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
.short flags; \
.popsection;\
10001:
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
break BRK_BUG
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
+
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
#define __BUG_FLAGS(flags) \
- asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags, %0)) : : "i"
(__BUG_FUNC));
#define __WARN_FLAGS(flags)\
do { \
instrumentation_begin();\
- __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
annotate_reachable(); \
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH v2 08/14] arm64: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index c762038ba400..c6d22e3cd840 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -8,36 +8,45 @@
#include
#ifdef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
-#define __BUGVERBOSE_LOCATION(file, line) \
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str,"aMS",@progbits,1; \
14472: .string file; \
.popsection;\
\
.long 14472b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
#else
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#endif
#ifdef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table,"aw"; \
.align 2; \
14470: .long 14471f - .; \
-_BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
- .short flags; \
+_BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
+ .short flags; \
.popsection;\
14471:
#else
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
brk BUG_BRK_IMM
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
#endif /* __ASM_ASM_BUG_H */
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 28be048db3f6..044c5e24a17d 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -11,8 +11,14 @@
#include
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
- asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i"
(__BUG_FUNC));
#define BUG() do { \
__BUG_FLAGS(0); \
--
2.39.2
[PATCH v2 14/14] powerpc: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 1db485aacbd9..5b06745d20aa 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -14,6 +14,9 @@
.section __bug_table,"aw"
5001: .4byte \addr - .
.4byte 5002f - .
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+.4byte 0
+#endif
.short \line, \flags
.org 5001b+BUG_ENTRY_SIZE
.previous
@@ -32,30 +35,46 @@
#endif /* verbose */
#else /* !__ASSEMBLY__ */
-/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
- sizeof(struct bug_entry), respectively */
+/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3,%4 to be FILE, __func__, LINE,
flags
+ and sizeof(struct bug_entry), respectively */
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .4byte %1 - .\n"
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte %0 - .\n"\
- " .short %1, %2\n"\
- ".org 2b+%3\n" \
+ __BUG_FUNC_PTR \
+ " .short %2, %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#else
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
- " .short %2\n"\
- ".org 2b+%3\n" \
+ " .short %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#endif
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define BUG_ENTRY(insn, flags, ...)\
__asm__ __volatile__( \
"1: " insn "\n" \
_EMIT_BUG_ENTRY \
- : : "i" (__FILE__), "i" (__LINE__), \
+ : : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
##__VA_ARGS__)
@@ -80,7 +99,7 @@
if (x) \
BUG(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "r" ((__force long)(x)));
\
+ BUG_ENTRY(PPC_TLNEI " %5, 0", 0, "r" ((__force long)(x)));
\
} \
} while (0)
@@ -90,7 +109,7 @@
if (__ret_warn_on) \
__WARN(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", \
+ BUG_ENTRY(PPC_TLNEI " %5, 0", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
"r" (__ret_warn_on)); \
} \
--
2.39.2
[PATCH v2 11/14] s390: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index c500d45fb465..44d4e9f24ae0 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -8,6 +8,15 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .long %0-.\n"
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define __EMIT_BUG(x) do { \
asm_inline volatile(\
"0: mc 0,0\n" \
@@ -17,10 +26,12 @@
".section __bug_table,\"aw\"\n" \
"2: .long 0b-.\n" \
" .long 1b-.\n" \
- " .short %0,%1\n"\
- " .org2b+%2\n"\
+ __BUG_FUNC_PTR \
+ " .short %1,%2\n"\
+ " .org2b+%3\n"\
".previous\n" \
- : : "i" (__LINE__), \
+ : : "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (x),\
"i" (sizeof(struct bug_entry)));\
} while (0)
--
2.39.2
[PATCH v2 13/14] riscv: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
index 1aaea81fb141..79f360af4ad8 100644
--- a/arch/riscv/include/asm/bug.h
+++ b/arch/riscv/include/asm/bug.h
@@ -30,26 +30,39 @@
typedef u32 bug_insn_t;
#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
-#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ."
-#define __BUG_ENTRY_FILE RISCV_INT " %0 - ."
+#define __BUG_REL(val) RISCV_INT " " __stringify(val) " - ."
#else
-#define __BUG_ENTRY_ADDR RISCV_PTR " 1b"
-#define __BUG_ENTRY_FILE RISCV_PTR " %0"
+#define __BUG_REL(val) RISCV_PTR " " __stringify(val)
#endif
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%1)
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- __BUG_ENTRY_FILE "\n\t" \
- RISCV_SHORT " %1\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ __BUG_REL(%0) "\n\t"\
+ __BUG_FUNC_PTR "\n\t" \
+ RISCV_SHORT " %2\n\t" \
+ RISCV_SHORT " %3"
#else
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ RISCV_SHORT " %3"
#endif
#ifdef CONFIG_GENERIC_BUG
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
do { \
__asm__ __volatile__ ( \
@@ -58,10 +71,11 @@ do {
\
".pushsection __bug_table,\"aw\"\n\t" \
"2:\n\t"\
__BUG_ENTRY "\n\t" \
- ".org 2b + %3\n\t" \
+ ".org 2b + %4\n\t" \
".popsection" \
: \
- : "i" (__FILE__), "i" (__LINE__), \
+ : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
--
2.39.2
[PATCH v2 10/14] parisc: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Acked-by: Helge Deller
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 833555f74ffa..b59c3f7380bf 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -23,8 +23,17 @@
# define __BUG_REL(val) ".word " __stringify(val)
#endif
-
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define BUG() \
do {\
asm volatile("\n" \
@@ -33,10 +42,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %c3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (0), "i" (sizeof(struct bug_entry)) ); \
unreachable(); \
} while(0)
@@ -58,10 +69,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
} while(0)
@@ -74,7 +87,7 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t.short %0\n"\
-"\t.blockz %1-4-2\n" \
+"\t.blockz %c1-(.-2b)\n" \
"\t.popsection"\
: : "i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
--
2.39.2
[PATCH v2 12/14] sh: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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 */
+
#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_FUNCNULL
+#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))); \
--
2.39.2
[PATCH v2 07/14] x86: Add support for suppressing warning backtraces
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index a3ec87d198ac..7698dfa74c98 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -23,18 +23,28 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _BUG_FLAGS(ins, flags, extra) \
do { \
asm_inline volatile("1:\t" ins "\n" \
".pushsection __bug_table,\"aw\"\n"\
"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
"\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
-"\t.word %c1""\t# bug_entry::line\n" \
-"\t.word %c2""\t# bug_entry::flags\n" \
-"\t.org 2b+%c3\n" \
+"\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
+"\t.word %c2""\t# bug_entry::line\n" \
+"\t.word %c3""\t# bug_entry::flags\n" \
+"\t.org 2b+%c4\n" \
".popsection\n"\
extra \
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
@@ -80,7 +90,8 @@ do {
\
do { \
__auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin();\
- _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH v2 05/14] drm: Suppress intentional warning backtraces in scaling unit tests
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
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(+)
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
b/drivers/gpu/drm/tests/drm_rect_test.c
index 76332cd2ead8..75614cb4deb5 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -406,22 +406,28 @@ KUNIT_ARRAY_PARAM(drm_rect_scale, drm_rect_scale_cases,
drm_rect_scale_case_desc
static void drm_test_rect_calc_hscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_hscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
static void drm_test_rect_calc_vscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_vscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
--
2.39.2
[PATCH v2 06/14] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests
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
Cc: David Gow
Cc: Jakub Kicinski
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
- 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(+)
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index 4dbd0dc6aea2..b427dd1a3c93 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#include
+#include
#include
#include
#include
@@ -49,6 +50,7 @@ static int dev_addr_test_init(struct kunit *test)
KUNIT_FAIL(test, "Can't register netdev %d", err);
}
+ debug_locks_silent = 1;
rtnl_lock();
return 0;
}
@@ -56,8 +58,12 @@ static int dev_addr_test_init(struct kunit *test)
static void dev_addr_test_exit(struct kunit *test)
{
struct net_device *netdev = test->priv;
+ DEFINE_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ START_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
rtnl_unlock();
+ END_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ debug_locks_silent = 0;
unregister_netdev(netdev);
free_netdev(netdev);
}
--
2.39.2
[PATCH v2 01/14] bug/kunit: Core support for suppressing warning backtraces
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
Cc: Daniel Diaz
Cc: Naresh Kamboju
Cc: Kees Cook
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
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
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 6e794420bd39..c170b6477689 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -18,6 +18,7 @@
#endif
#ifndef __ASSEMBLY__
+#include
#include
#include
@@ -39,8 +40,14 @@ struct bug_entry {
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
const char *file;
+#ifdef HAVE_BUG_FUNCTION
+ const char *function;
+#endif
#else
signed int file_disp;
+#ifdef HAVE_BUG_FUNCTION
+ signed int function_disp;
+#endif
#endif
unsigned short line;
#endif
@@ -96,15 +103,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt,
...);
#define __WARN() __WARN_printf(TAINT_WARN, NULL)
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- warn_slowpath_fmt(__FILE__, __LINE__, taint, arg); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);\
instrumentation_end(); \
} while (0)
#else
#define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- __warn_printk(arg); \
- __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+ if (!IS_SUPPRESSED_WARNING(__func__)) { \
+ __warn_printk(arg); \
+ __WARN_FLAGS(BUGFLAG_NO_CUT_HERE |
BUGFLAG_TAINT(taint));\
+ } \
instrumentation_end(); \
} while (0)
#define WARN_ON_ONCE(condition) ({ \
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
new file mode 100644
index ..bd0fe047572b
--- /dev/null
+++ b/include/kunit/bug.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck
+ */
+
+#ifndef _KUNIT_BUG_H
+#define _KUNIT_BUG_H
+
+#ifndef __ASSEMBLY__
+
+#include
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+
+#include
+#include
+
+struct __suppressed_warning {
+ struct list_head node;
+ const char
[PATCH v2 04/14] kunit: Add documentation for warning backtrace suppression API
Document API functions for suppressing warning backtraces.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
Documentation/dev-tools/kunit/usage.rst | 30 -
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst
b/Documentation/dev-tools/kunit/usage.rst
index 22955d56b379..8d3d36d4103d 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -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.
- }
\ No newline at end of file
+ }
--
2.39.2
[PATCH v2 02/14] kunit: bug: Count suppressed warning backtraces
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
Reviewed-by: Kees Cook
Tested-by: Linux Kernel Functional Testing
Signed-off-by: Guenter Roeck
---
- 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(-)
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
index bd0fe047572b..72e9fb23bbd5 100644
--- a/include/kunit/bug.h
+++ b/include/kunit/bug.h
@@ -20,6 +20,7 @@
struct __suppressed_warning {
struct list_head node;
const char *function;
+ int counter;
};
void __start_suppress_warning(struct __suppressed_warning *warning);
@@ -28,7 +29,7 @@ bool __is_suppressed_warning(const char *function);
#define DEFINE_SUPPRESSED_WARNING(func)\
struct __suppressed_warning __kunit_suppress_##func = \
- { .function = __stringify(func) }
+ { .function = __stringify(func), .counter = 0 }
#define START_SUPPRESSED_WARNING(func) \
__start_suppress_warning(&__kunit_suppress_##func)
@@ -39,12 +40,16 @@ bool __is_suppressed_warning(const char *function);
#define IS_SUPPRESSED_WARNING(func) \
__is_suppressed_warning(func)
+#define SUPPRESSED_WARNING_COUNT(func) \
+ (__kunit_suppress_##func.counter)
+
#else /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#define DEFINE_SUPPRESSED_WARNING(func)
#define START_SUPPRESSED_WARNING(func)
#define END_SUPPRESSED_WARNING(func)
#define IS_SUPPRESSED_WARNING(func) (false)
+#define SUPPRESSED_WARNING_COUNT(func) (0)
#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#endif /* __ASSEMBLY__ */
diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
index f93544d7a9d1..13b3d896c114 100644
--- a/lib/kunit/bug.c
+++ b/lib/kunit/bug.c
@@ -32,8 +32,10 @@ bool __is_suppressed_warning(const char *function)
return false;
list_for_each_entry(warning, _warnings, node) {
- if (!strcmp(function, warning->function))
+ if (!strcmp(function, warning->function)) {
+ warning->counter++;
return true;
+ }
}
return false;
}
--
2.39.2
[PATCH v2 03/14] kunit: Add test cases for backtrace warning suppression
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
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
- 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
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 545b57c3be48..3eee1bd0ce5e 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -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.o
obj-$(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)
diff --git a/lib/kunit/backtrace-suppression-test.c
b/lib/kunit/backtrace-suppression-test.c
new file mode 100644
index ..47c619283802
--- /dev/null
+++ b/lib/kunit/backtrace-suppression-test.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for suppressing warning tracebacks
+ *
+ * Copyright (C) 2024, Guenter Roeck
+ * Author: Guenter Roeck
+ */
+
+#include
+#include
+
+static void backtrace_suppression_test_warn_direct(struct kunit *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);
+}
+
+static void trigger_backtrace_warn(void)
+{
+ WARN(1, "This backtrace should be suppressed");
+}
+
+static void backtrace_suppression_test_warn_indirect(struct kunit *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);
+}
+
+static void backtrace_suppression_test_warn_multi(struct kunit *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);
+}
+
+static void backtrace_suppression_test_warn_on_direct(struct kunit *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);
+}
+
+static void trigger_backtrace_warn_on(void)
+{
+ WARN_ON(1);
+}
+
+static void backtrace_suppression_test_warn_on_indirect(struct kunit *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);
+}
+
+static struct kunit_case backtrace_suppression_test_cases[] = {
+ KUNIT_CASE(backtrace_suppression_test_warn_direct),
+
[PATCH v2 00/14] Add support for suppressing warning backtraces
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.
Re: [PATCH v5 08/11] devm-helpers: Add resource managed version of debugfs directory create function
On 3/23/24 09:43, Marek Behún wrote: A few drivers register a devm action to remove a debugfs directory, implementing a one-liner function that calls debufs_remove_recursive(). Help drivers avoid this repeated implementations by adding managed version of debugfs directory create function. Use the new function devm_debugfs_create_dir() in the following drivers: drivers/crypto/caam/ctrl.c drivers/gpu/drm/bridge/ti-sn65dsi86.c drivers/hwmon/hp-wmi-sensors.c drivers/hwmon/mr75203.c drivers/hwmon/pmbus/pmbus_core.c Please split this up into multiple patches, at least separating out the hwmon patches. Guenter
Re: [v3,5/5] drm/xe: Enable 32bits build
On 3/18/24 06:28, Lucas De Marchi wrote: On Sun, Mar 17, 2024 at 09:14:14AM -0700, Guenter Roeck wrote: Hi, On Thu, Jan 18, 2024 at 04:16:12PM -0800, Lucas De Marchi wrote: Now that all the issues with 32bits are fixed, enable it again. Reviewed-by: Matt Roper Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/xe/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig index 1b57ae38210d..1b0ef91a5d2c 100644 --- a/drivers/gpu/drm/xe/Kconfig +++ b/drivers/gpu/drm/xe/Kconfig @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_XE tristate "Intel Xe Graphics" - depends on DRM && PCI && MMU && (m || (y && KUNIT=y)) && 64BIT + depends on DRM && PCI && MMU && (m || (y && KUNIT=y)) I am curious about changes like this. Enabling 32-bit builds results in build failures for mips_allmodconfig because the driver redefines END. END is also used as macro in assembler code, the define happens to be included for mips builds, and it would be difficult to change it there. Unlike the i915 code, DRM_XE is not marked as depending on x86. This means it will be built for pretty much all "allmodconfig" builds for all architectures. Yet, there have been recent complaints about "allmodconfig" builds of drm code causing build failures on "oddball" architectures. Is there an assumption that DRM_XE (or DRM in general) is manually excluded from all architectures where it fails to build ? If so, would for all the reports we've been receiving we fixed the build and improved CI to try to avoid the regressions. DRM_XE doesn't really depend on x86, but I see your point of filtering out the "oddball architectures" or just expose the ones we know it builds against. Yet, I don't see that approach done in the wild in other drivers. At least on the build side, we constantly check the reports from lkp like https://lore.kernel.org/all/202403152008.klwyyggo-...@intel.com/ which also includes mips: mips allnoconfig gcc mips allyesconfig gcc is that not sufficient? allyesconfig should be covering it afaics FWIW: The kissb build system reports the problem as well, so it isn't just me. http://kisskb.ellerman.id.au/kisskb/buildresult/15143996/ Sure, that is allmodconfig vs. allyesconfig, but that does not make a difference. The compiler version doesn't make a difference either. kissb runs tests with gcc-8 and gcc-13, and they both fail. Guenter
Re: [v3,5/5] drm/xe: Enable 32bits build
On 3/18/24 06:28, Lucas De Marchi wrote: On Sun, Mar 17, 2024 at 09:14:14AM -0700, Guenter Roeck wrote: Hi, On Thu, Jan 18, 2024 at 04:16:12PM -0800, Lucas De Marchi wrote: Now that all the issues with 32bits are fixed, enable it again. Reviewed-by: Matt Roper Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/xe/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig index 1b57ae38210d..1b0ef91a5d2c 100644 --- a/drivers/gpu/drm/xe/Kconfig +++ b/drivers/gpu/drm/xe/Kconfig @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_XE tristate "Intel Xe Graphics" - depends on DRM && PCI && MMU && (m || (y && KUNIT=y)) && 64BIT + depends on DRM && PCI && MMU && (m || (y && KUNIT=y)) I am curious about changes like this. Enabling 32-bit builds results in build failures for mips_allmodconfig because the driver redefines END. END is also used as macro in assembler code, the define happens to be included for mips builds, and it would be difficult to change it there. Unlike the i915 code, DRM_XE is not marked as depending on x86. This means it will be built for pretty much all "allmodconfig" builds for all architectures. Yet, there have been recent complaints about "allmodconfig" builds of drm code causing build failures on "oddball" architectures. Is there an assumption that DRM_XE (or DRM in general) is manually excluded from all architectures where it fails to build ? If so, would for all the reports we've been receiving we fixed the build and improved CI to try to avoid the regressions. DRM_XE doesn't really depend on x86, but I see your point of filtering out the "oddball architectures" or just expose the ones we know it builds against. Yet, I don't see that approach done in the wild in other drivers. At least on the build side, we constantly check the reports from lkp like https://lore.kernel.org/all/202403152008.klwyyggo-...@intel.com/ which also includes mips: mips allnoconfig gcc mips allyesconfig gcc is that not sufficient? allyesconfig should be covering it afaics All I can say is that drivers/gpu/drm/xe/xe_lrc.c doesn't build for mips builds in the mainline kernel. This is for both allmodconfig and allyesconfig. Both automatically build 32-bit kernels. Those builds are only enabled with this commit. The problem is also seen with 64-bit builds, but those are not enabled with allmodconfig/alldefconfig. I don't know what and how exactly 0-day runs its tests, but the failure is also seen in the drm-xe-next branch. I am going to blame this on my environment and just exclude DRM_XE from mips test builds going forward. Thanks, Guenter
[PATCH] drm/sun4i: Drop COMPILE_TEST
Attempts to build the sun4i drm code on various architectures using gcc 11.x
or older fails with
ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
This is due to commit 358e76fd613a ("drm/sun4i: hdmi: Consolidate
atomic_check and mode_valid") which introduces a constant 64-bit divide
operation. Some compilers / compiler versions fail to translate this
operation into fixed code.
Manual exclusion lists such as "Only build test this code on this subset of
architectures" or "Do not test this code on this set of architectures"
do not scale. Remove COMPILE_TEST support for the suni4 drm driver instead
to ensure that test builds are only performed on supported architectures.
Fixes: 358e76fd613a ("drm/sun4i: hdmi: Consolidate atomic_check and mode_valid")
Signed-off-by: Guenter Roeck
---
drivers/gpu/drm/sun4i/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 4741d9f6544c..015539bfda2a 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -2,7 +2,7 @@
config DRM_SUN4I
tristate "DRM Support for Allwinner A10 Display Engine"
depends on DRM && COMMON_CLK
- depends on ARCH_SUNXI || COMPILE_TEST
+ depends on ARCH_SUNXI
select DRM_GEM_DMA_HELPER
select DRM_KMS_HELPER
select DRM_PANEL
--
2.39.2
Reporting (or not reporting) drm test build failures
Hi, recently there was a suggestion that drm build tests on architectures such as xtensa should not happen or not be reported. The current mainline kernel experiences a number of drm related build failures. Building csky:allmodconfig ... failed -- Error log: ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined! make[3]: [scripts/Makefile.modpost:145: Module.symvers] Error 1 (ignored) [ also seen with xtensa] Building mips:allmodconfig ... failed -- Error log: drivers/gpu/drm/xe/xe_lrc.c:100: error: "END" redefined I really don't want to waste people's time if reporting such problems is considered noise/nuisance. Is there some guidance available explaining which architectures and/or platforms are "fair game" for drm build tests, or, better, which platforms/architectures should explicitly _not_ build test the drm subsystem ? Thanks, Guenter
Re: [PATCH 00/14] Add support for suppressing warning backtraces
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 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
Re: [PATCH v2 1/4] drm/msm/dp: Avoid a long timeout for AUX transfer if nothing connected
On Fri, Mar 15, 2024 at 2:37 PM Douglas Anderson wrote:
>
> As documented in the description of the transfer() function of
> "struct drm_dp_aux", the transfer() function can be called at any time
> regardless of the state of the DP port. Specifically if the kernel has
> the DP AUX character device enabled and userspace accesses
> "/dev/drm_dp_auxN" directly then the AUX transfer function will be
> called regardless of whether a DP device is connected.
>
> For eDP panels we have a special rule where we wait (with a 5 second
> timeout) for HPD to go high. This rule was important before all panels
> drivers were converted to call wait_hpd_asserted() and actually can be
> removed in a future commit.
>
> For external DP devices we never checked for HPD. That means that
> trying to access the DP AUX character device (AKA `hexdump -C
> /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my
> system:
> $ time hexdump -C /dev/drm_dp_aux0
> hexdump: /dev/drm_dp_aux0: Connection timed out
> real0m8.200s
> We want access to the drm_dp_auxN character device to fail faster than
> 8 seconds when no DP cable is plugged in.
>
> Let's add a test to make transfers fail right away if a device isn't
> plugged in. Rather than testing the HPD line directly, we have the
> dp_display module tell us when AUX transfers should be enabled so we
> can handle cases where HPD is signaled out of band like with Type C.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Douglas Anderson
Reviewed-by: Guenter Roeck
> ---
>
> Changes in v2:
> - Don't look at the HPD line directly; have dp_display call us.
>
> drivers/gpu/drm/msm/dp/dp_aux.c | 20
> drivers/gpu/drm/msm/dp/dp_aux.h | 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 4
> 3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
> index 03f4951c49f4..e67a80d56948 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.c
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.c
> @@ -35,6 +35,7 @@ struct dp_aux_private {
> bool no_send_stop;
> bool initted;
> bool is_edp;
> + bool enable_xfers;
> u32 offset;
> u32 segment;
>
> @@ -301,6 +302,17 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
> goto exit;
> }
>
> + /*
> +* If we're using DP and an external display isn't connected then the
> +* transfer won't succeed. Return right away. If we don't do this we
> +* can end up with long timeouts if someone tries to access the DP AUX
> +* character device when no DP device is connected.
> +*/
> + if (!aux->is_edp && !aux->enable_xfers) {
> + ret = -ENXIO;
> + goto exit;
> + }
> +
> /*
> * For eDP it's important to give a reasonably long wait here for HPD
> * to be asserted. This is because the panel driver may have _just_
> @@ -433,6 +445,14 @@ irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux)
> return IRQ_HANDLED;
> }
>
> +void dp_aux_enable_xfers(struct drm_dp_aux *dp_aux, bool enabled)
> +{
> + struct dp_aux_private *aux;
> +
> + aux = container_of(dp_aux, struct dp_aux_private, dp_aux);
> + aux->enable_xfers = enabled;
> +}
> +
> void dp_aux_reconfig(struct drm_dp_aux *dp_aux)
> {
> struct dp_aux_private *aux;
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h
> index 511305da4f66..f3052cb43306 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.h
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.h
> @@ -12,6 +12,7 @@
> int dp_aux_register(struct drm_dp_aux *dp_aux);
> void dp_aux_unregister(struct drm_dp_aux *dp_aux);
> irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux);
> +void dp_aux_enable_xfers(struct drm_dp_aux *dp_aux, bool enabled);
> void dp_aux_init(struct drm_dp_aux *dp_aux);
> void dp_aux_deinit(struct drm_dp_aux *dp_aux);
> void dp_aux_reconfig(struct drm_dp_aux *dp_aux);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 4c72124ffb5d..b0f3e2ef5a6b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -565,6 +565,8 @@ static int dp_hpd_plug_handle(struct dp_display_private
> *dp, u32 data)
> int ret;
> struct platform_device *pdev = dp->dp_display.pdev;
>
> + dp_aux_enable_xfers(dp->aux, true);
> +
> mutex_lock(>event_mutex);
>
> state = dp->hpd_state;
> @@ -629,6 +631,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private
> *dp, u32 data)
> u32 state;
> struct platform_device *pdev = dp->dp_display.pdev;
>
> + dp_aux_enable_xfers(dp->aux, false);
> +
> mutex_lock(>event_mutex);
>
> state = dp->hpd_state;
> --
> 2.44.0.291.gc1ea87d7ee-goog
>
Re: [PATCH 00/14] Add support for suppressing warning backtraces
On 3/14/24 08:02, Maxime Ripard wrote: On Thu, Mar 14, 2024 at 07:37:13AM -0700, 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 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. + How are tests using that API supposed to handle it then? Select the config option or put an ifdef? If the former, we end up in the same situation than without the symbol. If the latter, we end up in a similar situation than disabling KUNIT entirely, with some tests not being run which is just terrible. The API definitions are themselves within #ifdef and dummies if KUNIT_SUPPRESS_BACKTRACE (currently CONFIG_KUNIT) is disabled. In include/kunit/bug.h: #ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE ... #else #define DEFINE_SUPPRESSED_WARNING(func) #define START_SUPPRESSED_WARNING(func) #define END_SUPPRESSED_WARNING(func) #define IS_SUPPRESSED_WARNING(func) (false) #define SUPPRESSED_WARNING_COUNT(func) (0) #endif Only difference to the current patch series would be - #if IS_ENABLED(CONFIG_KUNIT) + #ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE in that file and elsewhere. With KUNIT_SUPPRESS_BACKTRACE=n you'd still get warning backtraces triggered by the tests, but the number of tests executed as well as the test results would still be the same. Thanks, Guenter
Re: [PATCH 00/14] Add support for suppressing warning backtraces
On 3/14/24 06:36, Geert Uytterhoeven wrote: Hi Günter, On Tue, Mar 12, 2024 at 6:03 PM Guenter Roeck 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. + Thanks, Guenter
Re: [PATCH 11/14] s390: Add support for suppressing warning backtraces
On 3/14/24 00:57, Geert Uytterhoeven wrote:
Hi Günter,
On Tue, Mar 12, 2024 at 6:06 PM 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 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).
Signed-off-by: Guenter Roeck
Thanks for your patch!
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -8,19 +8,30 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .long %0-.\n"
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#define __EMIT_BUG(x) do { \
asm_inline volatile(\
"0: mc 0,0\n" \
".section .rodata.str,\"aMS\",@progbits,1\n"\
"1: .asciz \""__FILE__"\"\n" \
".previous\n" \
- ".section __bug_table,\"awM\",@progbits,%2\n" \
+ ".section __bug_table,\"awM\",@progbits,%3\n" \
This change conflicts with commit 3938490e78f443fb ("s390/bug:
remove entry size from __bug_table section") in linus/master.
I guess it should just be dropped?
Yes, I know. I'll send v2 rebased to v6.9-rc1 once it is available and,
yes, the change will be gone after that.
Thanks,
Guenter
Re: [PATCH 1/3] drm/msm/dp: Avoid a long timeout for AUX transfer if nothing connected
On Tue, Mar 12, 2024 at 5:14 PM Douglas Anderson wrote:
>
> As documented in the description of the transfer() function of
> "struct drm_dp_aux", the transfer() function can be called at any time
> regardless of the state of the DP port. Specifically if the kernel has
> the DP AUX character device enabled and userspace accesses
> "/dev/drm_dp_auxN" directly then the AUX transfer function will be
> called regardless of whether a DP device is connected.
>
> For eDP panels we have a special rule where we wait (with a 5 second
> timeout) for HPD to go high. This rule was important before all panels
> drivers were converted to call wait_hpd_asserted() and actually can be
> removed in a future commit.
>
> For external DP devices we never checked for HPD. That means that
> trying to access the DP AUX character device (AKA `hexdump -C
> /dev/drm_dp_auxN`) would very, very slowly timeout. Specifically on my
> system:
> $ time hexdump -C /dev/drm_dp_aux0
> hexdump: /dev/drm_dp_aux0: Connection timed out
>
> real0m8.200s
>
> Let's add a check for HPD to avoid the slow timeout. This matches
> what, for instance, the intel_dp_aux_xfer() function does when it
> calls intel_tc_port_connected_locked(). That call has a document by it
> explaining that it's important to avoid the long timeouts.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Douglas Anderson
> ---
>
> drivers/gpu/drm/msm/dp/dp_aux.c | 8 +++-
> drivers/gpu/drm/msm/dp/dp_catalog.c | 10 ++
> drivers/gpu/drm/msm/dp/dp_catalog.h | 1 +
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
> index 03f4951c49f4..de0b0eabced9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.c
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.c
> @@ -307,7 +307,8 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
> * turned on the panel and then tried to do an AUX transfer. The panel
> * driver has no way of knowing when the panel is ready, so it's up
> * to us to wait. For DP we never get into this situation so let's
> -* avoid ever doing the extra long wait for DP.
> +* avoid ever doing the extra long wait for DP and just query HPD
> +* directly.
> */
> if (aux->is_edp) {
> ret = dp_catalog_aux_wait_for_hpd_connect_state(aux->catalog);
> @@ -315,6 +316,11 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
> DRM_DEBUG_DP("Panel not ready for aux
> transactions\n");
> goto exit;
> }
> + } else {
> + if (!dp_catalog_aux_is_hpd_connected(aux->catalog)) {
> + ret = -ENXIO;
> + goto exit;
> + }
> }
>
> dp_aux_update_offset_and_segment(aux, msg);
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index 5142aeb705a4..93e2d413a1e7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -266,6 +266,16 @@ int dp_catalog_aux_wait_for_hpd_connect_state(struct
> dp_catalog *dp_catalog)
> 2000, 50);
> }
>
> +bool dp_catalog_aux_is_hpd_connected(struct dp_catalog *dp_catalog)
> +{
> + struct dp_catalog_private *catalog = container_of(dp_catalog,
> + struct dp_catalog_private, dp_catalog);
> +
> + /* poll for hpd connected status every 2ms and timeout after 500ms */
Maybe I am missing something, but the comment doesn't seem to match
the code below.
Guenter
> + return readl(catalog->io->dp_controller.aux.base +
> REG_DP_DP_HPD_INT_STATUS) &
> + DP_DP_HPD_STATE_STATUS_CONNECTED;
> +}
> +
> static void dump_regs(void __iomem *base, int len)
> {
> int i;
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
> b/drivers/gpu/drm/msm/dp/dp_catalog.h
> index 38786e855b51..1694040c530f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
> @@ -86,6 +86,7 @@ void dp_catalog_aux_reset(struct dp_catalog *dp_catalog);
> void dp_catalog_aux_enable(struct dp_catalog *dp_catalog, bool enable);
> void dp_catalog_aux_update_cfg(struct dp_catalog *dp_catalog);
> int dp_catalog_aux_wait_for_hpd_connect_state(struct dp_catalog *dp_catalog);
> +bool dp_catalog_aux_is_hpd_connected(struct dp_catalog *dp_catalog);
> u32 dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog);
>
> /* DP Controller APIs */
> --
> 2.44.0.278.ge034bb2e1d-goog
>
[PATCH 12/14] sh: Add support for suppressing warning backtraces
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 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).
Signed-off-by: Guenter Roeck
---
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..cadc335eb759 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
+
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR"\t.long %O2\n"
+#else
+# define __BUG_FUNC_PTR
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#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_FUNCNULL
+#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))); \
--
2.39.2
[PATCH 14/14] powerpc: Add support for suppressing warning backtraces
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 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).
Signed-off-by: Guenter Roeck
---
arch/powerpc/include/asm/bug.h | 37 +-
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 1db485aacbd9..330d4983f90e 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -14,6 +14,9 @@
.section __bug_table,"aw"
5001: .4byte \addr - .
.4byte 5002f - .
+#if IS_ENABLED(CONFIG_KUNIT)
+.4byte 0
+#endif
.short \line, \flags
.org 5001b+BUG_ENTRY_SIZE
.previous
@@ -32,30 +35,46 @@
#endif /* verbose */
#else /* !__ASSEMBLY__ */
-/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
- sizeof(struct bug_entry), respectively */
+/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3,%4 to be FILE, __func__, LINE,
flags
+ and sizeof(struct bug_entry), respectively */
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .4byte %1 - .\n"
+#else
+# define __BUG_FUNC_PTR
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte %0 - .\n"\
- " .short %1, %2\n"\
- ".org 2b+%3\n" \
+ __BUG_FUNC_PTR \
+ " .short %2, %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#else
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
- " .short %2\n"\
- ".org 2b+%3\n" \
+ " .short %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#endif
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define BUG_ENTRY(insn, flags, ...)\
__asm__ __volatile__( \
"1: " insn "\n" \
_EMIT_BUG_ENTRY \
- : : "i" (__FILE__), "i" (__LINE__), \
+ : : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
##__VA_ARGS__)
@@ -80,7 +99,7 @@
if (x) \
BUG(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "r" ((__force long)(x)));
\
+ BUG_ENTRY(PPC_TLNEI " %5, 0", 0, "r" ((__force long)(x)));
\
} \
} while (0)
@@ -90,7 +109,7 @@
if (__ret_warn_on) \
__WARN(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", \
+ BUG_ENTRY(PPC_TLNEI " %5, 0", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
"r" (__ret_warn_on)); \
} \
--
2.39.2
[PATCH 13/14] riscv: Add support for suppressing warning backtraces
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 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.
Signed-off-by: Guenter Roeck
---
arch/riscv/include/asm/bug.h | 38
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
index 1aaea81fb141..8955ee5c1c27 100644
--- a/arch/riscv/include/asm/bug.h
+++ b/arch/riscv/include/asm/bug.h
@@ -30,26 +30,39 @@
typedef u32 bug_insn_t;
#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
-#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ."
-#define __BUG_ENTRY_FILE RISCV_INT " %0 - ."
+#define __BUG_REL(val) RISCV_INT " " __stringify(val) " - ."
#else
-#define __BUG_ENTRY_ADDR RISCV_PTR " 1b"
-#define __BUG_ENTRY_FILE RISCV_PTR " %0"
+#define __BUG_REL(val) RISCV_PTR " " __stringify(val)
#endif
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%1)
+#else
+# define __BUG_FUNC_PTR
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- __BUG_ENTRY_FILE "\n\t" \
- RISCV_SHORT " %1\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ __BUG_REL(%0) "\n\t"\
+ __BUG_FUNC_PTR "\n\t" \
+ RISCV_SHORT " %2\n\t" \
+ RISCV_SHORT " %3"
#else
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ RISCV_SHORT " %3"
#endif
#ifdef CONFIG_GENERIC_BUG
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
do { \
__asm__ __volatile__ ( \
@@ -58,10 +71,11 @@ do {
\
".pushsection __bug_table,\"aw\"\n\t" \
"2:\n\t"\
__BUG_ENTRY "\n\t" \
- ".org 2b + %3\n\t" \
+ ".org 2b + %4\n\t" \
".popsection" \
: \
- : "i" (__FILE__), "i" (__LINE__), \
+ : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
--
2.39.2
[PATCH 11/14] s390: Add support for suppressing warning backtraces
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 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).
Signed-off-by: Guenter Roeck
---
arch/s390/include/asm/bug.h | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index aebe1e22c7be..01e2aa4069d7 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -8,19 +8,30 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .long %0-.\n"
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#define __EMIT_BUG(x) do { \
asm_inline volatile(\
"0: mc 0,0\n" \
".section .rodata.str,\"aMS\",@progbits,1\n"\
"1: .asciz \""__FILE__"\"\n" \
".previous\n" \
- ".section __bug_table,\"awM\",@progbits,%2\n" \
+ ".section __bug_table,\"awM\",@progbits,%3\n" \
"2: .long 0b-.\n" \
" .long 1b-.\n" \
- " .short %0,%1\n"\
- " .org2b+%2\n"\
+ __BUG_FUNC_PTR \
+ " .short %1,%2\n"\
+ " .org2b+%3\n"\
".previous\n" \
- : : "i" (__LINE__), \
+ : : "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (x),\
"i" (sizeof(struct bug_entry)));\
} while (0)
--
2.39.2
[PATCH 04/14] kunit: Add documentation for warning backtrace suppression API
Document API functions for suppressing warning backtraces.
Signed-off-by: Guenter Roeck
---
Documentation/dev-tools/kunit/usage.rst | 30 -
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst
b/Documentation/dev-tools/kunit/usage.rst
index 22955d56b379..8d3d36d4103d 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -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.
- }
\ No newline at end of file
+ }
--
2.39.2
[PATCH 10/14] parisc: Add support for suppressing warning backtraces
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 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.
Signed-off-by: Guenter Roeck
---
arch/parisc/include/asm/bug.h | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 833555f74ffa..792dacc2a653 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -23,8 +23,17 @@
# define __BUG_REL(val) ".word " __stringify(val)
#endif
-
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#define BUG() \
do {\
asm volatile("\n" \
@@ -33,10 +42,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %c3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (0), "i" (sizeof(struct bug_entry)) ); \
unreachable(); \
} while(0)
@@ -58,10 +69,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
} while(0)
@@ -74,7 +87,7 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t.short %0\n"\
-"\t.blockz %1-4-2\n" \
+"\t.blockz %c1-(.-2b)\n" \
"\t.popsection"\
: : "i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
--
2.39.2
[PATCH 09/14] loongarch: Add support for suppressing warning backtraces
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 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).
Signed-off-by: Guenter Roeck
---
arch/loongarch/include/asm/bug.h | 38 +++-
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index d4ca3ba25418..25f2b5ae7702 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -3,47 +3,63 @@
#define __ASM_BUG_H
#include
+#include
#include
#ifndef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#else
-#define __BUGVERBOSE_LOCATION(file, line) \
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str, "aMS", @progbits, 1; \
10002: .string file; \
.popsection;\
\
.long 10002b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
#endif
#ifndef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#else
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table, "aw"; \
.align 2; \
1: .long 10001f - .; \
- _BUGVERBOSE_LOCATION(__FILE__, __LINE__)\
+ _BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
.short flags; \
.popsection;\
10001:
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
break BRK_BUG
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
+
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
#define __BUG_FLAGS(flags) \
- asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags, %0)) : : "i"
(__BUG_FUNC));
#define __WARN_FLAGS(flags)\
do { \
instrumentation_begin();\
- __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH 07/14] x86: Add support for suppressing warning backtraces
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 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).
Signed-off-by: Guenter Roeck
---
arch/x86/include/asm/bug.h | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index a3ec87d198ac..d87bf8bab238 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -23,18 +23,28 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#define _BUG_FLAGS(ins, flags, extra) \
do { \
asm_inline volatile("1:\t" ins "\n" \
".pushsection __bug_table,\"aw\"\n"\
"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
"\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
-"\t.word %c1""\t# bug_entry::line\n" \
-"\t.word %c2""\t# bug_entry::flags\n" \
-"\t.org 2b+%c3\n" \
+"\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
+"\t.word %c2""\t# bug_entry::line\n" \
+"\t.word %c3""\t# bug_entry::flags\n" \
+"\t.org 2b+%c4\n" \
".popsection\n"\
extra \
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
@@ -80,7 +90,8 @@ do {
\
do { \
__auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin();\
- _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH 08/14] arm64: Add support for suppressing warning backtraces
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 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).
Signed-off-by: Guenter Roeck
---
arch/arm64/include/asm/asm-bug.h | 29 +++--
arch/arm64/include/asm/bug.h | 8 +++-
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index c762038ba400..6884089a7191 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -8,36 +8,45 @@
#include
#ifdef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
-#define __BUGVERBOSE_LOCATION(file, line) \
+
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str,"aMS",@progbits,1; \
14472: .string file; \
.popsection;\
\
.long 14472b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
#else
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#endif
#ifdef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table,"aw"; \
.align 2; \
14470: .long 14471f - .; \
-_BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
- .short flags; \
+_BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
+ .short flags; \
.popsection;\
14471:
#else
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
brk BUG_BRK_IMM
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
#endif /* __ASM_ASM_BUG_H */
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 28be048db3f6..044c5e24a17d 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -11,8 +11,14 @@
#include
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
- asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i"
(__BUG_FUNC));
#define BUG() do { \
__BUG_FLAGS(0); \
--
2.39.2
[PATCH 06/14] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests
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.
Cc: David Gow
Cc: Jakub Kicinski
Signed-off-by: Guenter Roeck
---
net/core/dev_addr_lists_test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index 4dbd0dc6aea2..b427dd1a3c93 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#include
+#include
#include
#include
#include
@@ -49,6 +50,7 @@ static int dev_addr_test_init(struct kunit *test)
KUNIT_FAIL(test, "Can't register netdev %d", err);
}
+ debug_locks_silent = 1;
rtnl_lock();
return 0;
}
@@ -56,8 +58,12 @@ static int dev_addr_test_init(struct kunit *test)
static void dev_addr_test_exit(struct kunit *test)
{
struct net_device *netdev = test->priv;
+ DEFINE_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ START_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
rtnl_unlock();
+ END_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ debug_locks_silent = 0;
unregister_netdev(netdev);
free_netdev(netdev);
}
--
2.39.2
[PATCH 03/14] kunit: Add test cases for backtrace warning suppression
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.
Signed-off-by: Guenter Roeck
---
lib/kunit/Makefile | 3 +-
lib/kunit/backtrace-suppression-test.c | 104 +
2 files changed, 106 insertions(+), 1 deletion(-)
create mode 100644 lib/kunit/backtrace-suppression-test.c
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 545b57c3be48..6a44d2b54ea9 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -19,7 +19,8 @@ endif
obj-y += hooks.o \
bug.o
-obj-$(CONFIG_KUNIT_TEST) +=kunit-test.o
+obj-$(CONFIG_KUNIT_TEST) +=kunit-test.o \
+ backtrace-suppression-test.o
# string-stream-test compiles built-in only.
ifeq ($(CONFIG_KUNIT_TEST),y)
diff --git a/lib/kunit/backtrace-suppression-test.c
b/lib/kunit/backtrace-suppression-test.c
new file mode 100644
index ..47c619283802
--- /dev/null
+++ b/lib/kunit/backtrace-suppression-test.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for suppressing warning tracebacks
+ *
+ * Copyright (C) 2024, Guenter Roeck
+ * Author: Guenter Roeck
+ */
+
+#include
+#include
+
+static void backtrace_suppression_test_warn_direct(struct kunit *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);
+}
+
+static void trigger_backtrace_warn(void)
+{
+ WARN(1, "This backtrace should be suppressed");
+}
+
+static void backtrace_suppression_test_warn_indirect(struct kunit *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);
+}
+
+static void backtrace_suppression_test_warn_multi(struct kunit *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);
+}
+
+static void backtrace_suppression_test_warn_on_direct(struct kunit *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);
+}
+
+static void trigger_backtrace_warn_on(void)
+{
+ WARN_ON(1);
+}
+
+static void backtrace_suppression_test_warn_on_indirect(struct kunit *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);
+}
+
+static struct kunit_case backtrace_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),
+ {}
+};
+
+static struct kunit_suite backtrace_suppression_test_suite = {
+ .name = "backtrace-suppression-test",
+ .test_cases = backtrace_su
[PATCH 05/14] drm: Suppress intentional warning backtraces in scaling unit tests
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.
Signed-off-by: Guenter Roeck
---
drivers/gpu/drm/tests/drm_rect_test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
b/drivers/gpu/drm/tests/drm_rect_test.c
index 76332cd2ead8..75614cb4deb5 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -406,22 +406,28 @@ KUNIT_ARRAY_PARAM(drm_rect_scale, drm_rect_scale_cases,
drm_rect_scale_case_desc
static void drm_test_rect_calc_hscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_hscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
static void drm_test_rect_calc_vscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_vscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
--
2.39.2
[PATCH 02/14] kunit: bug: Count suppressed warning backtraces
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.
Signed-off-by: Guenter Roeck
---
include/kunit/bug.h | 7 ++-
lib/kunit/bug.c | 4 +++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
index 1e34da961599..2097a854ac8c 100644
--- a/include/kunit/bug.h
+++ b/include/kunit/bug.h
@@ -20,6 +20,7 @@
struct __suppressed_warning {
struct list_head node;
const char *function;
+ int counter;
};
void __start_suppress_warning(struct __suppressed_warning *warning);
@@ -28,7 +29,7 @@ bool __is_suppressed_warning(const char *function);
#define DEFINE_SUPPRESSED_WARNING(func)\
struct __suppressed_warning __kunit_suppress_##func = \
- { .function = __stringify(func) }
+ { .function = __stringify(func), .counter = 0 }
#define START_SUPPRESSED_WARNING(func) \
__start_suppress_warning(&__kunit_suppress_##func)
@@ -39,12 +40,16 @@ bool __is_suppressed_warning(const char *function);
#define IS_SUPPRESSED_WARNING(func) \
__is_suppressed_warning(func)
+#define SUPPRESSED_WARNING_COUNT(func) \
+ (__kunit_suppress_##func.counter)
+
#else /* CONFIG_KUNIT */
#define DEFINE_SUPPRESSED_WARNING(func)
#define START_SUPPRESSED_WARNING(func)
#define END_SUPPRESSED_WARNING(func)
#define IS_SUPPRESSED_WARNING(func) (false)
+#define SUPPRESSED_WARNING_COUNT(func) (0)
#endif /* CONFIG_KUNIT */
#endif /* __ASSEMBLY__ */
diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
index f93544d7a9d1..13b3d896c114 100644
--- a/lib/kunit/bug.c
+++ b/lib/kunit/bug.c
@@ -32,8 +32,10 @@ bool __is_suppressed_warning(const char *function)
return false;
list_for_each_entry(warning, _warnings, node) {
- if (!strcmp(function, warning->function))
+ if (!strcmp(function, warning->function)) {
+ warning->counter++;
return true;
+ }
}
return false;
}
--
2.39.2
[PATCH 01/14] bug/kunit: Core support for suppressing warning backtraces
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.
Cc: Dan Carpenter
Cc: Daniel Diaz
Cc: Naresh Kamboju
Cc: Kees Cook
Signed-off-by: Guenter Roeck
---
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/Makefile| 6 +++--
lib/kunit/bug.c | 40 ++
7 files changed, 169 insertions(+), 9 deletions(-)
create mode 100644 include/kunit/bug.h
create mode 100644 lib/kunit/bug.c
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 6e794420bd39..c170b6477689 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -18,6 +18,7 @@
#endif
#ifndef __ASSEMBLY__
+#include
#include
#include
@@ -39,8 +40,14 @@ struct bug_entry {
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
const char *file;
+#ifdef HAVE_BUG_FUNCTION
+ const char *function;
+#endif
#else
signed int file_disp;
+#ifdef HAVE_BUG_FUNCTION
+ signed int function_disp;
+#endif
#endif
unsigned short line;
#endif
@@ -96,15 +103,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt,
...);
#define __WARN() __WARN_printf(TAINT_WARN, NULL)
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- warn_slowpath_fmt(__FILE__, __LINE__, taint, arg); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);\
instrumentation_end(); \
} while (0)
#else
#define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- __warn_printk(arg); \
- __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+ if (!IS_SUPPRESSED_WARNING(__func__)) { \
+ __warn_printk(arg); \
+ __WARN_FLAGS(BUGFLAG_NO_CUT_HERE |
BUGFLAG_TAINT(taint));\
+ } \
instrumentation_end(); \
} while (0)
#define WARN_ON_ONCE(condition) ({ \
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
new file mode 100644
index ..1e34da961599
--- /dev/null
+++ b/include/kunit/bug.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck
+ */
+
+#ifndef _KUNIT_BUG_H
+#define _KUNIT_BUG_H
+
+#ifndef __ASSEMBLY__
+
+#include
+
+#if IS_ENABLED(CONFIG_KUNIT)
+
+#include
+#include
+
+struct __suppressed_warning {
+ struct list_head node;
+ const char *function;
+};
+
+void __start_suppress_warning(struct __suppressed_warning *warning);
+void __end_suppress_warning(struct __suppressed_warning *warning);
+bool __is_suppressed_warning(const char *function);
+
+#define DEFINE_SUPPRESSED_WARNING(func)\
+ struct __suppressed_warning __kunit_suppress_##func = \
+ { .function = __stringify(func) }
+
+#define START_SUPPRESSED_WARNING(func) \
+ __start_suppress_warning(&__kunit_suppress_##func)
+
+#define END_SUPPRESSED_WARNING(func) \
+ __end_suppress_warning(&__kunit_suppress_##func)
+
+#define IS_SUPPR
[PATCH 00/14] Add support for suppressing warning backtraces
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. 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 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. 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: - 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
Re: [PATCH] drm/i915/hwmon: Fix locking inversion in sysfs getter
On 3/11/24 09:58, Rodrigo Vivi wrote:
On Mon, Mar 11, 2024 at 09:06:46AM +0100, Janusz Krzysztofik wrote:
In i915 hwmon sysfs getter path we now take a hwmon_lock, then acquire an
rpm wakeref. That results in lock inversion:
<4> [197.079335] ==
<4> [197.085473] WARNING: possible circular locking dependency detected
<4> [197.091611] 6.8.0-rc7-Patchwork_129026v7-gc4dc92fb1152+ #1 Not tainted
<4> [197.098096] --
<4> [197.104231] prometheus-node/839 is trying to acquire lock:
<4> [197.109680] 82764d80 (fs_reclaim){+.+.}-{0:0}, at:
__kmalloc+0x9a/0x350
<4> [197.116939]
but task is already holding lock:
<4> [197.122730] 88811b772a40 (>hwmon_lock){+.+.}-{3:3}, at:
hwm_energy+0x4b/0x100 [i915]
<4> [197.131543]
which lock already depends on the new lock.
...
<4> [197.507922] Chain exists of:
fs_reclaim --> >reset.mutex --> >hwmon_lock
<4> [197.518528] Possible unsafe locking scenario:
<4> [197.524411]CPU0CPU1
<4> [197.528916]
<4> [197.533418] lock(>hwmon_lock);
<4> [197.537237]lock(>reset.mutex);
<4> [197.543376]lock(>hwmon_lock);
<4> [197.549682] lock(fs_reclaim);
...
<4> [197.632548] Call Trace:
<4> [197.634990]
<4> [197.637088] dump_stack_lvl+0x64/0xb0
<4> [197.640738] check_noncircular+0x15e/0x180
<4> [197.652968] check_prev_add+0xe9/0xce0
<4> [197.656705] __lock_acquire+0x179f/0x2300
<4> [197.660694] lock_acquire+0xd8/0x2d0
<4> [197.673009] fs_reclaim_acquire+0xa1/0xd0
<4> [197.680478] __kmalloc+0x9a/0x350
<4> [197.689063] acpi_ns_internalize_name.part.0+0x4a/0xb0
<4> [197.694170] acpi_ns_get_node_unlocked+0x60/0xf0
<4> [197.720608] acpi_ns_get_node+0x3b/0x60
<4> [197.724428] acpi_get_handle+0x57/0xb0
<4> [197.728164] acpi_has_method+0x20/0x50
<4> [197.731896] acpi_pci_set_power_state+0x43/0x120
<4> [197.736485] pci_power_up+0x24/0x1c0
<4> [197.740047] pci_pm_default_resume_early+0x9/0x30
<4> [197.744725] pci_pm_runtime_resume+0x2d/0x90
<4> [197.753911] __rpm_callback+0x3c/0x110
<4> [197.762586] rpm_callback+0x58/0x70
<4> [197.766064] rpm_resume+0x51e/0x730
<4> [197.769542] rpm_resume+0x267/0x730
<4> [197.773020] rpm_resume+0x267/0x730
<4> [197.776498] rpm_resume+0x267/0x730
<4> [197.779974] __pm_runtime_resume+0x49/0x90
<4> [197.784055] __intel_runtime_pm_get+0x19/0xa0 [i915]
<4> [197.789070] hwm_energy+0x55/0x100 [i915]
<4> [197.793183] hwm_read+0x9a/0x310 [i915]
<4> [197.797124] hwmon_attr_show+0x36/0x120
<4> [197.800946] dev_attr_show+0x15/0x60
<4> [197.804509] sysfs_kf_seq_show+0xb5/0x100
However, the lock is only intended to protect either a hwmon overflow
counter or rmw hardware operations. There is no need to hold the lock,
only the wakeref, while reading from hardware.
Acquire the lock after hardware read under rpm wakeref.
Fixes: c41b8bdcc297 ("drm/i915/hwmon: Show device level energy usage")
Signed-off-by: Janusz Krzysztofik
Cc: # v6.2+
---
drivers/gpu/drm/i915/i915_hwmon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c
b/drivers/gpu/drm/i915/i915_hwmon.c
index 8c3f443c8347e..faf7670de6e06 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -136,11 +136,11 @@ hwm_energy(struct hwm_drvdata *ddat, long *energy)
else
rgaddr = hwmon->rg.energy_status_all;
- mutex_lock(>hwmon_lock);
-
with_intel_runtime_pm(uncore->rpm, wakeref)
reg_val = intel_uncore_read(uncore, rgaddr);
+ mutex_lock(>hwmon_lock);
+
This is not enough.
check hwm_locked_with_pm_intel_uncore_rmw()
It looks like we need to rethink this lock entirely here.
I would have assumed that the lock was supposed to ensure that
reading the register value and the subsequent update of accum_energy
and reg_val_prev was synchronized. This is no longer the case
after this patch has been applied. Given that, I would agree that
it would make sense to define what the lock is supposed to protect
before changing its scope.
Guenter
Re: [RFC PATCH 0/5] Add support for suppressing warning backtraces
On Tue, Mar 05, 2024 at 10:40:28AM -0800, Guenter Roeck 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 marks the warning message in drm_calc_scale() in the DRM > subsystem as intentional where warranted. This patch is intended to serve > as an example for the use of the functionality introduced with this series. > The last three patches in the series introduce the necessary architecture > specific changes for x86, arm64, and loongarch. > > 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. > > 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. > > Some questions: > > - Is the general approach promising ? If not, are there other possible > solutions ? > - Function pointers are only added to the __bug_table section if both > CONFIG_KUNIT and CONFIG_DEBUG_BUGVERBOSE are enabled. This avoids image > size increases if CONFIG_KUNIT=n. Downside is slightly more complex > architecture specific assembler code. If function pointers were always > added to the __bug_table section, vmlinux image size would increase by > approximately 0.6-0.7%. Is the increased complexity in assembler code > worth the reduced image size ? I think so, but I would like to hear > other opinions. > - There are additional possibilities associated with storing the bug > function name in the __bug_table section. It could be independent of > KUNIT, it could be a configuration flag, and/or it could be used to > display the name of the offending function in BUG/WARN messages. > Is any of those of interest ? > I am ready to send a full version of this series with support for all affected architectures. I am undecided if I should send it now, based on v6.8, and send v2 after rebasing it to v6.9-rc1, or if I should just wait for v6.9-rc1. I understand that some maintainers dislike getting new patch series while the commit window is is open. On the ther side, I tested the series thoroughly on top of v6.8-rc7, and initial v6.9 release candidates may have their own problems. Given that, I tend to send the series now. Any thoughts ? Unless there is strong negative feedback, I'll likely do that in a day or two. Thanks, Guenter
Re: [RFC PATCH 0/5] Add support for suppressing warning backtraces
Hi Daniel, On 3/6/24 10:24, Daniel Díaz wrote: [ ... ] Thank you SO very much for this work! This is very much appreciated! Thanks a lot for the feedback. We run into these warnings at LKFT all the time, and making sure that the noise doesn't drown the relevant signal is very important. Can you send me a list of all the warnings you are seeing ? I do see lots of warnings when running drm tests in qemu, but I am not sure if those are caused by emulation problems or if they are expected. A list of warnings seen on real hardware would help me prepare additional patches to address (or, rather, suppress) those. Thanks, Guenter
Re: [RFC PATCH 1/5] bug: Core support for suppressing warning backtraces
On 3/5/24 11:54, Kees Cook wrote: On Tue, Mar 05, 2024 at 10:40:29AM -0800, Guenter Roeck wrote: [...] warning = (bug->flags & BUGFLAG_WARNING) != 0; once = (bug->flags & BUGFLAG_ONCE) != 0; done = (bug->flags & BUGFLAG_DONE) != 0; + if (warning && IS_SUPPRESSED_WARNING(function)) + return BUG_TRAP_TYPE_WARN; + I had to re-read __report_bug() more carefully, but yes, this works -- it's basically leaving early, like "once" does. This looks like a reasonable approach! Something very similar to this is checking that a warning happens. i.e. you talk about drm selftests checking function return values, but I've got a bunch of tests (LKDTM) that live outside of KUnit because I haven't had a clean way to check for specific warnings/bugs. I feel like future changes built on top of this series could add counters or something that KUnit could examine. E.g. I did this manually for some fortify tests: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/hardening=4ce615e798a752d4431fcc52960478906dec2f0e Sounds like a good idea. It should be straightforward to add a counter to struct __suppressed_warning. This way the calling code could easily check if an expected warning backtrace actually happened. Thanks, Guenter
[RFC PATCH 4/5] arm64: Add support for suppressing warning tracebacks
Add support for selectively suppressing WARNING tracebacks to arm64.
This requires adding the function triggering tracebacks to the __bug_table
object section.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT 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).
Signed-off-by: Guenter Roeck
---
arch/arm64/include/asm/asm-bug.h | 29 +++--
arch/arm64/include/asm/bug.h | 8 +++-
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index c762038ba400..6884089a7191 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -8,36 +8,45 @@
#include
#ifdef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
-#define __BUGVERBOSE_LOCATION(file, line) \
+
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str,"aMS",@progbits,1; \
14472: .string file; \
.popsection;\
\
.long 14472b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
#else
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#endif
#ifdef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table,"aw"; \
.align 2; \
14470: .long 14471f - .; \
-_BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
- .short flags; \
+_BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
+ .short flags; \
.popsection;\
14471:
#else
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
brk BUG_BRK_IMM
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
#endif /* __ASM_ASM_BUG_H */
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 28be048db3f6..044c5e24a17d 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -11,8 +11,14 @@
#include
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
- asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i"
(__BUG_FUNC));
#define BUG() do { \
__BUG_FLAGS(0); \
--
2.39.2
[RFC PATCH 5/5] loongarch: Add support for suppressing warning tracebacks
Add support for selectively suppressing WARNING tracebacks to loongarch.
This requires adding the function triggering tracebacks to the __bug_table
object section.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT 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).
Signed-off-by: Guenter Roeck
---
arch/loongarch/include/asm/bug.h | 38 +++-
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index d4ca3ba25418..25f2b5ae7702 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -3,47 +3,63 @@
#define __ASM_BUG_H
#include
+#include
#include
#ifndef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#else
-#define __BUGVERBOSE_LOCATION(file, line) \
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str, "aMS", @progbits, 1; \
10002: .string file; \
.popsection;\
\
.long 10002b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
#endif
#ifndef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#else
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table, "aw"; \
.align 2; \
1: .long 10001f - .; \
- _BUGVERBOSE_LOCATION(__FILE__, __LINE__)\
+ _BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
.short flags; \
.popsection;\
10001:
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
break BRK_BUG
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
+
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
#define __BUG_FLAGS(flags) \
- asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags, %0)) : : "i"
(__BUG_FUNC));
#define __WARN_FLAGS(flags)\
do { \
instrumentation_begin();\
- __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
instrumentation_end(); \
} while (0)
--
2.39.2
[RFC PATCH 3/5] x86: Add support for suppressing warning tracebacks
Add support for selectively suppressing WARNING tracebacks to x86.
This requires adding the function triggering tracebacks to the __bug_table
object section.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT 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).
Signed-off-by: Guenter Roeck
---
arch/x86/include/asm/bug.h | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index a3ec87d198ac..8e45143fa676 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -21,6 +21,15 @@
# define __BUG_REL(val)".long " __stringify(val) " - ."
#endif
+#if IS_ENABLED(CONFIG_KUNIT)
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* IS_ENABLED(CONFIG_KUNIT) */
+
#ifdef CONFIG_DEBUG_BUGVERBOSE
#define _BUG_FLAGS(ins, flags, extra) \
@@ -29,12 +38,13 @@ do {
\
".pushsection __bug_table,\"aw\"\n"\
"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
"\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
-"\t.word %c1""\t# bug_entry::line\n" \
-"\t.word %c2""\t# bug_entry::flags\n" \
-"\t.org 2b+%c3\n" \
+"\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
+"\t.word %c2""\t# bug_entry::line\n" \
+"\t.word %c3""\t# bug_entry::flags\n" \
+"\t.org 2b+%c4\n" \
".popsection\n"\
extra \
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
@@ -80,7 +90,8 @@ do {
\
do { \
__auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin();\
- _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
instrumentation_end(); \
} while (0)
--
2.39.2
[RFC PATCH 2/5] drm: Suppress intentional warning backtraces in scaling unit tests
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.
Signed-off-by: Guenter Roeck
---
drivers/gpu/drm/tests/drm_rect_test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
b/drivers/gpu/drm/tests/drm_rect_test.c
index 76332cd2ead8..75614cb4deb5 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -406,22 +406,28 @@ KUNIT_ARRAY_PARAM(drm_rect_scale, drm_rect_scale_cases,
drm_rect_scale_case_desc
static void drm_test_rect_calc_hscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_hscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
static void drm_test_rect_calc_vscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_vscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
--
2.39.2
[RFC PATCH 1/5] bug: Core support for suppressing warning backtraces
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.
Cc: Dan Carpenter
Cc: Daniel Diaz
Cc: Naresh Kamboju
Cc: Kees Cook
Signed-off-by: Guenter Roeck
---
include/asm-generic/bug.h | 16 +---
include/kunit/bug.h | 51 +++
include/linux/bug.h | 13 ++
lib/bug.c | 51 ---
lib/kunit/Makefile| 6 +++--
lib/kunit/bug.c | 40 ++
6 files changed, 168 insertions(+), 9 deletions(-)
create mode 100644 include/kunit/bug.h
create mode 100644 lib/kunit/bug.c
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 6e794420bd39..b0069564eb8f 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -2,6 +2,7 @@
#ifndef _ASM_GENERIC_BUG_H
#define _ASM_GENERIC_BUG_H
+#include
#include
#include
#include
@@ -39,8 +40,14 @@ struct bug_entry {
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
const char *file;
+#ifdef HAVE_BUG_FUNCTION
+ const char *function;
+#endif
#else
signed int file_disp;
+#ifdef HAVE_BUG_FUNCTION
+ signed int function_disp;
+#endif
#endif
unsigned short line;
#endif
@@ -96,15 +103,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt,
...);
#define __WARN() __WARN_printf(TAINT_WARN, NULL)
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- warn_slowpath_fmt(__FILE__, __LINE__, taint, arg); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);\
instrumentation_end(); \
} while (0)
#else
#define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- __warn_printk(arg); \
- __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+ if (!IS_SUPPRESSED_WARNING(__func__)) { \
+ __warn_printk(arg); \
+ __WARN_FLAGS(BUGFLAG_NO_CUT_HERE |
BUGFLAG_TAINT(taint));\
+ } \
instrumentation_end(); \
} while (0)
#define WARN_ON_ONCE(condition) ({ \
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
new file mode 100644
index ..11b8ae795320
--- /dev/null
+++ b/include/kunit/bug.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck
+ */
+
+#ifndef _KUNIT_BUG_H
+#define _KUNIT_BUG_H
+
+#ifndef __ASSEMBLY__
+
+#include
+
+#if IS_ENABLED(CONFIG_KUNIT)
+
+#include
+#include
+
+struct __suppressed_warning {
+ struct list_head node;
+ const char *function;
+};
+
+void __start_suppress_warning(struct __suppressed_warning *warning);
+void __end_suppress_warning(struct __suppressed_warning *warning);
+bool __is_suppressed_warning(const char *function);
+
+#define DEFINE_SUPPRESSED_WARNING(func)\
+ struct __suppressed_warning __kunit_suppress_##func = \
+ { .function = __stringify(func) }
+
+#define START_SUPPRESSED_WARNING(func) \
+ __start_suppress_warning(&__kunit_suppress_##func)
+
+#define END_SUPPRESSED_WARNING(func) \
+ __end_suppress_warning(&__kunit_suppress_##func)
+
+#define IS_SUPPR
[RFC PATCH 0/5] Add support for suppressing warning backtraces
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 marks the warning message in drm_calc_scale() in the DRM subsystem as intentional where warranted. This patch is intended to serve as an example for the use of the functionality introduced with this series. The last three patches in the series introduce the necessary architecture specific changes for x86, arm64, and loongarch. 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. 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. Some questions: - Is the general approach promising ? If not, are there other possible solutions ? - Function pointers are only added to the __bug_table section if both CONFIG_KUNIT and CONFIG_DEBUG_BUGVERBOSE are enabled. This avoids image size increases if CONFIG_KUNIT=n. Downside is slightly more complex architecture specific assembler code. If function pointers were always added to the __bug_table section, vmlinux image size would increase by approximately 0.6-0.7%. Is the increased complexity in assembler code worth the reduced image size ? I think so, but I would like to hear other opinions. - There are additional possibilities associated with storing the bug function name in the __bug_table section. It could be independent of KUNIT, it could be a configuration flag, and/or it could be used to display the name of the offending function in BUG/WARN messages. Is any of those of interest ? -------- Guenter Roeck (5): bug: Core support for suppressing warning backtraces drm: Suppress intentional warning backtraces in scaling unit tests x86: Add support for suppressing warning tracebacks arm64: Add support for suppressing warning tracebacks loongarch: Add support for suppressing warning tracebacks arch/arm64/include/asm/asm-bug.h | 29 +--- arch/arm64/include/asm/bug.h | 8 +- arch/loongarch/include/asm/bug.h | 38 ++ arch/x86/include/asm/bug.h| 21 +++ drivers/gpu/drm/tests/drm_rect_test.c | 6 + include/asm-generic/bug.h | 16 --- include/kunit/bug.h | 51 +++ include/linux/bug.h | 13 + lib/bug.c | 51 --- lib/kunit/Makefile| 6 +++-- lib/kunit/bug.c | 40 +++ 11 files changed, 243 insertions(+), 36 deletions(-) create mode 100644 include/kunit/bug.h create mode 100644 lib/kunit/bug.c
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Mon, Mar 4, 2024 at 9:09 AM Maxime Ripard wrote: [ ...] > > And singling out DRM because it regularly allegedly breaks things on > xtensa or m68k and claiming we're not taking CI seriously because of it > is completely ridiculous. If the all the subsystems were taking CI as > seriously as DRM, we would be in a much better place. > FWIW: $ git grep CONFIG_DRM arch/xtensa/ arch/m68k/ arch/m68k/configs/virt_defconfig:CONFIG_DRM=y arch/m68k/configs/virt_defconfig:CONFIG_DRM_FBDEV_EMULATION=y arch/m68k/configs/virt_defconfig:CONFIG_DRM_VIRTIO_GPU=y arch/xtensa/configs/virt_defconfig:CONFIG_DRM=y arch/xtensa/configs/virt_defconfig:CONFIG_DRM_VGEM=y arch/xtensa/configs/virt_defconfig:CONFIG_DRM_VIRTIO_GPU=y If that isn't supported, it might really make sense to declare that CONFIG_DRM depends on !xtensa and !m68k. Guenter
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Mon, Mar 4, 2024 at 9:09 AM Maxime Ripard wrote: > And singling out DRM because it regularly allegedly breaks things on > xtensa or m68k and claiming we're not taking CI seriously because of it > is completely ridiculous. If the all the subsystems were taking CI as > seriously as DRM, we would be in a much better place. > The failure I reported as an example was on arm, not on xtensa or m68k I'll disable CONFIG_DRM in my build tests for xtensa and m68k going forward. Thanks, Guenter
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Mon, Mar 4, 2024 at 8:05 AM Maxime Ripard wrote: > > On Mon, Mar 04, 2024 at 07:46:34AM -0800, Guenter Roeck wrote: > > On Mon, Mar 4, 2024 at 1:24 AM Maxime Ripard wrote: > > [ ... ] > > > > > > If anything, it's more of a side-effect to the push for COMPILE_TEST > > > than anything. > > > > > > > If the drm subsystem maintainers don't want people to build it with > > COMPILE_TEST while at the same time not limiting it to platforms where > > it doesn't even build, I'd suggest making it dependent on > > !COMPILE_TEST. > > I don't think we want anything. My point was that you can't have an > option that is meant to explore for bad practices and expose drivers > that don't go through the proper abstraction, and at the same time > complain that things gets broken. It's the whole point of it. > Can we get back to the original problem, please ? Build errors such as failed 32-bit builds are a nuisance for those running build tests. It seems to me that an automated infrastructure to prevent such build errors from making it into the kernel should be desirable. You seem to disagree, and at least it looked like you complained about the existence of COMPILE_TEST, or that people are doing COMPILE_TEST builds. What is your suggested alternative ? Disabling build tests on drm doesn't seem to be it, and it seems you don't like the idea of a basic generic CI either, but what is it ? > > The same applies to all other subsystems where maintainers don't want > > build tests to run but also don't want to add restrictions such as > > "64-bit only". After all, this was just one example. > > We have drivers for some 32 bits platforms. > I said "such as". Again, that was an example. In this case it would obviously only apply to parts of drm which are not supported on 32-bit systems (and, presumably, the parts of drm which are supposed to be supported on 32-bit systems should build on those). Thanks, Guenter
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Mon, Mar 4, 2024 at 1:24 AM Maxime Ripard wrote: [ ... ] > > If anything, it's more of a side-effect to the push for COMPILE_TEST > than anything. > If the drm subsystem maintainers don't want people to build it with COMPILE_TEST while at the same time not limiting it to platforms where it doesn't even build, I'd suggest making it dependent on !COMPILE_TEST. The same applies to all other subsystems where maintainers don't want build tests to run but also don't want to add restrictions such as "64-bit only". After all, this was just one example. Guenter
Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing
On Thu, Feb 29, 2024 at 12:21 PM Linus Torvalds wrote: > > On Thu, 29 Feb 2024 at 01:23, Nikolai Kondrashov wrote: > > > > However, I think a better approach would be *not* to add the .gitlab-ci.yaml > > file in the root of the source tree, but instead change the very same repo > > setting to point to a particular entry YAML, *inside* the repo (somewhere > > under "ci" directory) instead. > > I really don't want some kind of top-level CI for the base kernel project. > > We already have the situation that the drm people have their own ci > model. II'm ok with that, partly because then at least the maintainers > of that subsystem can agree on the rules for that one subsystem. > > I'm not at all interested in having something that people will then > either fight about, or - more likely - ignore, at the top level > because there isn't some global agreement about what the rules are. > > For example, even just running checkpatch is often a stylistic thing, > and not everybody agrees about all the checkpatch warnings. > While checkpatch is indeed of arguable value, I think it would help a lot not having to bother about the persistent _build_ failures on 32-bit systems. You mentioned the fancy drm CI system above, but they don't run tests and not even test builds on 32-bit targets, which has repeatedly caused (and currently does cause) build failures in drm code when trying to build, say, arm:allmodconfig in linux-next. Most trivial build failures in linux-next (and, yes, sometimes mainline) could be prevented with a simple generic CI. Sure, argue against checkpatch as much as you like, but the code should at least _build_, and it should not be necessary for random people to report build failures to the submitters. Guenter > I would suggest the CI project be separate from the kernel. > > And having that slack channel that is restricted to particular > companies is just another sign of this whole disease. > > If you want to make a google/microsoft project to do kernel CI, then > more power to you, but don't expect it to be some kind of agreed-upon > kernel project when it's a closed system. > >Linus >
Re: [PATCH v5] drm/test: add a test suite for GEM objects backed by shmem
On Thu, Feb 22, 2024 at 05:33:48PM +0100, Marco Pagani wrote: > > > > In this context, the TTM unit tests fail as well in qemu, with worse result: > > It seems there is some bad cleanup after a failed test case, causing list > > corruptions in the drm core and ultimately a crash. I don't know if this > > is also caused by the missing dma_mask initialization. > > > > That's interesting. Which --arch argument are you using to run the > tests with QEMU? Example (I am not sure if any of those parameters matters; it is just one of my tests): qemu-system-x86_64 -kernel arch/x86/boot/bzImage -M q35 -cpu IvyBridge \ -no-reboot -snapshot -smp 2 \ -device e1000,netdev=net0 -netdev user,id=net0 -m 512 \ -drive file=rootfs.ext2,format=raw,if=ide \ --append "earlycon=uart8250,io,0x3f8,9600n8 root=/dev/sda1 console=ttyS0" \ -d unimp,guest_errors -nographic -monitor none This results in: [ ... ] [5.989496] KTAP version 1 [5.989639] # Subtest: ttm_device [5.989711] # module: ttm_device_test [5.989760] 1..5 [6.002044] ok 1 ttm_device_init_basic [6.013557] ok 2 ttm_device_init_multiple ILLOPC: b8ac9350: 0f 0b [6.022481] ok 3 ttm_device_fini_basic [6.026172] [ cut here ] [6.026315] WARNING: CPU: 1 PID: 1575 at drivers/gpu/drm/ttm/ttm_device.c:206 ttm_device_init+0x170/0x190 ... [6.135016] ok 3 Above the allocation limit [6.138759] [ cut here ] [6.138925] WARNING: CPU: 1 PID: 1595 at kernel/dma/mapping.c:503 dma_alloc_attrs+0xf6/0x100 ... [6.143850] # ttm_pool_alloc_basic: ASSERTION FAILED at drivers/gpu/drm/ttm/tests/ttm_pool_test.c:162 [6.143850] Expected err == 0, but [6.143850] err == -12 (0xfff4) [6.148824] not ok 4 One page, with coherent DMA mappings enabled >From there things go downhill. [6.152821] list_add corruption. prev->next should be next (bbd53950), but was . (prev=8af1c38f9e20). and so on until the emulation crashes. Guenter
Re: [PATCH 1/2] devm-helpers: Add resource managed version of mutex init
Oops, sorry for the noise. Shortened reply below.
On 2/22/24 07:24, Guenter Roeck wrote:
On 2/22/24 06:58, Marek Behún wrote:
A few drivers are doing resource-managed mutex initialization by
implementing ad-hoc one-liner mutex dropping functions and using them
with devm_add_action_or_reset(). Help drivers avoid these repeated
one-liners by adding managed version of mutex initialization.
[ ... ]
+static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
+{
+ mutex_init(lock);
+
+ /*
+ * mutex_destroy() is an empty function if CONFIG_DEBUG_MUTEXES is
+ * disabled. No need to allocate an action in that case.
+ */
+ if (IS_ENABLED(CONFIG_DEBUG_MUTEXES))
+ return devm_add_action_or_reset(dev, devm_mutex_drop, lock);
+ else
else after return is unnecessary.
+ return 0;
+}
+
#endif
Re: [PATCH v5] drm/test: add a test suite for GEM objects backed by shmem
Hi Marco, On 2/22/24 07:32, Marco Pagani wrote: On 2024-02-18 16:49, Guenter Roeck wrote: Hi, On Thu, Nov 30, 2023 at 06:14:16PM +0100, Marco Pagani wrote: This patch introduces an initial KUnit test suite for GEM objects backed by shmem buffers. Suggested-by: Javier Martinez Canillas Signed-off-by: Marco Pagani When running this in qemu, I get lots of warnings backtraces in the drm core. WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:327 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:173 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:385 WARNING: CPU: 0 PID: 1341 at drivers/gpu/drm/drm_gem_shmem_helper.c:211 WARNING: CPU: 0 PID: 1345 at kernel/dma/mapping.c:194 WARNING: CPU: 0 PID: 1347 at drivers/gpu/drm/drm_gem_shmem_helper.c:429 WARNING: CPU: 0 PID: 1349 at drivers/gpu/drm/drm_gem_shmem_helper.c:445 It looks like dma_resv_assert_held() asserts each time it is executed. The backtrace in kernel/dma/mapping.c is triggered by if (WARN_ON_ONCE(!dev->dma_mask)) return 0; in __dma_map_sg_attrs(). Is this a possible problem in the test code, or can it be caused by some limitations or bugs in the qemu emulation ? If so, do you have any thoughts or ideas what those limitations / bugs might be ? Hi Guenter, Thanks for reporting this issue. As you correctly noted, the warnings appear to be caused by the dma_mask in the mock device being uninitialized. I'll send a patch to fix it soon. Thanks a lot for the update. In this context, the TTM unit tests fail as well in qemu, with worse result: It seems there is some bad cleanup after a failed test case, causing list corruptions in the drm core and ultimately a crash. I don't know if this is also caused by the missing dma_mask initialization. Thanks, Guenter
Re: [PATCH 1/2] devm-helpers: Add resource managed version of mutex init
On 2/22/24 06:58, Marek Behún wrote:
A few drivers are doing resource-managed mutex initialization by
implementing ad-hoc one-liner mutex dropping functions and using them
with devm_add_action_or_reset(). Help drivers avoid these repeated
one-liners by adding managed version of mutex initialization.
Use the new function devm_mutex_init() in the following drivers:
drivers/gpio/gpio-pisosr.c
drivers/gpio/gpio-sim.c
drivers/gpu/drm/xe/xe_hwmon.c
drivers/hwmon/nzxt-smart2.c
drivers/leds/leds-is31fl319x.c
drivers/power/supply/mt6370-charger.c
drivers/power/supply/rt9467-charger.c
Signed-off-by: Marek Behún
---
drivers/gpio/gpio-pisosr.c| 9 ++-
drivers/gpio/gpio-sim.c | 12 ++
drivers/gpu/drm/xe/xe_hwmon.c | 11 ++---
drivers/hwmon/nzxt-smart2.c | 9 ++-
drivers/leds/leds-is31fl319x.c| 9 ++-
drivers/power/supply/mt6370-charger.c | 11 +
drivers/power/supply/rt9467-charger.c | 34 ---
include/linux/devm-helpers.h | 32 +
8 files changed, 47 insertions(+), 80 deletions(-)
diff --git a/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c
index e3013e778e15..dddbf37e855f 100644
--- a/drivers/gpio/gpio-pisosr.c
+++ b/drivers/gpio/gpio-pisosr.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -116,11 +117,6 @@ static const struct gpio_chip template_chip = {
.can_sleep = true,
};
-static void pisosr_mutex_destroy(void *lock)
-{
- mutex_destroy(lock);
-}
-
static int pisosr_gpio_probe(struct spi_device *spi)
{
struct device *dev = >dev;
@@ -147,8 +143,7 @@ static int pisosr_gpio_probe(struct spi_device *spi)
return dev_err_probe(dev, PTR_ERR(gpio->load_gpio),
"Unable to allocate load GPIO\n");
- mutex_init(>lock);
- ret = devm_add_action_or_reset(dev, pisosr_mutex_destroy, >lock);
+ ret = devm_mutex_init(dev, >lock);
if (ret)
return ret;
diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
index c4106e37e6db..fcfcaa4efe70 100644
--- a/drivers/gpio/gpio-sim.c
+++ b/drivers/gpio/gpio-sim.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -307,13 +308,6 @@ static ssize_t gpio_sim_sysfs_pull_store(struct device
*dev,
return len;
}
-static void gpio_sim_mutex_destroy(void *data)
-{
- struct mutex *lock = data;
-
- mutex_destroy(lock);
-}
-
static void gpio_sim_put_device(void *data)
{
struct device *dev = data;
@@ -457,9 +451,7 @@ static int gpio_sim_add_bank(struct fwnode_handle *swnode,
struct device *dev)
if (ret)
return ret;
- mutex_init(>lock);
- ret = devm_add_action_or_reset(dev, gpio_sim_mutex_destroy,
- >lock);
+ ret = devm_mutex_init(dev, >lock);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
index 174ed2185481..bb88ae1c196c 100644
--- a/drivers/gpu/drm/xe/xe_hwmon.c
+++ b/drivers/gpu/drm/xe/xe_hwmon.c
@@ -3,6 +3,7 @@
* Copyright © 2023 Intel Corporation
*/
+#include
#include
#include
#include
@@ -729,13 +730,6 @@ xe_hwmon_get_preregistration_info(struct xe_device *xe)
xe_hwmon_energy_get(hwmon, );
}
-static void xe_hwmon_mutex_destroy(void *arg)
-{
- struct xe_hwmon *hwmon = arg;
-
- mutex_destroy(>hwmon_lock);
-}
-
void xe_hwmon_register(struct xe_device *xe)
{
struct device *dev = xe->drm.dev;
@@ -751,8 +745,7 @@ void xe_hwmon_register(struct xe_device *xe)
xe->hwmon = hwmon;
- mutex_init(>hwmon_lock);
- if (devm_add_action_or_reset(dev, xe_hwmon_mutex_destroy, hwmon))
+ if (devm_mutex_init(dev, >hwmon_lock))
return;
/* primary GT to access device level properties */
diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
index 7aa586eb74be..00bc89607673 100644
--- a/drivers/hwmon/nzxt-smart2.c
+++ b/drivers/hwmon/nzxt-smart2.c
@@ -5,6 +5,7 @@
* Copyright (c) 2021 Aleksandr Mezin
*/
+#include
#include
#include
#include
@@ -721,11 +722,6 @@ static int __maybe_unused
nzxt_smart2_hid_reset_resume(struct hid_device *hdev)
return init_device(drvdata, drvdata->update_interval);
}
-static void mutex_fini(void *lock)
-{
- mutex_destroy(lock);
-}
-
static int nzxt_smart2_hid_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -741,8 +737,7 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
init_waitqueue_head(>wq);
- mutex_init(>mutex);
- ret = devm_add_action_or_reset(>dev, mutex_fini, >mutex);
+ ret = devm_mutex_init(>dev, >mutex);
if
Re: [PATCH 2/2] devm-helpers: Add resource managed version of debugfs directory create function
On 2/22/24 06:58, Marek Behún wrote:
A few drivers register a devm action to remove a debugfs directory,
implementing a one-liner function that calls debufs_remove_recursive().
Help drivers avoid this repeated implementations by adding managed
version of debugfs directory create function.
Use the new function devm_debugfs_create_dir() in the following
drivers:
drivers/crypto/caam/ctrl.c
drivers/gpu/drm/bridge/ti-sn65dsi86.c
drivers/hwmon/hp-wmi-sensors.c
drivers/hwmon/mr75203.c
drivers/hwmon/pmbus/pmbus_core.c
Also use the action function devm_debugfs_dir_recursive_drop() in
drivers
drivers/cxl/mem.c
drivers/gpio/gpio-mockup.c
Signed-off-by: Marek Behún
---
drivers/crypto/caam/ctrl.c| 16 +++--
drivers/cxl/mem.c | 9 ++---
drivers/gpio/gpio-mockup.c| 11 ++
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 13 ++--
drivers/hwmon/hp-wmi-sensors.c| 15 ++---
drivers/hwmon/mr75203.c | 15 +++--
drivers/hwmon/pmbus/pmbus_core.c | 16 +++--
include/linux/devm-helpers.h | 48 +++
8 files changed, 72 insertions(+), 71 deletions(-)
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index bdf367f3f679..ea3ed9a17f1a 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
#include
@@ -604,11 +605,6 @@ static int init_clocks(struct device *dev, const struct
caam_imx_data *data)
return devm_add_action_or_reset(dev, disable_clocks, ctrlpriv);
}
-static void caam_remove_debugfs(void *root)
-{
- debugfs_remove_recursive(root);
-}
-
#ifdef CONFIG_FSL_MC_BUS
static bool check_version(struct fsl_mc_version *mc_version, u32 major,
u32 minor, u32 revision)
@@ -1058,13 +1054,9 @@ static int caam_probe(struct platform_device *pdev)
ctrlpriv->era = caam_get_era(perfmon);
ctrlpriv->domain = iommu_get_domain_for_dev(dev);
- dfs_root = debugfs_create_dir(dev_name(dev), NULL);
- if (IS_ENABLED(CONFIG_DEBUG_FS)) {
- ret = devm_add_action_or_reset(dev, caam_remove_debugfs,
- dfs_root);
- if (ret)
- return ret;
- }
+ dfs_root = devm_debugfs_create_dir(dev, dev_name(dev), NULL);
+ if (IS_ERR(dfs_root))
+ return PTR_ERR(dfs_root);
caam_debugfs_init(ctrlpriv, perfmon, dfs_root);
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index c5c9d8e0d88d..4b38514887a4 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -2,6 +2,7 @@
/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
#include
#include
+#include
#include
#include
@@ -30,11 +31,6 @@ static void enable_suspend(void *data)
cxl_mem_active_dec();
}
-static void remove_debugfs(void *dentry)
-{
- debugfs_remove_recursive(dentry);
-}
-
static int cxl_mem_dpa_show(struct seq_file *file, void *data)
{
struct device *dev = file->private;
@@ -138,7 +134,8 @@ static int cxl_mem_probe(struct device *dev)
debugfs_create_file("clear_poison", 0200, dentry, cxlmd,
_poison_clear_fops);
- rc = devm_add_action_or_reset(dev, remove_debugfs, dentry);
+ rc = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
+ dentry);
if (rc)
return rc;
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 455eecf6380e..adbe0fe09490 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -390,13 +391,6 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
}
}
-static void gpio_mockup_debugfs_cleanup(void *data)
-{
- struct gpio_mockup_chip *chip = data;
-
- debugfs_remove_recursive(chip->dbg_dir);
-}
-
static void gpio_mockup_dispose_mappings(void *data)
{
struct gpio_mockup_chip *chip = data;
@@ -480,7 +474,8 @@ static int gpio_mockup_probe(struct platform_device *pdev)
gpio_mockup_debugfs_setup(dev, chip);
- return devm_add_action_or_reset(dev, gpio_mockup_debugfs_cleanup, chip);
+ return devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
+ chip->dbg_dir);
}
static const struct of_device_id gpio_mockup_of_match[] = {
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 62cc3893dca5..ad0ed2459394 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -427,18 +428,12 @@ static int status_show(struct
Re: [PATCH 9/9] kunit: Annotate _MSG assertion variants with gnu printf specifiers
On Wed, Feb 21, 2024 at 05:27:22PM +0800, David Gow wrote:
> KUnit's assertion macros have variants which accept a printf format
> string, to allow tests to specify a more detailed message on failure.
> These (and the related KUNIT_FAIL() macro) ultimately wrap the
> __kunit_do_failed_assertion() function, which accepted a printf format
> specifier, but did not have the __printf attribute, so gcc couldn't warn
> on incorrect agruments.
>
> It turns out there were quite a few tests with such incorrect arguments.
>
> Add the __printf() specifier now that we've fixed these errors, to
> prevent them from recurring.
>
> Suggested-by: Linus Torvalds
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> include/kunit/test.h | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index fcb4a4940ace..61637ef32302 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -579,12 +579,12 @@ void __printf(2, 3) kunit_log_append(struct
> string_stream *log, const char *fmt,
>
> void __noreturn __kunit_abort(struct kunit *test);
>
> -void __kunit_do_failed_assertion(struct kunit *test,
> -const struct kunit_loc *loc,
> -enum kunit_assert_type type,
> -const struct kunit_assert *assert,
> -assert_format_t assert_format,
> -const char *fmt, ...);
> +void __printf(6, 7) __kunit_do_failed_assertion(struct kunit *test,
> + const struct kunit_loc *loc,
> + enum kunit_assert_type type,
> + const struct kunit_assert
> *assert,
> + assert_format_t assert_format,
> + const char *fmt, ...);
>
> #define _KUNIT_FAILED(test, assert_type, assert_class, assert_format,
> INITIALIZER, fmt, ...) do { \
> static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Re: [PATCH 8/9] drm/xe/tests: Fix printf format specifiers in xe_migrate test
On Wed, Feb 21, 2024 at 05:27:21PM +0800, David Gow wrote:
> KUNIT_FAIL() is used to fail the xe_migrate test when an error occurs.
> However, there's a mismatch in the format specifier: '%li' is used to
> log 'err', which is an 'int'.
>
> Use '%i' instead of '%li', and for the case where we're printing an
> error pointer, just use '%pe', instead of extracting the error code
> manually with PTR_ERR(). (This also results in a nicer output when the
> error code is known.)
>
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> drivers/gpu/drm/xe/tests/xe_migrate.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/tests/xe_migrate.c
> b/drivers/gpu/drm/xe/tests/xe_migrate.c
> index a6523df0f1d3..c347e2c29f81 100644
> --- a/drivers/gpu/drm/xe/tests/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/tests/xe_migrate.c
> @@ -114,21 +114,21 @@ static void test_copy(struct xe_migrate *m, struct
> xe_bo *bo,
> region |
> XE_BO_NEEDS_CPU_ACCESS);
> if (IS_ERR(remote)) {
> - KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %li\n",
> -str, PTR_ERR(remote));
> + KUNIT_FAIL(test, "Failed to allocate remote bo for %s: %pe\n",
> +str, remote);
> return;
> }
>
> err = xe_bo_validate(remote, NULL, false);
> if (err) {
> - KUNIT_FAIL(test, "Failed to validate system bo for %s: %li\n",
> + KUNIT_FAIL(test, "Failed to validate system bo for %s: %i\n",
> str, err);
> goto out_unlock;
> }
>
> err = xe_bo_vmap(remote);
> if (err) {
> - KUNIT_FAIL(test, "Failed to vmap system bo for %s: %li\n",
> + KUNIT_FAIL(test, "Failed to vmap system bo for %s: %i\n",
> str, err);
> goto out_unlock;
> }
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Re: [PATCH 7/9] drm: tests: Fix invalid printf format specifiers in KUnit tests
On Wed, Feb 21, 2024 at 05:27:20PM +0800, David Gow wrote:
> The drm_buddy_test's alloc_contiguous test used a u64 for the page size,
> which was then updated to be an 'unsigned long' to avoid 64-bit
> multiplication division helpers.
>
> However, the variable is logged by some KUNIT_ASSERT_EQ_MSG() using the
> '%d' or '%llu' format specifiers, the former of which is always wrong,
> and the latter is no longer correct now that ps is no longer a u64. Fix
> these to all use '%lu'.
>
> Also, drm_mm_test calls KUNIT_FAIL() with an empty string as the
> message. gcc warns if a printf format string is empty (apparently), so
> give these some more detailed error messages, which should be more
> useful anyway.
>
> Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
> Fixes: fca7526b7d89 ("drm/tests/drm_buddy: fix build failure on 32-bit
> targets")
> Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> drivers/gpu/drm/tests/drm_buddy_test.c | 14 +++---
> drivers/gpu/drm/tests/drm_mm_test.c| 6 +++---
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
> b/drivers/gpu/drm/tests/drm_buddy_test.c
> index 8a464f7f4c61..3dbfa3078449 100644
> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> @@ -55,30 +55,30 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
> *test)
> KUNIT_ASSERT_FALSE_MSG(test,
> drm_buddy_alloc_blocks(, 0, mm_size,
> ps, ps, list, 0),
> -"buddy_alloc hit an error size=%d\n",
> +"buddy_alloc hit an error size=%lu\n",
> ps);
> } while (++i < n_pages);
>
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%d\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
>
> drm_buddy_free_list(, );
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 2 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 2 * ps);
> +"buddy_alloc didn't error size=%lu\n", 2 * ps);
>
> drm_buddy_free_list(, );
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
> /*
>* At this point we should have enough contiguous space for 2 blocks,
>* however they are never buddies (since we freed middle and right) so
> @@ -87,13 +87,13 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
> *test)
> KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 2 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc hit an error size=%d\n", 2 * ps);
> +"buddy_alloc hit an error size=%lu\n", 2 * ps);
>
> drm_buddy_free_list(, );
> KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCA
Re: [PATCH 6/9] net: test: Fix printf format specifier in skb_segment kunit test
On Wed, Feb 21, 2024 at 05:27:19PM +0800, David Gow wrote:
> KUNIT_FAIL() accepts a printf-style format string, but previously did
> not let gcc validate it with the __printf() attribute. The use of %lld
> for the result of PTR_ERR() is not correct.
>
> Instead, use %pe and pass the actual error pointer. printk() will format
> it correctly (and give a symbolic name rather than a number if
> available, which should make the output more readable, too).
>
> Fixes: b3098d32ed6e ("net: add skb_segment kunit test")
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> net/core/gso_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/gso_test.c b/net/core/gso_test.c
> index 4c2e77bd12f4..358c44680d91 100644
> --- a/net/core/gso_test.c
> +++ b/net/core/gso_test.c
> @@ -225,7 +225,7 @@ static void gso_test_func(struct kunit *test)
>
> segs = skb_segment(skb, features);
> if (IS_ERR(segs)) {
> - KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs));
> + KUNIT_FAIL(test, "segs error %pe", segs);
> goto free_gso_skb;
> } else if (!segs) {
> KUNIT_FAIL(test, "no segments");
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Re: [PATCH 5/9] rtc: test: Fix invalid format specifier.
On Wed, Feb 21, 2024 at 05:27:18PM +0800, David Gow wrote:
> 'days' is a s64 (from div_s64), and so should use a %lld specifier.
>
> This was found by extending KUnit's assertion macros to use gcc's
> __printf attribute.
>
> Fixes: 1d1bb12a8b18 ("rtc: Improve performance of rtc_time64_to_tm(). Add
> tests.")
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> drivers/rtc/lib_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/lib_test.c b/drivers/rtc/lib_test.c
> index d5caf36c56cd..225c859d6da5 100644
> --- a/drivers/rtc/lib_test.c
> +++ b/drivers/rtc/lib_test.c
> @@ -54,7 +54,7 @@ static void rtc_time64_to_tm_test_date_range(struct kunit
> *test)
>
> days = div_s64(secs, 86400);
>
> - #define FAIL_MSG "%d/%02d/%02d (%2d) : %ld", \
> + #define FAIL_MSG "%d/%02d/%02d (%2d) : %lld", \
> year, month, mday, yday, days
>
> KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year,
> FAIL_MSG);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Re: [PATCH 4/9] time: test: Fix incorrect format specifier
On Wed, Feb 21, 2024 at 05:27:17PM +0800, David Gow wrote:
> 'days' is a s64 (from div_s64), and so should use a %lld specifier.
>
> This was found by extending KUnit's assertion macros to use gcc's
> __printf attribute.
>
> Fixes: 276010551664 ("time: Improve performance of time64_to_tm()")
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> kernel/time/time_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/time_test.c b/kernel/time/time_test.c
> index ca058c8af6ba..3e5d422dd15c 100644
> --- a/kernel/time/time_test.c
> +++ b/kernel/time/time_test.c
> @@ -73,7 +73,7 @@ static void time64_to_tm_test_date_range(struct kunit *test)
>
> days = div_s64(secs, 86400);
>
> - #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %ld", \
> + #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %lld", \
> year, month, mdday, yday, days
>
> KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year,
> FAIL_MSG);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Re: [PATCH 3/9] lib: memcpy_kunit: Fix an invalid format specifier in an assertion msg
On Wed, Feb 21, 2024 at 05:27:16PM +0800, David Gow wrote:
> The 'i' passed as an assertion message is a size_t, so should use '%zu',
> not '%d'.
>
> This was found by annotating the _MSG() variants of KUnit's assertions
> to let gcc validate the format strings.
>
> Fixes: bb95ebbe89a7 ("lib: Introduce CONFIG_MEMCPY_KUNIT_TEST")
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> lib/memcpy_kunit.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c
> index 440aee705ccc..30e00ef0bf2e 100644
> --- a/lib/memcpy_kunit.c
> +++ b/lib/memcpy_kunit.c
> @@ -32,7 +32,7 @@ struct some_bytes {
> BUILD_BUG_ON(sizeof(instance.data) != 32); \
> for (size_t i = 0; i < sizeof(instance.data); i++) {\
> KUNIT_ASSERT_EQ_MSG(test, instance.data[i], v, \
> - "line %d: '%s' not initialized to 0x%02x @ %d (saw
> 0x%02x)\n", \
> + "line %d: '%s' not initialized to 0x%02x @ %zu (saw
> 0x%02x)\n", \
> __LINE__, #instance, v, i, instance.data[i]); \
> } \
> } while (0)
> @@ -41,7 +41,7 @@ struct some_bytes {
> BUILD_BUG_ON(sizeof(one) != sizeof(two)); \
> for (size_t i = 0; i < sizeof(one); i++) { \
> KUNIT_EXPECT_EQ_MSG(test, one.data[i], two.data[i], \
> - "line %d: %s.data[%d] (0x%02x) != %s.data[%d]
> (0x%02x)\n", \
> + "line %d: %s.data[%zu] (0x%02x) != %s.data[%zu]
> (0x%02x)\n", \
> __LINE__, #one, i, one.data[i], #two, i, two.data[i]); \
> } \
> kunit_info(test, "ok: " TEST_OP "() " name "\n"); \
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg
On Wed, Feb 21, 2024 at 05:27:15PM +0800, David Gow wrote:
> The correct format specifier for p - n (both p and n are pointers) is
> %td, as the type should be ptrdiff_t.
>
> This was discovered by annotating KUnit assertion macros with gcc's
> printf specifier, but note that gcc incorrectly suggested a %d or %ld
> specifier (depending on the pointer size of the architecture being
> built).
>
> Fixes: 0ea09083116d ("lib/cmdline: Allow get_options() to take 0 to validate
> the input")
> Signed-off-by: David Gow
Tested-by: Guenter Roeck
> ---
> lib/cmdline_kunit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
> index d4572dbc9145..705b82736be0 100644
> --- a/lib/cmdline_kunit.c
> +++ b/lib/cmdline_kunit.c
> @@ -124,7 +124,7 @@ static void cmdline_do_one_range_test(struct kunit *test,
> const char *in,
> n, e[0], r[0]);
>
> p = memchr_inv([1], 0, sizeof(r) - sizeof(r[0]));
> - KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %u out of bound",
> n, p - r);
> + KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %td out of
> bound", n, p - r);
> }
>
> static void cmdline_test_range(struct kunit *test)
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
