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