The 32-bit arm, mips and powerpc the tinyconfig builds failed on today's Linux next-20240712 tag with gcc and clang builds. The defconfig builds pass.
GOOD: next-20240711 BAD: next-20240712
Build error: ------ arm-linux-gnueabihf-ld: kernel/task_work.o: in function `task_work_add': task_work.c:(.text+0xc2): undefined reference to `irq_work_queue'
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Build link: ----- [1] https://storage.tuxsuite.com/public/linaro/lkft/builds/2j8Y3PffxvTIFJIR8HODm... [2] https://storage.tuxsuite.com/public/linaro/lkft/builds/2j8Y3PffxvTIFJIR8HODm...
Build details: -------- git_describe: next-20240712 git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next git_sha: 3fe121b622825ff8cc995a1e6b026181c48188db git_short_log: 3fe121b62282 ("Add linux-next specific files for 20240712") arch: arm, mips, powerpc, s390 config: tinyconfig toolchain: gcc-13, gcc-12 and clang
-- Linaro LKFT https://lkft.linaro.org
On Fri, Jul 12, 2024, at 14:13, Naresh Kamboju wrote:
The 32-bit arm, mips and powerpc the tinyconfig builds failed on today's Linux next-20240712 tag with gcc and clang builds. The defconfig builds pass.
GOOD: next-20240711 BAD: next-20240712
Build error:
arm-linux-gnueabihf-ld: kernel/task_work.o: in function `task_work_add': task_work.c:(.text+0xc2): undefined reference to `irq_work_queue'
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
The call to this function was added in 466e4d801cd4 ("task_work: Add TWA_NMI_CURRENT as an additional notify mode."). It's possible that we may have to always enable IRQ_WORK even on non-SMP kernels now. In practice it is already enabled in most configurations for one reason or another, the the cost is likely very small.
Otherwise checking for CONFIG_HAVE_NMI in the new code might work.
Arnd
On Fri, Jul 12, 2024 at 02:28:38PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 14:13, Naresh Kamboju wrote:
The 32-bit arm, mips and powerpc the tinyconfig builds failed on today's Linux next-20240712 tag with gcc and clang builds. The defconfig builds pass.
GOOD: next-20240711 BAD: next-20240712
Build error:
arm-linux-gnueabihf-ld: kernel/task_work.o: in function `task_work_add': task_work.c:(.text+0xc2): undefined reference to `irq_work_queue'
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
The call to this function was added in 466e4d801cd4 ("task_work: Add TWA_NMI_CURRENT as an additional notify mode."). It's possible
Thanks, that initial report was mostly useless without that sha. I do wonder why 0day build robot didn't complain to me about this. It seems something it should definitely find.
that we may have to always enable IRQ_WORK even on non-SMP kernels now. In practice it is already enabled in most configurations for one reason or another, the the cost is likely very small.
Otherwise checking for CONFIG_HAVE_NMI in the new code might work.
ARM seems to have HAVE_NMI while also being one of the architectures that is now failing.
I'm a bit confused though, perf is already depending on irq_work (and perf is the user of TWA_NMI_CURRENT). So I'm not exactly sure what config fail is leading to all this.
I suppose we can do something like the below.. it'll end up in a runtime fail for whoever manages to use TWA_NMI_CURRENT without also having irq_work enabled, but that should currently be nobody.
--- kernel/task_work.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/kernel/task_work.c b/kernel/task_work.c index 5c2daa7ad3f9..276e245b7e7e 100644 --- a/kernel/task_work.c +++ b/kernel/task_work.c @@ -81,9 +81,11 @@ int task_work_add(struct task_struct *task, struct callback_head *work, case TWA_SIGNAL_NO_IPI: __set_notify_signal(task); break; +#ifdef CONFIG_IRQ_WORK case TWA_NMI_CURRENT: irq_work_queue(this_cpu_ptr(&irq_work_NMI_resume)); break; +#endif default: WARN_ON_ONCE(1); break;
On Fri, Jul 12, 2024, at 15:24, Peter Zijlstra wrote:
On Fri, Jul 12, 2024 at 02:28:38PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 14:13, Naresh Kamboju wrote:
that we may have to always enable IRQ_WORK even on non-SMP kernels now. In practice it is already enabled in most configurations for one reason or another, the the cost is likely very small.
Otherwise checking for CONFIG_HAVE_NMI in the new code might work.
ARM seems to have HAVE_NMI while also being one of the architectures that is now failing.
Right, in this case we would also need
--- a/arch/Kconfig +++ b/arch/Kconfig @@ -236,6 +236,7 @@ config HAVE_FUNCTION_ERROR_INJECTION
config HAVE_NMI bool + select IRQ_WORK
config HAVE_FUNCTION_DESCRIPTORS bool
I'm a bit confused though, perf is already depending on irq_work (and perf is the user of TWA_NMI_CURRENT). So I'm not exactly sure what config fail is leading to all this.
Ok, this is the bit I was missing. If it's only needed for perf, then the problem is that the caller is built in unconditionally even when perf is disabled, otherwise it would be caught by the existing:
config PERF_EVENTS bool "Kernel performance events and counters" select IRQ_WORK
I suppose we can do something like the below.. it'll end up in a runtime fail for whoever manages to use TWA_NMI_CURRENT without also having irq_work enabled, but that should currently be nobody.
break;+#ifdef CONFIG_IRQ_WORK case TWA_NMI_CURRENT: irq_work_queue(this_cpu_ptr(&irq_work_NMI_resume)); break; +#endif default:
This looks more fragile than necessary. as it might fail if anything else starts using TWA_NMI_CURRENT without selecting IRQ_WORK. I would prefer either something that makes it still run into a compile-time error (e.g. hiding the TWA_NMI_CURRENT inside the same #ifdef), or just making IRQ_WORK unconditional as I first suggested.
Configurations without IRQ_WORK are extremely rare, since it also gets selected by CONFIG_PRINTK, which can only be turned off for CONFIG_EXPERT=y and is almost always a good idea even for small kernels.
Arnd
On Fri, Jul 12, 2024 at 04:11:54PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 15:24, Peter Zijlstra wrote:
On Fri, Jul 12, 2024 at 02:28:38PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 14:13, Naresh Kamboju wrote:
that we may have to always enable IRQ_WORK even on non-SMP kernels now. In practice it is already enabled in most configurations for one reason or another, the the cost is likely very small.
Otherwise checking for CONFIG_HAVE_NMI in the new code might work.
ARM seems to have HAVE_NMI while also being one of the architectures that is now failing.
Right, in this case we would also need
--- a/arch/Kconfig +++ b/arch/Kconfig @@ -236,6 +236,7 @@ config HAVE_FUNCTION_ERROR_INJECTION config HAVE_NMI bool
select IRQ_WORKconfig HAVE_FUNCTION_DESCRIPTORS bool
Yeah, that works for me I suppose.
On Fri, Jul 12, 2024 at 05:03:33PM +0200, Peter Zijlstra wrote:
On Fri, Jul 12, 2024 at 04:11:54PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 15:24, Peter Zijlstra wrote:
On Fri, Jul 12, 2024 at 02:28:38PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 14:13, Naresh Kamboju wrote:
that we may have to always enable IRQ_WORK even on non-SMP kernels now. In practice it is already enabled in most configurations for one reason or another, the the cost is likely very small.
Otherwise checking for CONFIG_HAVE_NMI in the new code might work.
ARM seems to have HAVE_NMI while also being one of the architectures that is now failing.
Right, in this case we would also need
--- a/arch/Kconfig +++ b/arch/Kconfig @@ -236,6 +236,7 @@ config HAVE_FUNCTION_ERROR_INJECTION config HAVE_NMI bool
select IRQ_WORKconfig HAVE_FUNCTION_DESCRIPTORS bool
Yeah, that works for me I suppose.
Was there any conclusion to this thread that I missed? The configurations Naresh mentioned in the original post are now broken in mainline because the pull request was sent without any solution to this issue.
FWIW, that diff doesn't fix ARCH=powerpc tinyconfig, as it does not have CONFIG_HAVE_NMI. kernel/task_work.c is unconditionally built so shouldn't IRQ_WORK now be 'default y' (or just eliminated altogether)?
Cheers, Nathan
On Thu, Jul 18, 2024 at 09:25:27AM -0700, Nathan Chancellor wrote:
On Fri, Jul 12, 2024 at 05:03:33PM +0200, Peter Zijlstra wrote:
On Fri, Jul 12, 2024 at 04:11:54PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 15:24, Peter Zijlstra wrote:
On Fri, Jul 12, 2024 at 02:28:38PM +0200, Arnd Bergmann wrote:
On Fri, Jul 12, 2024, at 14:13, Naresh Kamboju wrote:
that we may have to always enable IRQ_WORK even on non-SMP kernels now. In practice it is already enabled in most configurations for one reason or another, the the cost is likely very small.
Otherwise checking for CONFIG_HAVE_NMI in the new code might work.
ARM seems to have HAVE_NMI while also being one of the architectures that is now failing.
Right, in this case we would also need
--- a/arch/Kconfig +++ b/arch/Kconfig @@ -236,6 +236,7 @@ config HAVE_FUNCTION_ERROR_INJECTION config HAVE_NMI bool
select IRQ_WORKconfig HAVE_FUNCTION_DESCRIPTORS bool
Yeah, that works for me I suppose.
Was there any conclusion to this thread that I missed? The configurations Naresh mentioned in the original post are now broken in mainline because the pull request was sent without any solution to this issue.
FWIW, that diff doesn't fix ARCH=powerpc tinyconfig, as it does not have CONFIG_HAVE_NMI. kernel/task_work.c is unconditionally built so shouldn't IRQ_WORK now be 'default y' (or just eliminated altogether)?
Actually, many tinyconfig builds are now broken.
Example from alpha:
alpha-linux-ld: kernel/task_work.o: in function `task_work_add': (.text+0x1b4): undefined reference to `irq_work_queue' alpha-linux-ld: (.text+0x1bc): undefined reference to `irq_work_queue'
alpha does not set "HAVE_NMI". AFAICS task_work.c is always built, and it now calls irq_work_queue() unconditionally, so unless that is changed IRQ_WORK is now mandatory and must not just depend on HAVE_NMI or anything else.
Guenter
-
Arnd Bergmann -
Guenter Roeck -
Naresh Kamboju -
Nathan Chancellor -
Peter Zijlstra