| Message ID | CAK8P3a2jgQcLaDXX6eOTNrU0RJ2O625e75LBMy6v2ABP0cdoww@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] asm-generic: updates for 6.0 | expand |
On Fri, Aug 5, 2022 at 3:25 AM Arnd Bergmann <arnd@kernel.org> wrote: > > - A patch from Sebastian Siewior to rework the handling of > IRQ stacks in softirqs across architectures, which is > needed for enabling PREEMPT_RT. I am quite fed up with the chaos and garbage that PREEMPT_RT has caused this release. Once again, this pull request contains senseless code because of a PREEMPT_RT patch that was merged based on some bogus "the RT code needs this". First off, the RT code isn't currently enabled in upstream kernels, so none of this merits any kind of big hurry and mindless "need to apply because it's a bug". Secondly, that patch is HORRENDOUSLY UGLY. I hereby ask every single maintainer to immediately stop taking these bogus patches that contain variations on random #ifdef CONFIG_PREEMPT_RT because they are clearly left-over turds from the RT tree that were unbelievably ugly hacks, and should never have been merged upstream. Why am I so upset? WE ALREADY HAVE A DIFFERENT CONFIG VARIABLE EXPLICITLY FOR THIS! In fact, you can *see* that config variable in the patch. There's a very specific HAVE_SOFTIRQ_ON_OWN_STACK variable that has the following help message (even if that help will never be shown because it's not an actual question, it's a helper config variable that gets selected): config HAVE_SOFTIRQ_ON_OWN_STACK bool help Architecture provides a function to run __do_softirq() on a separate stack. and that config variable ALREADY PROTECTS the do_softirq_own_stack() declaration in asm-generic. The very one you just added the CONFIG_PREEMPT_RT thing around. In other words, the RT patch is just mindless and ugly, and the right thing to do would have been (a) make HAVE_SOFTIRQ_ON_OWN_STACK have a depends on !PREEMPT_RT (b) as PREEMPT_RT is enabled one architecture at a time, you can make the architecture header files also use that HAVE_SOFTIRQ_ON_OWN_STACK thing, which makes a whole lot more sense than sprinkling random CONFIG_REEMPT_RT things around. I have pulled this, but I'm really *really* fed up with these PREEMPT_RT patches that add code that MAKES NO SENSE. In just this merge window: We had it in the dentry tree. Then we had it in the printk tree to the point where I refused to even pull it. And now we have it in the asm-generic tree too. The rule about RT patches has *always* been that we merge them as they become clean enough to make sense. That rule seems to have entirely flown out the window here, and suddenly it has become a sport to add random senseless #ifdef CONFIG_PREEMPT_RT lines to code. At least the dentry case had a nice big comment (which really was required exactly because the code made no sense on its own). This patch had nothing of the sort. PREEMPT_RT is special enough that it really needs to spend a _lot_ more time making the code sensible, rather than add random hacks like this. And when we have a config parameter that is *explicitly* about this very issue, we should use that one, not some PREEMPT_RT hack. And the RT tree has had literally decades where people tried very hard to do exactly that - make proper abstractions, and make sure that merging the RT patches made sense even outside the context of the RT code. Now suddenly all that "this code has to make sense" seems to be history. And it really shouldn't be. Linus
The pull request you sent on Fri, 5 Aug 2022 12:25:12 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git tags/asm-generic-6.0
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/3bd6e5854bf9bb5436d6b533e206561839e3b284
Thank you!
On Tue, Aug 9, 2022 at 5:57 PM Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > Remove the CONFIG_PREEMPT_RT symbol from the ifdef around > do_softirq_own_stack() and move it to Kconfig instead. > > Enable softirq stacks based on SOFTIRQ_ON_OWN_STACK which depends on > HAVE_SOFTIRQ_ON_OWN_STACK and its default value is set to !PREEMPT_RT. > This ensures that softirq stacks are not used on PREEMPT_RT and avoids > a 'select' statement on an option which has a 'depends' statement. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > arch/Kconfig | 4 ++++ > arch/arm/kernel/irq.c | 2 +- > arch/parisc/kernel/irq.c | 2 +- > arch/powerpc/kernel/irq.c | 4 ++-- > arch/s390/include/asm/softirq_stack.h | 2 +- > arch/sh/kernel/irq.c | 2 +- > arch/sparc/kernel/irq_64.c | 2 +- > arch/x86/include/asm/irq_stack.h | 2 +- > arch/x86/kernel/irq_32.c | 2 +- > include/asm-generic/softirq_stack.h | 2 +- Thanks for the patch, I assume Linus will want to pick this up himself. Let me know if I should give it a spin in the asm-generic tree first for additional build testing. Acked-by: Arnd Bergmann <arnd@arndb.de>
On Tue, Aug 09 2022 at 17:57, Sebastian Andrzej Siewior wrote: > > +config SOFTIRQ_ON_OWN_STACK > + def_bool !PREEMPT_RT > + depends on HAVE_SOFTIRQ_ON_OWN_STACK def_bool !PREEMPT_RT && HAVE_SOFTIRQ_ON_OWN_STACK No?
The following changes since commit b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3: Linux 5.19-rc2 (2022-06-12 16:11:37 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git tags/asm-generic-6.0 for you to fetch changes up to 6f05e014b96c8846cdc39acdf10bbdbafb9c78a0: uapi: asm-generic: fcntl: Fix typo 'the the' in comment (2022-07-22 14:54:22 +0200) ---------------------------------------------------------------- asm-generic: updates for 6.0 There are three independent sets of changes: - Sai Prakash Ranjan adds tracing support to the asm-generic version of the MMIO accessors, which is intended to help understand problems with device drivers and has been part of Qualcomm's vendor kernels for many years. - A patch from Sebastian Siewior to rework the handling of IRQ stacks in softirqs across architectures, which is needed for enabling PREEMPT_RT. - The last patch to remove the CONFIG_VIRT_TO_BUS option and some of the code behind that, after the last users of this old interface made it in through the netdev, scsi, media and staging trees. ---------------------------------------------------------------- Arnd Bergmann (2): Merge branch 'asm-generic-mmiotrace' into asm-generic arch/*/: remove CONFIG_VIRT_TO_BUS Prasad Sodagudi (1): lib: Add register read/write tracing support Sai Prakash Ranjan (8): arm64: io: Use asm-generic high level MMIO accessors coresight: etm4x: Use asm-generic IO memory barriers irqchip/tegra: Fix overflow implicit truncation warnings drm/meson: Fix overflow implicit truncation warnings KVM: arm64: Add a flag to disable MMIO trace for nVHE KVM asm-generic/io: Add logging support for MMIO accessors serial: qcom_geni_serial: Disable MMIO tracing for geni serial soc: qcom: geni: Disable MMIO tracing for GENI SE Sebastian Andrzej Siewior (1): arch/*: Disable softirq stacks on PREEMPT_RT. Slark Xiao (1): uapi: asm-generic: fcntl: Fix typo 'the the' in comment Documentation/core-api/bus-virt-phys-mapping.rst | 220 --------------------- Documentation/core-api/dma-api-howto.rst | 14 -- Documentation/core-api/index.rst | 1 - .../translations/zh_CN/core-api/index.rst | 1 - arch/Kconfig | 3 + arch/alpha/Kconfig | 1 - arch/alpha/include/asm/floppy.h | 2 +- arch/alpha/include/asm/io.h | 8 +- arch/arm/kernel/irq.c | 3 +- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/io.h | 41 +--- arch/arm64/kvm/hyp/nvhe/Makefile | 7 +- arch/ia64/Kconfig | 1 - arch/ia64/include/asm/io.h | 8 - arch/m68k/Kconfig | 1 - arch/m68k/include/asm/virtconvert.h | 4 +- arch/microblaze/Kconfig | 1 - arch/microblaze/include/asm/io.h | 2 - arch/mips/Kconfig | 1 - arch/mips/include/asm/io.h | 9 - arch/parisc/Kconfig | 1 - arch/parisc/include/asm/floppy.h | 4 +- arch/parisc/include/asm/io.h | 2 - arch/parisc/kernel/irq.c | 2 + arch/powerpc/Kconfig | 1 - arch/powerpc/include/asm/io.h | 2 - arch/powerpc/kernel/irq.c | 4 + arch/riscv/include/asm/page.h | 1 - arch/s390/include/asm/softirq_stack.h | 3 +- arch/sh/kernel/irq.c | 2 + arch/sparc/kernel/irq_64.c | 2 + arch/x86/Kconfig | 1 - arch/x86/include/asm/io.h | 9 - arch/xtensa/Kconfig | 1 - arch/xtensa/include/asm/io.h | 3 - drivers/gpu/drm/meson/meson_viu.c | 22 +-- drivers/hwtracing/coresight/coresight-etm4x-core.c | 8 +- drivers/hwtracing/coresight/coresight-etm4x.h | 8 +- drivers/irqchip/irq-tegra.c | 10 +- drivers/soc/qcom/qcom-geni-se.c | 3 + drivers/tty/serial/qcom_geni_serial.c | 3 + include/asm-generic/io.h | 105 ++++++++-- include/asm-generic/softirq_stack.h | 2 +- include/trace/events/rwmmio.h | 97 +++++++++ lib/Kconfig | 7 + lib/Makefile | 2 + lib/trace_readwrite.c | 47 +++++ mm/Kconfig | 8 - tools/include/uapi/asm-generic/fcntl.h | 2 +- 49 files changed, 314 insertions(+), 377 deletions(-) delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst create mode 100644 include/trace/events/rwmmio.h create mode 100644 lib/trace_readwrite.c