On Fri, Oct 14, 2022 at 6:47 PM Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> wrote: > > On Thu, Sep 29, 2022 at 11:07:14AM -0700, Paul E. McKenney wrote: > > Hello! > > > > This RFC series provides the second version of an NMI-safe SRCU reader API > > in the guise of srcu_read_lock_nmisafe() and srcu_read_unlock_nmisafe(). > > Just a comment about high-level use of the new NMI-safe SRCU api: say if for > certain arch, NMI cannot be interrupted (by breakpoint or something), then > using NMI-safe API will force such arch to potentially use more expensive RMW > atomic than the previously cheap local non-atomic operations that the arch > was able to get away with. > > Thoughts? Otherwise, LGTM. > I take it back. You are indeed guarding it correctly as below. I got confused by another patch that was doing debug checking even for arch that does not need it (which is a good thing). +config NEED_SRCU_NMI_SAFE + def_bool HAVE_NMI && !ARCH_HAS_NMI_SAFE_THIS_CPU_OPS && !TINY_SRCU + Thanks! - Joel > thanks, > > - Joel > > > > A given srcu_struct structure must use either the traditional > > srcu_read_lock() and srcu_read_unlock() API or the new _nmisafe() API: > > Mixing and matching is not permitted. So much so that kernels built > > with CONFIG_PROVE_RCU=y will complain if you try it. > > > > The reason for this restriction is that I have yet to find a use case > > that is not a accident waiting to happen. And if free intermixing > > were permitted, it is pretty much a given that someone somewhere will > > get confused and use srcu_read_lock_nmisafe() within NMI handlers and > > srcu_read_lock() elsewhere, which will not (repeat, NOT) provide NMI > > safety. > > > > I do not expect to push this into the v6.1 merge window. However, if > > the printk() series that needs it goes in, then I will push it as a fix > > for the resulting regression. > > > > The series is as follows: > > > > 1. Convert ->srcu_lock_count and ->srcu_unlock_count to atomic. > > > > 2. Create an srcu_read_lock_nmisafe() and srcu_read_unlock_nmisafe(). > > > > 3. Check for consistent per-CPU per-srcu_struct NMI safety. > > > > 4. Check for consistent global per-srcu_struct NMI safety. > > > > 5. Add ARCH_HAS_NMI_SAFE_THIS_CPU_OPS Kconfig option. > > > > 6. Add ARCH_HAS_NMI_SAFE_THIS_CPU_OPS Kconfig option. > > > > 7. Add ARCH_HAS_NMI_SAFE_THIS_CPU_OPS Kconfig option. > > > > 8. Add ARCH_HAS_NMI_SAFE_THIS_CPU_OPS Kconfig option. > > > > Changes since v1 RFC: > > > > 1. Added enabling patches for arm64, loongarch, s390, and x86. > > These have what appear to me to be NMI-safe this_cpu_inc() > > implementations. > > > > 2. Fix a build error on !SMP kernels built without SRCU. > > > > 3. Fix a build error on !SMP kernels. > > > > Thanx, Paul > > > > ------------------------------------------------------------------------ > > > > b/arch/arm64/Kconfig | 1 > > b/arch/loongarch/Kconfig | 1 > > b/arch/s390/Kconfig | 1 > > b/arch/x86/Kconfig | 1 > > b/include/linux/srcu.h | 39 +++++++++++++++++++++ > > b/include/linux/srcutiny.h | 11 ++++++ > > b/include/linux/srcutree.h | 4 +- > > b/kernel/rcu/Kconfig | 3 + > > b/kernel/rcu/rcutorture.c | 11 ++++-- > > b/kernel/rcu/srcutree.c | 24 ++++++------- > > include/linux/srcu.h | 4 +- > > include/linux/srcutiny.h | 4 +- > > include/linux/srcutree.h | 12 +++++- > > kernel/rcu/srcutree.c | 82 +++++++++++++++++++++++++++++++++++++++------ > > 14 files changed, 166 insertions(+), 32 deletions(-)