| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <eea8f144-70bf-4444-969b-bdd263bddf48@paulmck-laptop>
Date: Wed, 24 Sep 2025 01:56:28 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Zqiang <qiang.zhang@...ux.dev>
Cc: rcu@...r.kernel.org, linux-kernel@...r.kernel.org, kernel-team@...a.com,
rostedt@...dmis.org, Andrii Nakryiko <andrii@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, bpf@...r.kernel.org
Subject: Re: [PATCH 23/34] srcu: Create an srcu_expedite_current() function
On Wed, Sep 24, 2025 at 12:10:22AM +0000, Zqiang wrote:
> >
> > This commit creates an srcu_expedite_current() function that expedites
> > the current (and possibly the next) SRCU grace period for the specified
> > srcu_struct structure. This functionality will be inherited by RCU
> > Tasks Trace courtesy of its mapping to SRCU fast.
> >
> > If the current SRCU grace period is already waiting, that wait will
> > complete before the expediting takes effect. If there is no SRCU grace
> > period in flight, this function might well create one.
> >
> > Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
> > Cc: Andrii Nakryiko <andrii@...nel.org>
> > Cc: Alexei Starovoitov <ast@...nel.org>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Cc: <bpf@...r.kernel.org>
> > ---
> > include/linux/srcutiny.h | 1 +
> > include/linux/srcutree.h | 8 ++++++
> > kernel/rcu/srcutree.c | 58 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 67 insertions(+)
> >
> > diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> > index 00e5f05288d5e7..941e8210479607 100644
> > --- a/include/linux/srcutiny.h
> > +++ b/include/linux/srcutiny.h
> > @@ -104,6 +104,7 @@ static inline void srcu_barrier(struct srcu_struct *ssp)
> > synchronize_srcu(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_force(ssp, read_flavor) do { } while (0)
> >
> > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > index 1adc58d2ab6425..4a5d1c0e7db361 100644
> > --- a/include/linux/srcutree.h
> > +++ b/include/linux/srcutree.h
> > @@ -42,6 +42,8 @@ struct srcu_data {
> > struct timer_list delay_work; /* Delay for CB invoking */
> > struct work_struct work; /* Context for CB invoking. */
> > struct rcu_head srcu_barrier_head; /* For srcu_barrier() use. */
> > + struct rcu_head srcu_ec_head; /* For srcu_expedite_current() use. */
> > + int srcu_ec_state; /* State for srcu_expedite_current(). */
> > struct srcu_node *mynode; /* Leaf srcu_node. */
> > unsigned long grpmask; /* Mask for leaf srcu_node */
> > /* ->srcu_data_have_cbs[]. */
> > @@ -135,6 +137,11 @@ struct srcu_struct {
> > #define SRCU_STATE_SCAN1 1
> > #define SRCU_STATE_SCAN2 2
> >
> > +/* Values for srcu_expedite_current() state (->srcu_ec_state). */
> > +#define SRCU_EC_IDLE 0
> > +#define SRCU_EC_PENDING 1
> > +#define SRCU_EC_REPOST 2
> > +
> > /*
> > * Values for initializing gp sequence fields. Higher values allow wrap arounds to
> > * occur earlier.
> > @@ -220,6 +227,7 @@ struct srcu_struct {
> > int __srcu_read_lock(struct srcu_struct *ssp) __acquires(ssp);
> > void synchronize_srcu_expedited(struct srcu_struct *ssp);
> > void srcu_barrier(struct srcu_struct *ssp);
> > +void srcu_expedite_current(struct srcu_struct *ssp);
> > void srcu_torture_stats_print(struct srcu_struct *ssp, char *tt, char *tf);
> >
> > // Converts a per-CPU pointer to an ->srcu_ctrs[] array element to that
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 1ff94b76d91f15..f2f11492e6936e 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -1688,6 +1688,64 @@ void srcu_barrier(struct srcu_struct *ssp)
> > }
> > EXPORT_SYMBOL_GPL(srcu_barrier);
> >
> > +/* Callback for srcu_expedite_current() usage. */
> > +static void srcu_expedite_current_cb(struct rcu_head *rhp)
> > +{
> > + unsigned long flags;
> > + bool needcb = false;
> > + struct srcu_data *sdp = container_of(rhp, struct srcu_data, srcu_ec_head);
> > +
> > + spin_lock_irqsave_sdp_contention(sdp, &flags);
> > + if (sdp->srcu_ec_state == SRCU_EC_IDLE) {
> > + WARN_ON_ONCE(1);
> > + } else if (sdp->srcu_ec_state == SRCU_EC_PENDING) {
> > + sdp->srcu_ec_state = SRCU_EC_IDLE;
> > + } else {
> > + WARN_ON_ONCE(sdp->srcu_ec_state != SRCU_EC_REPOST);
> > + sdp->srcu_ec_state = SRCU_EC_PENDING;
> > + needcb = true;
> > + }
> > + spin_unlock_irqrestore_rcu_node(sdp, flags);
> > + // If needed, requeue ourselves as an expedited SRCU callback.
> > + if (needcb)
> > + __call_srcu(sdp->ssp, &sdp->srcu_ec_head, srcu_expedite_current_cb, false);
> > +}
> > +
> > +/**
> > + * srcu_expedite_current - Expedite the current SRCU grace period
> > + * @ssp: srcu_struct to expedite.
> > + *
> > + * Cause the current SRCU grace period to become expedited. The grace
> > + * period following the current one might also be expedited. If there is
> > + * no current grace period, one might be created. If the current grace
> > + * period is currently sleeping, that sleep will complete before expediting
> > + * will take effect.
> > + */
> > +void srcu_expedite_current(struct srcu_struct *ssp)
> > +{
> > + unsigned long flags;
> > + bool needcb = false;
> > + struct srcu_data *sdp;
> > +
> > + preempt_disable();
> > + sdp = this_cpu_ptr(ssp->sda);
> > + spin_lock_irqsave_sdp_contention(sdp, &flags);
>
> For PREEMPT_RT kernels, a locking warnings may occur here.
As in the above "preempt_disable()" needs to be "migrate_disable()",
correct?
One could argue that preempt_disable() is faster and that it works fine
in !PREEMPT_RT kernels, but srcu_expedite_current() should not be called
all that frequently and is relatively high overhead anyway.
Or did you have some other fix in mind?
> > + if (sdp->srcu_ec_state == SRCU_EC_IDLE) {
> > + sdp->srcu_ec_state = SRCU_EC_PENDING;
> > + needcb = true;
> > + } else if (sdp->srcu_ec_state == SRCU_EC_PENDING) {
> > + sdp->srcu_ec_state = SRCU_EC_REPOST;
> > + } else {
> > + WARN_ON_ONCE(sdp->srcu_ec_state != SRCU_EC_REPOST);
> > + }
> > + spin_unlock_irqrestore_rcu_node(sdp, flags);
> > + // If needed, queue an expedited SRCU callback.
> > + if (needcb)
> > + __call_srcu(ssp, &sdp->srcu_ec_head, srcu_expedite_current_cb, false);
>
> The locking warnings may also occur in the __call_srcu().
And use of migrate_disable() fixes things here, correct?
Thanx, Paul
> Thanks
> Zqiang
>
> > + preempt_enable();
> > +}
> > +EXPORT_SYMBOL_GPL(srcu_expedite_current);
> > +
> > /**
> > * srcu_batches_completed - return batches completed.
> > * @ssp: srcu_struct on which to report batch completion.
> > --
> > 2.40.1
> >
Powered by blists - more mailing lists