On Mon, Jan 26, 2026 at 09:31AM -0800, Bart Van Assche wrote:
> On 12/19/25 7:40 AM, Marco Elver wrote:
> > +/*
> > + * No-op helper to denote that ssp must be held. Because SRCU-protected
> > pointers
> > + * should still be marked with __rcu_guarded, and we do not want to mark
> > them
> > + * with __guarded_by(ssp) as it would complicate annotations for writers,
> > we
> > + * choose the following strategy: srcu_dereference_check() calls this
> > helper
> > + * that checks that the passed ssp is held, and then fake-acquires 'RCU'.
> > + */
> > +static inline void __srcu_read_lock_must_hold(const struct srcu_struct
> > *ssp) __must_hold_shared(ssp) { }
> > /**
> > * srcu_dereference_check - fetch SRCU-protected pointer for later
> > dereferencing
> > @@ -223,9 +233,15 @@ static inline int srcu_read_lock_held(const struct
> > srcu_struct *ssp)
> > * to 1. The @c argument will normally be a logical expression containing
> > * lockdep_is_held() calls.
> > */
> > -#define srcu_dereference_check(p, ssp, c) \
> > - __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
> > - (c) || srcu_read_lock_held(ssp), __rcu)
> > +#define srcu_dereference_check(p, ssp, c)
> > \
> > +({
> > \
> > + __srcu_read_lock_must_hold(ssp);
> > \
> > + __acquire_shared_ctx_lock(RCU); \
> > + __auto_type __v = __rcu_dereference_check((p), __UNIQUE_ID(rcu),
> > \
> > + (c) || srcu_read_lock_held(ssp), __rcu);
> > \
> > + __release_shared_ctx_lock(RCU); \
> > + __v;
> > \
> > +})
>
> Hi Marco,
>
> The above change is something I'm not happy about. The original
> implementation of the srcu_dereference_check() macro shows that it is
> sufficient to either hold an SRCU reader lock or the updater lock ('c').
> The addition of "__srcu_read_lock_must_hold()" will cause compilation to
> fail if the caller doesn't hold an SRCU reader lock. I'm concerned that
> this will either lead to adding __no_context_analysis to SRCU updater
> code that uses srcu_dereference_check() or to adding misleading
> __assume_ctx_lock(ssp) annotations in SRCU updater code.
Right, and it doesn't help 'c' is an arbitrary condition. But it's
fundamentally difficult to say "hold either this or that lock".
That being said, I don't think it's wrong to write e.g.:
spin_lock(&updater_lock);
__acquire_shared(ssp);
...
// writes happen through rcu_assign_pointer()
// reads can happen through srcu_dereference_check()
...
__release_shared(ssp);
spin_unlock(&updater_lock);
, given holding the updater lock implies reader access.
And given the analysis is opt-in (CONTEXT_ANALYSIS := y), I think
it's a manageable problem.
If you have a different idea how we can solve this, please let us know.
One final note, usage of srcu_dereference_check() is rare enough:
arch/x86/kvm/hyperv.c: irq_rt =
srcu_dereference_check(kvm->irq_routing, &kvm->irq_srcu,
arch/x86/kvm/x86.c:
kvm_free_msr_filter(srcu_dereference_check(kvm->arch.msr_filter, &kvm->srcu,
1));
arch/x86/kvm/x86.c:
kfree(srcu_dereference_check(kvm->arch.pmu_event_filter, &kvm->srcu, 1));
drivers/gpio/gpiolib.c: label = srcu_dereference_check(desc->label,
&desc->gdev->desc_srcu,
drivers/hv/mshv_irq.c: girq_tbl =
srcu_dereference_check(partition->pt_girq_tbl,
drivers/hwtracing/stm/core.c: link =
srcu_dereference_check(src->link, &stm_source_srcu, 1);
drivers/infiniband/hw/hfi1/user_sdma.c: pq =
srcu_dereference_check(fd->pq, &fd->pq_srcu,
fs/quota/dquot.c: struct dquot *dquot =
srcu_dereference_check(
fs/quota/dquot.c: struct dquot *dquot =
srcu_dereference_check(
fs/quota/dquot.c: put[cnt] =
srcu_dereference_check(dquots[cnt], &dquot_srcu,
fs/quota/dquot.c: transfer_from[cnt] =
srcu_dereference_check(dquots[cnt],
include/linux/kvm_host.h: return
srcu_dereference_check(kvm->memslots[as_id], &kvm->srcu,
virt/kvm/irqchip.c: irq_rt =
srcu_dereference_check(kvm->irq_routing, &kvm->irq_srcu,
, that I think it's easy enough to annotate these places with the above
suggestions in case you're trying out global enablement.
