* [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST
@ 2026-04-14 21:56 Julian Braha
2026-04-29 14:42 ` Alexander Gordeev
0 siblings, 1 reply; 5+ messages in thread
From: Julian Braha @ 2026-04-14 21:56 UTC (permalink / raw)
To: agordeev, gor, hca, iii
Cc: meted, borntraeger, svens, akpm, linux-s390, linux-kernel,
Julian Braha
These config options currently have unconditional defaults of 'n' from the
def_tristate statement, which shadow the later default of
'KUNIT_ALL_TESTS', causing it to be dead code.
It looks to me like the commit 25d36a85c61b ("s390/test_unwind: convert to KUnit")
added the KUNIT_ALL_TESTS default to S390_UNWIND_SELFTEST, but mistakenly
didn't remove the previous 'n' default.
Then, the later commit 90c5318795ee ("s390/module: test loading modules with a lot of relocations")
copied the Kconfig layout from S390_UNWIND_SELFTEST when adding the
S390_MODULES_SANITY_TEST config option, without noticing the existing mistake.
This dead code was found by kconfirm, a static analysis tool for Kconfig.
Fixes: 25d36a85c61b ("s390/test_unwind: convert to KUnit")
Fixes: 90c5318795ee ("s390/module: test loading modules with a lot of relocations")
Signed-off-by: Julian Braha <julianbraha@gmail.com>
---
arch/s390/Kconfig | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index edc927d9e85a..1b3afe89cb00 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -999,9 +999,8 @@ config S390_MODULES_SANITY_TEST_HELPERS
menu "Selftests"
config S390_UNWIND_SELFTEST
- def_tristate n
+ def_tristate KUNIT_ALL_TESTS
depends on KUNIT
- default KUNIT_ALL_TESTS
prompt "Test unwind functions"
help
This option enables s390 specific stack unwinder testing kernel
@@ -1023,9 +1022,8 @@ config S390_KPROBES_SANITY_TEST
Say N if you are unsure.
config S390_MODULES_SANITY_TEST
- def_tristate n
+ def_tristate KUNIT_ALL_TESTS
depends on KUNIT
- default KUNIT_ALL_TESTS
prompt "Enable s390 specific modules tests"
select S390_MODULES_SANITY_TEST_HELPERS
help
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST
2026-04-14 21:56 [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST Julian Braha
@ 2026-04-29 14:42 ` Alexander Gordeev
2026-04-29 21:39 ` Julian Braha
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Gordeev @ 2026-04-29 14:42 UTC (permalink / raw)
To: Julian Braha
Cc: gor, hca, iii, meted, borntraeger, svens, akpm, linux-s390,
linux-kernel
On Tue, Apr 14, 2026 at 10:56:51PM +0100, Julian Braha wrote:
> These config options currently have unconditional defaults of 'n' from the
> def_tristate statement, which shadow the later default of
> 'KUNIT_ALL_TESTS', causing it to be dead code.
>
> It looks to me like the commit 25d36a85c61b ("s390/test_unwind: convert to KUnit")
> added the KUNIT_ALL_TESTS default to S390_UNWIND_SELFTEST, but mistakenly
> didn't remove the previous 'n' default.
>
> Then, the later commit 90c5318795ee ("s390/module: test loading modules with a lot of relocations")
> copied the Kconfig layout from S390_UNWIND_SELFTEST when adding the
> S390_MODULES_SANITY_TEST config option, without noticing the existing mistake.
>
> This dead code was found by kconfirm, a static analysis tool for Kconfig.
This tool is still in development, right?
Anyway, I do not quite get what do you mean with the dead code.
Could you please elaborate?
> Fixes: 25d36a85c61b ("s390/test_unwind: convert to KUnit")
> Fixes: 90c5318795ee ("s390/module: test loading modules with a lot of relocations")
> Signed-off-by: Julian Braha <julianbraha@gmail.com>
> ---
> arch/s390/Kconfig | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST
2026-04-29 14:42 ` Alexander Gordeev
@ 2026-04-29 21:39 ` Julian Braha
2026-05-06 12:37 ` Alexander Gordeev
0 siblings, 1 reply; 5+ messages in thread
From: Julian Braha @ 2026-04-29 21:39 UTC (permalink / raw)
To: Alexander Gordeev
Cc: gor, hca, iii, meted, borntraeger, svens, akpm, linux-s390,
linux-kernel
On Wed, 29 Apr 2026, Alexander Gordeev <agordeev@linux.ibm.com> wrote:
> This tool is still in development, right?
You can find the latest release (v0.7) outside the tree here:
https://github.com/julianbraha/kconfirm
And there's also a recent RFC to move it into the tree here:
https://lore.kernel.org/all/20260427174429.779474-1-julianbraha@gmail.com/
> Anyway, I do not quite get what do you mean with the dead code.
> Could you please elaborate?
Sure! In kconfig, default statements are evaluated in the order
they appear (top --> bottom). This means that if you have an
unconditional default statement at the top, e.g. 'default n', then all
of the default statements that follow it will never be evaluated - in
other words, they are dead code.
In the case of this code in particular, the 'def_tristate n' at the top
does two things:
1. declares the type of the config option as a tristate,
2. sets the default value to 'n' unconditionally.
So, the 'default KUNIT_ALL_TESTS' statement that follows is dead code.
See also this sentence from the "Menu attributes" section of the kconfig
docs:
"If multiple default values are visible, only the first defined one is
active."
Source:
https://docs.kernel.org/kbuild/kconfig-language.html
- Julian Braha
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST
2026-04-29 21:39 ` Julian Braha
@ 2026-05-06 12:37 ` Alexander Gordeev
2026-05-06 16:51 ` Julian Braha
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Gordeev @ 2026-05-06 12:37 UTC (permalink / raw)
To: Julian Braha
Cc: gor, hca, iii, meted, borntraeger, svens, akpm, linux-s390,
linux-kernel
On Wed, Apr 29, 2026 at 10:39:14PM +0100, Julian Braha wrote:
Hi Julian,
> On Wed, 29 Apr 2026, Alexander Gordeev <agordeev@linux.ibm.com> wrote:
> > This tool is still in development, right?
>
> You can find the latest release (v0.7) outside the tree here:
> https://github.com/julianbraha/kconfirm
>
> And there's also a recent RFC to move it into the tree here:
> https://lore.kernel.org/all/20260427174429.779474-1-julianbraha@gmail.com/
>
> > Anyway, I do not quite get what do you mean with the dead code.
> > Could you please elaborate?
>
> Sure! In kconfig, default statements are evaluated in the order
> they appear (top --> bottom). This means that if you have an
> unconditional default statement at the top, e.g. 'default n', then all
> of the default statements that follow it will never be evaluated - in
> other words, they are dead code.
>
> In the case of this code in particular, the 'def_tristate n' at the top
> does two things:
> 1. declares the type of the config option as a tristate,
> 2. sets the default value to 'n' unconditionally.
>
> So, the 'default KUNIT_ALL_TESTS' statement that follows is dead code.
>
> See also this sentence from the "Menu attributes" section of the kconfig
> docs:
> "If multiple default values are visible, only the first defined one is
> active."
> Source:
> https://docs.kernel.org/kbuild/kconfig-language.html
Thanks a lot for the clarificaiton!
In the end of the day I see no impact on the end result (aka the
generated config) - please correct me if I am wrong.
I think once the tool is officially included you could refer to it
in the commit message, but please do not add the described commits
as Fixes - these are no real issues.
> - Julian Braha
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST
2026-05-06 12:37 ` Alexander Gordeev
@ 2026-05-06 16:51 ` Julian Braha
0 siblings, 0 replies; 5+ messages in thread
From: Julian Braha @ 2026-05-06 16:51 UTC (permalink / raw)
To: Alexander Gordeev
Cc: gor, hca, iii, meted, borntraeger, svens, akpm, linux-s390,
linux-kernel
On 5/6/26 13:37, Alexander Gordeev wrote:
> In the end of the day I see no impact on the end result (aka the
> generated config) - please correct me if I am wrong.
Yes, there is actually an impact on the end result / the generated
config:
When a user of the kernel build system (e.g. 'make menuconfig') enables
the config option KUNIT_ALL_TESTS, the user expects *all of the tests*
to be enabled. Which is what commit
25d36a85c61b ("s390/test_unwind: convert to KUnit")
was attempting to do with these tests, but it appears they made a
copy-paste mistake. This is the commit referenced in the Fixes tag.
The point is, these tests were intended to be included with
KUNIT_ALL_TESTS, but aren't. This is unintended behavior, and the
definition of a bug in the kernel configuration spec.
That being said, I do agree with you that this is pretty low in
severity, as far as bugs go. It is kunit tests we're talking about,
after all. I'll check with Greg to see if he thinks this is worthy of
a Fixes tag. Another, similar patch I sent out includes the Fixes tag,
and has not received any objections from its 3 reviewers and testers
so far:
https://lore.kernel.org/all/20260405161545.161006-1-julianbraha@gmail.com/
> I think once the tool is officially included you could refer to it
> in the commit message,
I will also check on the best practices for tool-assisted development.
In the past, I have received encouragement for acknowledging the tool:
https://lore.kernel.org/all/bfdfdb05-77e2-455d-b68d-9da3fd9d1c0d@lucifer.local/
and the official kernel docs require an acknowledgement for AI tool
assistance (although this tool is not AI and doesn't generate code...)
- Julian Braha
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-06 16:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-14 21:56 [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST Julian Braha
2026-04-29 14:42 ` Alexander Gordeev
2026-04-29 21:39 ` Julian Braha
2026-05-06 12:37 ` Alexander Gordeev
2026-05-06 16:51 ` Julian Braha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox