On Tue, Sep 01, 2026 at 03:48:15PM +0800, Kunwu Chan wrote:
> From: Kunwu Chan <[email protected]>
>
> Set srcu_reader_flavor to SRCU_READ_FLAVOR_ATOMIC in the Tiny SRCU
> atomic initialization paths, so that the entry-point checks added by
> the previous commit can identify atomic SRCU domains.
>
> For static initialization, add a flavor parameter to
> __SRCU_STRUCT_INIT() and pass SRCU_READ_FLAVOR_ATOMIC through
> DEFINE_SRCU_ATOMIC() and DEFINE_STATIC_SRCU_ATOMIC().
>
> For dynamic initialization, initialize srcu_reader_flavor to zero in
> init_srcu_struct_fields() for the generic initialization path, and
> set it to SRCU_READ_FLAVOR_ATOMIC in init_srcu_struct_atomic().
> Handle both CONFIG_DEBUG_LOCK_ALLOC and non-debug initialization paths.
>
> Enable srcu_check_read_flavor() for Tiny SRCU, matching the Tree SRCU
> behavior, so that readers can verify that the requested flavor matches
> the SRCU domain.
>
> Signed-off-by: Kunwu Chan <[email protected]>
Much better! Please see below for additional questions and comments.
> ---
> include/linux/srcutiny.h | 39 +++++++++++++++++++++++++++++----------
> kernel/rcu/srcutiny.c | 1 +
> 2 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> index 2b293336525a..9dce4b5aa084 100644
> --- a/include/linux/srcutiny.h
> +++ b/include/linux/srcutiny.h
> @@ -40,7 +40,7 @@ void srcu_drive_gp(struct work_struct *wp);
> void srcu_tiny_irq_work(struct irq_work *irq_work);
> void srcu_defer_drain(struct irq_work *irq_work);
>
> -#define __SRCU_STRUCT_INIT(name, __ignored, ___ignored, ____ignored) \
> +#define __SRCU_STRUCT_INIT(name, __ignored, ___ignored, flavor)
> \
> { \
> .srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
> .srcu_cb_tail = &name.srcu_cb_head, \
> @@ -49,6 +49,7 @@ void srcu_defer_drain(struct irq_work *irq_work);
> .defer_cbs = LLIST_HEAD_INIT(name.defer_cbs), \
> .defer_iw = { .node = { .u_flags = IRQ_WORK_HARD_IRQ }, \
> .func = srcu_defer_drain }, \
> + .srcu_reader_flavor = flavor, \
Don't we also need to define srcu_reader_flavor? Or is this patch
supposed to be applied on top of your previous one?
> __SRCU_DEP_MAP_INIT(name) \
> }
>
> @@ -57,29 +58,43 @@ void srcu_defer_drain(struct irq_work *irq_work);
> * Tree SRCU, which needs some per-CPU data.
> */
> #define DEFINE_SRCU(name) \
> - struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, name)
> + struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, 0)
> #define DEFINE_STATIC_SRCU(name) \
> - static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name,
> name)
> + static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, 0)
> #define DEFINE_SRCU_FAST(name) DEFINE_SRCU(name)
> #define DEFINE_STATIC_SRCU_FAST(name) \
> - static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name,
> name)
> + static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, 0)
> #define DEFINE_SRCU_FAST_UPDOWN(name) DEFINE_SRCU(name)
> #define DEFINE_STATIC_SRCU_FAST_UPDOWN(name) \
> - static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name,
> name)
> -#define DEFINE_SRCU_ATOMIC(name) DEFINE_SRCU(name)
> + static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, 0)
> +#define DEFINE_SRCU_ATOMIC(name) \
> + struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name,
> SRCU_READ_FLAVOR_ATOMIC)
> #define DEFINE_STATIC_SRCU_ATOMIC(name) \
> - static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name,
> name)
> + static struct srcu_struct name = \
> + __SRCU_STRUCT_INIT(name, name, name, SRCU_READ_FLAVOR_ATOMIC)
>
> // Dummy structure for srcu_notifier_head.
> struct srcu_usage { };
> #define __SRCU_USAGE_INIT(name) { }
> #define __init_srcu_struct_fast __init_srcu_struct
> #define __init_srcu_struct_fast_updown __init_srcu_struct
> -#define __init_srcu_struct_atomic __init_srcu_struct
> +#define __init_srcu_struct_atomic(ssp, name, key) \
> +({ \
> + int __ret = __init_srcu_struct(ssp, name, key); \
> + if (!__ret) \
> + (ssp)->srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC; \
> + __ret; \
> +})
> #ifndef CONFIG_DEBUG_LOCK_ALLOC
> #define init_srcu_struct_fast init_srcu_struct
> #define init_srcu_struct_fast_updown init_srcu_struct
> -#define init_srcu_struct_atomic init_srcu_struct
> +#define init_srcu_struct_atomic(ssp) \
> + ({ \
> + int __ret = init_srcu_struct(ssp); \
> + if (!__ret) \
> + (ssp)->srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC;\
> + __ret; \
> + })
> #endif // #ifndef CONFIG_DEBUG_LOCK_ALLOC
>
> void synchronize_srcu(struct srcu_struct *ssp);
> @@ -148,7 +163,11 @@ static inline void synchronize_srcu_expedited(struct
> srcu_struct *ssp)
> void srcu_barrier(struct srcu_struct *ssp);
>
> static inline void srcu_expedite_current(struct srcu_struct *ssp) { }
> -#define srcu_check_read_flavor(ssp, read_flavor) do { } while (0)
> +#define srcu_check_read_flavor(ssp, read_flavor) \
> + ({ \
> + u8 __f = (ssp)->srcu_reader_flavor; \
> + WARN_ON_ONCE(__f && !(__f & (read_flavor))); \
> + })
Why not make this a static inline function, along with
__init_srcu_struct_atomic() and init_srcu_struct_atomic() above?
That should simplify the code.
> /* Defined here to avoid size increase for non-torture kernels. */
> static inline void srcu_torture_stats_print(struct srcu_struct *ssp,
> diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> index 22f7716cbb0e..873b30ccf563 100644
> --- a/kernel/rcu/srcutiny.c
> +++ b/kernel/rcu/srcutiny.c
> @@ -42,6 +42,7 @@ static int init_srcu_struct_fields(struct srcu_struct *ssp)
> ssp->srcu_gp_running = false;
> ssp->srcu_gp_waiting = false;
> ssp->srcu_atomic_gp_flag = 0;
> + ssp->srcu_reader_flavor = 0;
> ssp->srcu_idx = 0;
> ssp->srcu_idx_max = 0;
> INIT_WORK(&ssp->srcu_work, srcu_drive_gp);
> --
> 2.43.0
One of the purposes of Tiny SRCU is to be tiny, and one could argue
that Tiny SRCU already has too much diagnostic code. Should we include
these patches at all? If we should, shouldn't they be under something
like CONFIG_PROVE_RCU? Should some of the existing Tiny SRCU debugging
code also be under something like CONFIG_PROVE_RCU?
Thoughts?
Thanx, Paul
