>
> On Fri, Nov 03, 2023 at 03:14:11PM +0800, Z qiang wrote:
> > >
> > > On Wed, Nov 01, 2023 at 11:35:07AM +0800, Zqiang wrote:
> > > > Currently, when running the rcutorture testing, if the fqs_task
> > > > kthread was created, the periodic fqs operations will be performed,
> > > > regardless of whether the grace-period is ongoing. however, if there
> > > > is no ongoing grace-period, invoke the rcu_force_quiescent_state() has
> > > > no effect, because when the new grace-period starting, will clear all
> > > > flags int rcu_state.gp_flags in rcu_gp_init(). this commit therefore add
> > > > rcu_gp_in_progress() check in rcu_force_quiescent_state(), if there is
> > > > no ongoing grace-period, return directly.
> > > >
> > > > Signed-off-by: Zqiang <qiang.zhang1211@xxxxxxxxx>
> > >
> > > Nice optimization, but one question below.
> > >
> > > Thanx, Paul
> > >
> > > > ---
> > > > kernel/rcu/tree.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > > index aa4c808978b8..5b4279ef66da 100644
> > > > --- a/kernel/rcu/tree.c
> > > > +++ b/kernel/rcu/tree.c
> > > > @@ -2338,6 +2338,8 @@ void rcu_force_quiescent_state(void)
> > > > struct rcu_node *rnp;
> > > > struct rcu_node *rnp_old = NULL;
> > > >
> > > > + if (!rcu_gp_in_progress())
> > > > + return;
> > >
> > > Suppose that the grace period that was in progress above ends right
> > > at this point in the code. We will still do the useless grace
> > > forcing of quiescent states. Which means that this code path
> > > does need to be tested.
> > >
> > > So, when you run rcutorture with this change, how often has the
> > > grace period ended before this function returns? If that happens
> > > reasonably often, say more than once per minute or so, then this
> > > works nicely. If not, we do need to do something to make sure
> > > that that code path gets tested.
> > >
> > > Thoughts?
> >
> > Thanks for the suggestion, I will add some debug information to test again.
>
> Very good, and I look forward to seeing what you come up with!
>
Hi, Paul
I added some debug code to run rcu torture tests:
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index 98e13be411af..248e13e1ccd6 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -548,6 +548,9 @@ void do_trace_rcu_torture_read(const char *rcutorturename,
unsigned long c_old,
unsigned long c);
void rcu_gp_set_torture_wait(int duration);
+void rcutorture_fqs_set(bool on);
+unsigned long rcutorture_fqs_nr(void);
+unsigned long rcutorture_fqs_valid_nr(void);
#else
static inline void rcutorture_get_gp_data(enum rcutorture_type test_type,
int *flags, unsigned long *gp_seq)
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 9bd6856135d7..15f506c26df3 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -1179,6 +1179,7 @@ rcu_torture_fqs(void *arg)
int oldnice = task_nice(current);
VERBOSE_TOROUT_STRING("rcu_torture_fqs task started");
+ rcutorture_fqs_set(true);
do {
fqs_resume_time = jiffies + fqs_stutter * HZ;
while (time_before(jiffies, fqs_resume_time) &&
@@ -1195,6 +1196,7 @@ rcu_torture_fqs(void *arg)
if (stutter_wait("rcu_torture_fqs"))
sched_set_normal(current, oldnice);
} while (!torture_must_stop());
+ rcutorture_fqs_set(false);
torture_kthread_stopping("rcu_torture_fqs");
return 0;
}
@@ -2213,6 +2215,7 @@ rcu_torture_stats_print(void)
pr_cont("read-exits: %ld ", data_race(n_read_exits)); // Statistic.
pr_cont("nocb-toggles: %ld:%ld\n",
atomic_long_read(&n_nocb_offload),
atomic_long_read(&n_nocb_deoffload));
+ pr_cont("nr_fqs: %ld:%ld\n", rcutorture_fqs_valid_nr(),
rcutorture_fqs_nr());
pr_alert("%s%s ", torture_type, TORTURE_FLAG);
if (atomic_read(&n_rcu_torture_mberror) ||
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index cb1caefa8bd0..9ae0c442e552 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2299,6 +2299,38 @@ static void force_qs_rnp(int (*f)(struct rcu_data *rdp))
}
}
+static bool rcu_fqs_enable;
+static unsigned long fqs_valid_nr;
+static unsigned long fqs_nr;
+
+void rcutorture_fqs_set(bool on)
+{
+ WRITE_ONCE(rcu_fqs_enable, on);
+}
+EXPORT_SYMBOL_GPL(rcutorture_fqs_set);
+
+unsigned long rcutorture_fqs_nr(void)
+{
+ return READ_ONCE(fqs_nr);
+}
+EXPORT_SYMBOL_GPL(rcutorture_fqs_nr);
+
+unsigned long rcutorture_fqs_valid_nr(void)
+{
+ return READ_ONCE(fqs_valid_nr);
+}
+EXPORT_SYMBOL_GPL(rcutorture_fqs_valid_nr);
+
+void rcutorture_fqs_account(void)
+{
+ if (rcu_fqs_enable) {
+ if (READ_ONCE(rcu_state.gp_state) == RCU_GP_WAIT_FQS ||
+ READ_ONCE(rcu_state.gp_state) ==
RCU_GP_DOING_FQS)
+ WRITE_ONCE(fqs_valid_nr, fqs_valid_nr + 1);
+ WRITE_ONCE(fqs_nr, fqs_nr + 1);
+ }
+}
+
/*
* Force quiescent states on reluctant CPUs, and also detect which
* CPUs are in dyntick-idle mode.
@@ -2333,6 +2365,7 @@ void rcu_force_quiescent_state(void)
WRITE_ONCE(rcu_state.gp_flags,
READ_ONCE(rcu_state.gp_flags) | RCU_GP_FLAG_FQS);
raw_spin_unlock_irqrestore_rcu_node(rnp_old, flags);
+ rcutorture_fqs_account();
rcu_gp_kthread_wake();
}
EXPORT_SYMBOL_GPL(rcu_force_quiescent_state);
runqemu kvm nographic slirp qemuparams="-smp 4 -m 1024"
bootparams="rcutorture.fqs_duration=6 rcutorture.fqs_holdoff=1
rcutorture.shutdown_secs=3600" -d
original
[ 3603.574361] nr_fqs: 1635:1723
apply this patch
[ 3603.990193] nr_fqs: 1787:1795
Thanks
Zqiang
>
> Thanx, Paul
>
> > Thanks
> > Zqiang
> >
> > >
> > > > /* Funnel through hierarchy to reduce memory contention. */
> > > > rnp = raw_cpu_read(rcu_data.mynode);
> > > > for (; rnp != NULL; rnp = rnp->parent) {
> > > > --
> > > > 2.17.1
> > > >
