* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST [not found] ` <b3dba59d-22cc-4404-8876-8fd4e22021b5@gmail.com> @ 2026-05-06 12:37 ` Alexander Gordeev 2026-05-06 16:51 ` Julian Braha 0 siblings, 1 reply; 4+ 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] 4+ messages in thread
* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST 2026-05-06 12:37 ` [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST Alexander Gordeev @ 2026-05-06 16:51 ` Julian Braha 2026-05-08 11:22 ` Alexander Gordeev 0 siblings, 1 reply; 4+ 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] 4+ messages in thread
* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST 2026-05-06 16:51 ` Julian Braha @ 2026-05-08 11:22 ` Alexander Gordeev 2026-05-08 12:35 ` Julian Braha 0 siblings, 1 reply; 4+ messages in thread From: Alexander Gordeev @ 2026-05-08 11:22 UTC (permalink / raw) To: Julian Braha Cc: gor, hca, iii, meted, borntraeger, svens, akpm, linux-s390, linux-kernel On Wed, May 06, 2026 at 05:51:19PM +0100, Julian Braha wrote: > 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: That is what is currently generated (likely because of "depends on KUNIT"): CONFIG_S390_UNWIND_SELFTEST=m CONFIG_S390_MODULES_SANITY_TEST=m # CONFIG_KUNIT_ALL_TESTS is not set > 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. I could not recreate it. Could you please provide the exact steps? > 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/ Well, this is not about severity, it is about whether an actual problem did exist and was fixed. The patch itself is fine > > 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...) I do appreciate your effort to bring a new tool! But unless it is part of the toolchain it shold not be referred to in a commit message as far as I am concerned. IOW, once it is in the tree, it is fine to mention it: https://lore.kernel.org/all/20260405122749.4990dcb538d457769a3276e0@linux-foundation.org/ officially > - Julian Braha Thanks! ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST 2026-05-08 11:22 ` Alexander Gordeev @ 2026-05-08 12:35 ` Julian Braha 0 siblings, 0 replies; 4+ messages in thread From: Julian Braha @ 2026-05-08 12:35 UTC (permalink / raw) To: Alexander Gordeev Cc: gor, hca, iii, meted, borntraeger, svens, akpm, linux-s390, linux-kernel On 5/8/26 12:22, Alexander Gordeev wrote: > I could not recreate it. Could you please provide the exact steps? Sure, here are the exact commands I ran to reproduce for S390_UNWIND_SELFTEST. Note that I'm on x86-64 and cross-compiling for s390: 1. Start from a minimal configuration: 'make ARCH=s390 CROSS_COMPILE=s390x-unknown-linux-gnu- allnoconfig' 2. Enter menuconfig: 'make ARCH=s390 CROSS_COMPILE=s390x-unknown-linux-gnu- menuconfig' 3. Set CONFIG_KUNIT=y 4. Set CONFIG_KUNIT_ALL_TESTS=y 5. Check '.config', and see that S390_UNWIND_SELFTEST is still disabled The steps to reproduce for S390_MODULES_SANITY_TEST are the same, but you also need to enable MODULES because of the additional dependency. > I do appreciate your effort to bring a new tool! But unless it is part > of > the toolchain it shold not be referred to in a commit message as far as > I am concerned. IOW, once it is in the tree, it is fine to mention it: Thanks :) Separately, I checked with Greg, and he says that: 1. it is a requirement to acknowledge tool usage, but 2. this patch (and others like it) shouldn't have the Fixes tag. So I will resubmit this patch without "Fixes", but still acknowledging the tool, see also: https://lore.kernel.org/all/2026050728-construct-tingle-0852@gregkh/ - Julian Braha ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-08 12:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260414215651.151228-1-julianbraha@gmail.com>
[not found] ` <a3adc8e8-ce4c-458c-a0c8-c0e66078589d-agordeev@linux.ibm.com>
[not found] ` <b3dba59d-22cc-4404-8876-8fd4e22021b5@gmail.com>
2026-05-06 12:37 ` [RESEND PATCH] s390: fix dead defaults for S390_MODULES_SANITY_TEST and S390_UNWIND_SELFTEST Alexander Gordeev
2026-05-06 16:51 ` Julian Braha
2026-05-08 11:22 ` Alexander Gordeev
2026-05-08 12:35 ` Julian Braha
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox