On Sun, Dec 31, 2023 at 12:05:13PM -0800, Darrick J. Wong wrote: > On the author's computer, calling an empty srcu notifier chain was > observed to have an overhead averaging ~40ns with a maximum of 60ns. > Adding a no-op notifier function increased the average to ~58ns and > 66ns. When the quotacheck live update notifier is attached, the average > increases to ~322ns with a max of 372ns to update scrub's in-memory > observation data, assuming no lock contention. > > With jump labels enabled, calls to empty srcu notifier chains are elided > from the call sites when there are no hooks registered, which means that > the overhead is 0.36ns when fsck is not running. For compilers that do > not support jump labels (all major architectures do), the overhead of a > no-op notifier call is less bad (on a many-cpu system) than the atomic > counter ops, so we make the hook switch itself a nop. Based on the next patch it seems like blocking notifier are the way to go and thus this patch should switch to using them and the above needs updates. > Note: This new code is also split out as a separate patch from its > initial user so that the author can move patches around his tree with > ease. For the final merge candidate at least this comment should go away, and maybe also the split.. > +config XFS_LIVE_HOOKS > + bool > + select JUMP_LABEL if HAVE_ARCH_JUMP_LABEL > + > config XFS_ONLINE_SCRUB > bool "XFS online metadata check support" > default n > depends on XFS_FS > depends on TMPFS && SHMEM > + select XFS_LIVE_HOOKS > select XFS_DRAIN_INTENTS I'm a bit confused by all the extra Kconfig options here. Why do we need XFS_LIVE_HOOKS, or the existing XFS_DRAIN_INTENTS instead of just switching the ifdefs to XFS_ONLINE_SCRUB and selecting JUMP_LABEL if HAVE_ARCH_JUMP_LABEL from XFS_ONLINE_SCRUB? Also while I'm at random Kconfig critique, n is the default default and the default n here can be dropped. I might just send a patch for that instead of bothering you once this series is in, though. Otherwise this looks good except for the choice of which notifier type to use.
