On Tue, Jan 02, 2024 at 02:28:04AM -0800, Christoph Hellwig wrote: > On Sun, Dec 31, 2023 at 12:05:29PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > > > Make it so that we can switch between notifier chain implementations for > > testing purposes. On the author's test system, calling an empty srcu > > notifier chain cost about 19ns per call, vs. 4ns for a blocking notifier > > chain. Hm. Might we actually want regular blocking notifiers? > > Sounds like it. But what is important is that we really shouldn't > provide both and punt the decision to the user.. How about this for a commit message: "Originally, I selected srcu notifiers to implement live hooks because they seemed to have fewer impacts to scalability. The per-call cost of srcu_notifier_call_chain is lower (19ns) than blocking_notifier_ (4ns), but the latter takes an rwsem. IIRC, rwsems have scalability problems when the cpu count gets high due to all the cacheline bouncing and atomic operations. I didn't want regular xfs operations to suffer memory contention on the blocking notifiers for the sake of something that won't be running most of the time. "Therefore, I stuck with srcu notifiers, despite trading off single threaded performance for multithreaded performance. I wasn't thrilled with the very high teardown time for srcu notifiers, since the caller has to wait for the next rcu grace period. "Then I discovered static branches. "Now suddenly I had a tool to reduce the pain of a high-contention rwsem to zero except in the case where scrub is running. This seemed a lot better to me -- zero runtime overhead when scrub is not running; low setup and teardown overhead for scrub; and cacheline bouncing problems only when there are a lot of threads running through the notifier call code *and* scrub is running. "This seems perfect, but static branches aren't supported on all the architectures that Linux supports. Further, I haven't really tested the impacts of scrub on big iron. This makes me hesitant to get rid of the SRCU notifier implementation while online fsck is still experimental. "Note that Kconfig automatically selects the best option for a particular architecture. Kernel builders should take the defaults." I dunno. Do you want me to rip out the srcu implementation and only provide the blocking notifiers? That's easy to do, though hard to undo once I've done it. Hmm. How many arches support static branches today? $ grep HAVE_ARCH_JUMP_LABEL arch/ arch/arc/Kconfig:52: select HAVE_ARCH_JUMP_LABEL if ISA_ARCV2 && !CPU_ENDIAN_BE32 arch/arm/Kconfig:77: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU arch/arm64/Kconfig:163: select HAVE_ARCH_JUMP_LABEL arch/arm64/Kconfig:164: select HAVE_ARCH_JUMP_LABEL_RELATIVE arch/csky/Kconfig:71: select HAVE_ARCH_JUMP_LABEL if !CPU_CK610 arch/csky/Kconfig:72: select HAVE_ARCH_JUMP_LABEL_RELATIVE arch/mips/Kconfig:53: select HAVE_ARCH_JUMP_LABEL arch/parisc/Kconfig:60: select HAVE_ARCH_JUMP_LABEL arch/parisc/Kconfig:61: select HAVE_ARCH_JUMP_LABEL_RELATIVE arch/powerpc/Kconfig:212: select HAVE_ARCH_JUMP_LABEL arch/powerpc/Kconfig:213: select HAVE_ARCH_JUMP_LABEL_RELATIVE arch/riscv/Kconfig:96: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL arch/riscv/Kconfig:97: select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL arch/s390/Kconfig:151: select HAVE_ARCH_JUMP_LABEL arch/s390/Kconfig:152: select HAVE_ARCH_JUMP_LABEL_RELATIVE arch/sparc/Kconfig:31: select HAVE_ARCH_JUMP_LABEL if SPARC64 arch/x86/Kconfig:176: select HAVE_ARCH_JUMP_LABEL arch/x86/Kconfig:177: select HAVE_ARCH_JUMP_LABEL_RELATIVE arch/xtensa/Kconfig:33: select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL arch/loongarch/Kconfig:94: select HAVE_ARCH_JUMP_LABEL arch/loongarch/Kconfig:95: select HAVE_ARCH_JUMP_LABEL_RELATIVE The main arches that xfs really cares about are arm64, ppc64, riscv, s390x, and x86_64, right? Perhaps there's a stronger case for only providing blocking notifiers and jump labels since there aren't many m68k xfs users, right? --D