[PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap

Joel Fernandes posted 2 patches 10 months ago
[PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Joel Fernandes 10 months ago
Currently, the ->gpwrap is not tested (at all per my testing) due to the
requirement of a large delta between a CPU's rdp->gp_seq and its node's
rnp->gpseq.

This results in no testing of ->gpwrap being set. This patch by default
adds 5 minutes of testing with ->gpwrap forced by lowering the delta
between rdp->gp_seq and rnp->gp_seq to just 8 GPs. All of this is
configurable, including the active time for the setting and a full
testing cycle.

By default, the first 25 minutes of a test will have the _default_
behavior there is right now (ULONG_MAX / 4) delta. Then for 5 minutes,
we switch to a smaller delta causing 1-2 wraps in 5 minutes. I believe
this is reasonable since we at least add a little bit of testing for
usecases where ->gpwrap is set.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/rcu/rcu.h        |  4 +++
 kernel/rcu/rcutorture.c | 67 ++++++++++++++++++++++++++++++++++++++++-
 kernel/rcu/tree.c       | 34 +++++++++++++++++++--
 kernel/rcu/tree.h       |  1 +
 4 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index eed2951a4962..516b26024a37 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -572,6 +572,8 @@ 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 rcu_set_gpwrap_lag(unsigned long lag);
+int rcu_get_gpwrap_count(int cpu);
 #else
 static inline void rcutorture_get_gp_data(int *flags, unsigned long *gp_seq)
 {
@@ -589,6 +591,8 @@ void do_trace_rcu_torture_read(const char *rcutorturename,
 	do { } while (0)
 #endif
 static inline void rcu_gp_set_torture_wait(int duration) { }
+static inline void rcu_set_gpwrap_lag(unsigned long lag) { }
+static inline int rcu_get_gpwrap_count(int cpu) { return 0; }
 #endif
 unsigned long long rcutorture_gather_gp_seqs(void);
 void rcutorture_format_gp_seqs(unsigned long long seqs, char *cp, size_t len);
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 4fa7772be183..74de92c3a9ab 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -115,6 +115,9 @@ torture_param(int, nreaders, -1, "Number of RCU reader threads");
 torture_param(int, object_debug, 0, "Enable debug-object double call_rcu() testing");
 torture_param(int, onoff_holdoff, 0, "Time after boot before CPU hotplugs (s)");
 torture_param(int, onoff_interval, 0, "Time between CPU hotplugs (jiffies), 0=disable");
+torture_param(int, gpwrap_lag_cycle_mins, 30, "Total cycle duration for ovf lag testing (in minutes)");
+torture_param(int, gpwrap_lag_active_mins, 5, "Duration for which ovf lag is active within each cycle (in minutes)");
+torture_param(int, gpwrap_lag_gps, 8, "Value to set for set_gpwrap_lag during an active testing period.");
 torture_param(int, nocbs_nthreads, 0, "Number of NOCB toggle threads, 0 to disable");
 torture_param(int, nocbs_toggle, 1000, "Time between toggling nocb state (ms)");
 torture_param(int, preempt_duration, 0, "Preemption duration (ms), zero to disable");
@@ -413,6 +416,8 @@ struct rcu_torture_ops {
 	bool (*reader_blocked)(void);
 	unsigned long long (*gather_gp_seqs)(void);
 	void (*format_gp_seqs)(unsigned long long seqs, char *cp, size_t len);
+	void (*set_gpwrap_lag)(unsigned long lag);
+	int (*get_gpwrap_count)(int cpu);
 	long cbflood_max;
 	int irq_capable;
 	int can_boost;
@@ -619,6 +624,8 @@ static struct rcu_torture_ops rcu_ops = {
 				  : NULL,
 	.gather_gp_seqs		= rcutorture_gather_gp_seqs,
 	.format_gp_seqs		= rcutorture_format_gp_seqs,
+	.set_gpwrap_lag		= rcu_set_gpwrap_lag,
+	.get_gpwrap_count	= rcu_get_gpwrap_count,
 	.irq_capable		= 1,
 	.can_boost		= IS_ENABLED(CONFIG_RCU_BOOST),
 	.extendables		= RCUTORTURE_MAX_EXTEND,
@@ -2394,6 +2401,7 @@ rcu_torture_stats_print(void)
 	int i;
 	long pipesummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
 	long batchsummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
+	long n_gpwraps = 0;
 	struct rcu_torture *rtcp;
 	static unsigned long rtcv_snap = ULONG_MAX;
 	static bool splatted;
@@ -2404,6 +2412,7 @@ rcu_torture_stats_print(void)
 			pipesummary[i] += READ_ONCE(per_cpu(rcu_torture_count, cpu)[i]);
 			batchsummary[i] += READ_ONCE(per_cpu(rcu_torture_batch, cpu)[i]);
 		}
+		n_gpwraps += cur_ops->get_gpwrap_count(cpu);
 	}
 	for (i = RCU_TORTURE_PIPE_LEN; i >= 0; i--) {
 		if (pipesummary[i] != 0)
@@ -2435,8 +2444,9 @@ rcu_torture_stats_print(void)
 		data_race(n_barrier_attempts),
 		data_race(n_rcu_torture_barrier_error));
 	pr_cont("read-exits: %ld ", data_race(n_read_exits)); // Statistic.
-	pr_cont("nocb-toggles: %ld:%ld\n",
+	pr_cont("nocb-toggles: %ld:%ld ",
 		atomic_long_read(&n_nocb_offload), atomic_long_read(&n_nocb_deoffload));
+	pr_cont("gpwraps: %ld\n", n_gpwraps);
 
 	pr_alert("%s%s ", torture_type, TORTURE_FLAG);
 	if (atomic_read(&n_rcu_torture_mberror) ||
@@ -3607,6 +3617,54 @@ static int rcu_torture_preempt(void *unused)
 
 static enum cpuhp_state rcutor_hp;
 
+static struct hrtimer gpwrap_lag_timer;
+static bool gpwrap_lag_active;
+
+/* Timer handler for toggling RCU grace-period sequence overflow test lag value */
+static enum hrtimer_restart rcu_gpwrap_lag_timer(struct hrtimer *timer)
+{
+	ktime_t next_delay;
+
+	if (gpwrap_lag_active) {
+		pr_alert("rcu-torture: Disabling ovf lag (value=0)\n");
+		cur_ops->set_gpwrap_lag(0);
+		gpwrap_lag_active = false;
+		next_delay = ktime_set((gpwrap_lag_cycle_mins - gpwrap_lag_active_mins) * 60, 0);
+	} else {
+		pr_alert("rcu-torture: Enabling ovf lag (value=%d)\n", gpwrap_lag_gps);
+		cur_ops->set_gpwrap_lag(gpwrap_lag_gps);
+		gpwrap_lag_active = true;
+		next_delay = ktime_set(gpwrap_lag_active_mins * 60, 0);
+	}
+
+	if (torture_must_stop())
+		return HRTIMER_NORESTART;
+
+	hrtimer_forward_now(timer, next_delay);
+	return HRTIMER_RESTART;
+}
+
+static int rcu_gpwrap_lag_init(void)
+{
+	if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
+		pr_alert("rcu-torture: lag timing parameters must be positive\n");
+		return -EINVAL;
+	}
+
+	hrtimer_setup(&gpwrap_lag_timer, rcu_gpwrap_lag_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	gpwrap_lag_active = false;
+	hrtimer_start(&gpwrap_lag_timer,
+		      ktime_set((gpwrap_lag_cycle_mins - gpwrap_lag_active_mins) * 60, 0), HRTIMER_MODE_REL);
+
+	return 0;
+}
+
+static void rcu_gpwrap_lag_cleanup(void)
+{
+	hrtimer_cancel(&gpwrap_lag_timer);
+	cur_ops->set_gpwrap_lag(0);
+	gpwrap_lag_active = false;
+}
 static void
 rcu_torture_cleanup(void)
 {
@@ -3776,6 +3834,9 @@ rcu_torture_cleanup(void)
 	torture_cleanup_end();
 	if (cur_ops->gp_slow_unregister)
 		cur_ops->gp_slow_unregister(NULL);
+
+	if (cur_ops->set_gpwrap_lag)
+		rcu_gpwrap_lag_cleanup();
 }
 
 static void rcu_torture_leak_cb(struct rcu_head *rhp)
@@ -4275,6 +4336,10 @@ rcu_torture_init(void)
 	torture_init_end();
 	if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister))
 		cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay);
+
+	if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init())
+		goto unwind;
+
 	return 0;
 
 unwind:
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 659f83e71048..6ec30d07759d 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -80,6 +80,15 @@ static void rcu_sr_normal_gp_cleanup_work(struct work_struct *);
 static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, rcu_data) = {
 	.gpwrap = true,
 };
+
+int rcu_get_gpwrap_count(int cpu)
+{
+	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
+
+	return READ_ONCE(rdp->gpwrap_count);
+}
+EXPORT_SYMBOL_GPL(rcu_get_gpwrap_count);
+
 static struct rcu_state rcu_state = {
 	.level = { &rcu_state.node[0] },
 	.gp_state = RCU_GP_IDLE,
@@ -757,6 +766,25 @@ void rcu_request_urgent_qs_task(struct task_struct *t)
 	smp_store_release(per_cpu_ptr(&rcu_data.rcu_urgent_qs, cpu), true);
 }
 
+/**
+ * rcu_set_gpwrap_lag - Set RCU GP sequence overflow lag value.
+ * @lag_gps: Set overflow lag to this many grace period worth of counters
+ * which is used by rcutorture to quickly force a gpwrap situation.
+ * @lag_gps = 0 means we reset it back to the boot-time value.
+ */
+static unsigned long seq_gpwrap_lag = ULONG_MAX / 4;
+
+void rcu_set_gpwrap_lag(unsigned long lag_gps)
+{
+	unsigned long lag_seq_count;
+
+	lag_seq_count = (lag_gps == 0)
+			? ULONG_MAX / 4
+			: lag_gps << RCU_SEQ_CTR_SHIFT;
+	WRITE_ONCE(seq_gpwrap_lag, lag_seq_count);
+}
+EXPORT_SYMBOL_GPL(rcu_set_gpwrap_lag);
+
 /*
  * When trying to report a quiescent state on behalf of some other CPU,
  * it is our responsibility to check for and handle potential overflow
@@ -767,9 +795,11 @@ void rcu_request_urgent_qs_task(struct task_struct *t)
 static void rcu_gpnum_ovf(struct rcu_node *rnp, struct rcu_data *rdp)
 {
 	raw_lockdep_assert_held_rcu_node(rnp);
-	if (ULONG_CMP_LT(rcu_seq_current(&rdp->gp_seq) + ULONG_MAX / 4,
-			 rnp->gp_seq))
+	if (ULONG_CMP_LT(rcu_seq_current(&rdp->gp_seq) + seq_gpwrap_lag,
+			 rnp->gp_seq)) {
 		WRITE_ONCE(rdp->gpwrap, true);
+		WRITE_ONCE(rdp->gpwrap_count, READ_ONCE(rdp->gpwrap_count) + 1);
+	}
 	if (ULONG_CMP_LT(rdp->rcu_iw_gp_seq + ULONG_MAX / 4, rnp->gp_seq))
 		rdp->rcu_iw_gp_seq = rnp->gp_seq + ULONG_MAX / 4;
 }
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index a9a811d9d7a3..63bea388c243 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -183,6 +183,7 @@ struct rcu_data {
 	bool		core_needs_qs;	/* Core waits for quiescent state. */
 	bool		beenonline;	/* CPU online at least once. */
 	bool		gpwrap;		/* Possible ->gp_seq wrap. */
+	unsigned int	gpwrap_count;	/* Count of GP sequence wrap. */
 	bool		cpu_started;	/* RCU watching this onlining CPU. */
 	struct rcu_node *mynode;	/* This CPU's leaf of hierarchy */
 	unsigned long grpmask;		/* Mask to apply to leaf qsmask. */
-- 
2.43.0
Re: [PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 10 months ago
On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
Currently, the ->gpwrap is not tested (at all per my testing) due to
the > requirement of a large delta between a CPU's rdp->gp_seq and its
node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
set. This patch by default > adds 5 minutes of testing with ->gpwrap
forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
just 8 GPs. All of this is > configurable, including the active time for
the setting and a full > testing cycle.  > > By default, the first 25
minutes of a test will have the _default_ > behavior there is right now
(ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delta
causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
at least add a little bit of testing for > usecases where ->gpwrap is set.
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

Much better, thank you!

One potential nit below.  I will run some tests on this version.

						Thanx, Paul

> ---
>  kernel/rcu/rcu.h        |  4 +++
>  kernel/rcu/rcutorture.c | 67 ++++++++++++++++++++++++++++++++++++++++-
>  kernel/rcu/tree.c       | 34 +++++++++++++++++++--
>  kernel/rcu/tree.h       |  1 +
>  4 files changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index eed2951a4962..516b26024a37 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -572,6 +572,8 @@ 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 rcu_set_gpwrap_lag(unsigned long lag);
> +int rcu_get_gpwrap_count(int cpu);
>  #else
>  static inline void rcutorture_get_gp_data(int *flags, unsigned long *gp_seq)
>  {
> @@ -589,6 +591,8 @@ void do_trace_rcu_torture_read(const char *rcutorturename,
>  	do { } while (0)
>  #endif
>  static inline void rcu_gp_set_torture_wait(int duration) { }
> +static inline void rcu_set_gpwrap_lag(unsigned long lag) { }
> +static inline int rcu_get_gpwrap_count(int cpu) { return 0; }
>  #endif
>  unsigned long long rcutorture_gather_gp_seqs(void);
>  void rcutorture_format_gp_seqs(unsigned long long seqs, char *cp, size_t len);
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index 4fa7772be183..74de92c3a9ab 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -115,6 +115,9 @@ torture_param(int, nreaders, -1, "Number of RCU reader threads");
>  torture_param(int, object_debug, 0, "Enable debug-object double call_rcu() testing");
>  torture_param(int, onoff_holdoff, 0, "Time after boot before CPU hotplugs (s)");
>  torture_param(int, onoff_interval, 0, "Time between CPU hotplugs (jiffies), 0=disable");
> +torture_param(int, gpwrap_lag_cycle_mins, 30, "Total cycle duration for ovf lag testing (in minutes)");
> +torture_param(int, gpwrap_lag_active_mins, 5, "Duration for which ovf lag is active within each cycle (in minutes)");
> +torture_param(int, gpwrap_lag_gps, 8, "Value to set for set_gpwrap_lag during an active testing period.");
>  torture_param(int, nocbs_nthreads, 0, "Number of NOCB toggle threads, 0 to disable");
>  torture_param(int, nocbs_toggle, 1000, "Time between toggling nocb state (ms)");
>  torture_param(int, preempt_duration, 0, "Preemption duration (ms), zero to disable");
> @@ -413,6 +416,8 @@ struct rcu_torture_ops {
>  	bool (*reader_blocked)(void);
>  	unsigned long long (*gather_gp_seqs)(void);
>  	void (*format_gp_seqs)(unsigned long long seqs, char *cp, size_t len);
> +	void (*set_gpwrap_lag)(unsigned long lag);
> +	int (*get_gpwrap_count)(int cpu);
>  	long cbflood_max;
>  	int irq_capable;
>  	int can_boost;
> @@ -619,6 +624,8 @@ static struct rcu_torture_ops rcu_ops = {
>  				  : NULL,
>  	.gather_gp_seqs		= rcutorture_gather_gp_seqs,
>  	.format_gp_seqs		= rcutorture_format_gp_seqs,
> +	.set_gpwrap_lag		= rcu_set_gpwrap_lag,
> +	.get_gpwrap_count	= rcu_get_gpwrap_count,
>  	.irq_capable		= 1,
>  	.can_boost		= IS_ENABLED(CONFIG_RCU_BOOST),
>  	.extendables		= RCUTORTURE_MAX_EXTEND,
> @@ -2394,6 +2401,7 @@ rcu_torture_stats_print(void)
>  	int i;
>  	long pipesummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
>  	long batchsummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
> +	long n_gpwraps = 0;
>  	struct rcu_torture *rtcp;
>  	static unsigned long rtcv_snap = ULONG_MAX;
>  	static bool splatted;
> @@ -2404,6 +2412,7 @@ rcu_torture_stats_print(void)
>  			pipesummary[i] += READ_ONCE(per_cpu(rcu_torture_count, cpu)[i]);
>  			batchsummary[i] += READ_ONCE(per_cpu(rcu_torture_batch, cpu)[i]);
>  		}
> +		n_gpwraps += cur_ops->get_gpwrap_count(cpu);
>  	}
>  	for (i = RCU_TORTURE_PIPE_LEN; i >= 0; i--) {
>  		if (pipesummary[i] != 0)
> @@ -2435,8 +2444,9 @@ rcu_torture_stats_print(void)
>  		data_race(n_barrier_attempts),
>  		data_race(n_rcu_torture_barrier_error));
>  	pr_cont("read-exits: %ld ", data_race(n_read_exits)); // Statistic.
> -	pr_cont("nocb-toggles: %ld:%ld\n",
> +	pr_cont("nocb-toggles: %ld:%ld ",
>  		atomic_long_read(&n_nocb_offload), atomic_long_read(&n_nocb_deoffload));
> +	pr_cont("gpwraps: %ld\n", n_gpwraps);
>  
>  	pr_alert("%s%s ", torture_type, TORTURE_FLAG);
>  	if (atomic_read(&n_rcu_torture_mberror) ||
> @@ -3607,6 +3617,54 @@ static int rcu_torture_preempt(void *unused)
>  
>  static enum cpuhp_state rcutor_hp;
>  
> +static struct hrtimer gpwrap_lag_timer;
> +static bool gpwrap_lag_active;
> +
> +/* Timer handler for toggling RCU grace-period sequence overflow test lag value */
> +static enum hrtimer_restart rcu_gpwrap_lag_timer(struct hrtimer *timer)
> +{
> +	ktime_t next_delay;
> +
> +	if (gpwrap_lag_active) {
> +		pr_alert("rcu-torture: Disabling ovf lag (value=0)\n");
> +		cur_ops->set_gpwrap_lag(0);
> +		gpwrap_lag_active = false;
> +		next_delay = ktime_set((gpwrap_lag_cycle_mins - gpwrap_lag_active_mins) * 60, 0);
> +	} else {
> +		pr_alert("rcu-torture: Enabling ovf lag (value=%d)\n", gpwrap_lag_gps);
> +		cur_ops->set_gpwrap_lag(gpwrap_lag_gps);
> +		gpwrap_lag_active = true;
> +		next_delay = ktime_set(gpwrap_lag_active_mins * 60, 0);
> +	}
> +
> +	if (torture_must_stop())
> +		return HRTIMER_NORESTART;
> +
> +	hrtimer_forward_now(timer, next_delay);
> +	return HRTIMER_RESTART;
> +}
> +
> +static int rcu_gpwrap_lag_init(void)
> +{
> +	if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
> +		pr_alert("rcu-torture: lag timing parameters must be positive\n");
> +		return -EINVAL;

When rcutorture is initiated by modprobe, this makes perfect sense.

But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
testing and do other testing but splat so that the bogus scripting can
be fixed, (2) Force default values and splat as before, (3) Splat and
halt the system.

The usual approach has been #1, but what makes sense in this case?

> +	}
> +
> +	hrtimer_setup(&gpwrap_lag_timer, rcu_gpwrap_lag_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	gpwrap_lag_active = false;
> +	hrtimer_start(&gpwrap_lag_timer,
> +		      ktime_set((gpwrap_lag_cycle_mins - gpwrap_lag_active_mins) * 60, 0), HRTIMER_MODE_REL);
> +
> +	return 0;
> +}
> +
> +static void rcu_gpwrap_lag_cleanup(void)
> +{
> +	hrtimer_cancel(&gpwrap_lag_timer);
> +	cur_ops->set_gpwrap_lag(0);
> +	gpwrap_lag_active = false;
> +}
>  static void
>  rcu_torture_cleanup(void)
>  {
> @@ -3776,6 +3834,9 @@ rcu_torture_cleanup(void)
>  	torture_cleanup_end();
>  	if (cur_ops->gp_slow_unregister)
>  		cur_ops->gp_slow_unregister(NULL);
> +
> +	if (cur_ops->set_gpwrap_lag)
> +		rcu_gpwrap_lag_cleanup();
>  }
>  
>  static void rcu_torture_leak_cb(struct rcu_head *rhp)
> @@ -4275,6 +4336,10 @@ rcu_torture_init(void)
>  	torture_init_end();
>  	if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister))
>  		cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay);
> +
> +	if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init())
> +		goto unwind;
> +
>  	return 0;
>  
>  unwind:
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 659f83e71048..6ec30d07759d 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -80,6 +80,15 @@ static void rcu_sr_normal_gp_cleanup_work(struct work_struct *);
>  static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, rcu_data) = {
>  	.gpwrap = true,
>  };
> +
> +int rcu_get_gpwrap_count(int cpu)
> +{
> +	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> +
> +	return READ_ONCE(rdp->gpwrap_count);
> +}
> +EXPORT_SYMBOL_GPL(rcu_get_gpwrap_count);
> +
>  static struct rcu_state rcu_state = {
>  	.level = { &rcu_state.node[0] },
>  	.gp_state = RCU_GP_IDLE,
> @@ -757,6 +766,25 @@ void rcu_request_urgent_qs_task(struct task_struct *t)
>  	smp_store_release(per_cpu_ptr(&rcu_data.rcu_urgent_qs, cpu), true);
>  }
>  
> +/**
> + * rcu_set_gpwrap_lag - Set RCU GP sequence overflow lag value.
> + * @lag_gps: Set overflow lag to this many grace period worth of counters
> + * which is used by rcutorture to quickly force a gpwrap situation.
> + * @lag_gps = 0 means we reset it back to the boot-time value.
> + */
> +static unsigned long seq_gpwrap_lag = ULONG_MAX / 4;
> +
> +void rcu_set_gpwrap_lag(unsigned long lag_gps)
> +{
> +	unsigned long lag_seq_count;
> +
> +	lag_seq_count = (lag_gps == 0)
> +			? ULONG_MAX / 4
> +			: lag_gps << RCU_SEQ_CTR_SHIFT;
> +	WRITE_ONCE(seq_gpwrap_lag, lag_seq_count);
> +}
> +EXPORT_SYMBOL_GPL(rcu_set_gpwrap_lag);
> +
>  /*
>   * When trying to report a quiescent state on behalf of some other CPU,
>   * it is our responsibility to check for and handle potential overflow
> @@ -767,9 +795,11 @@ void rcu_request_urgent_qs_task(struct task_struct *t)
>  static void rcu_gpnum_ovf(struct rcu_node *rnp, struct rcu_data *rdp)
>  {
>  	raw_lockdep_assert_held_rcu_node(rnp);
> -	if (ULONG_CMP_LT(rcu_seq_current(&rdp->gp_seq) + ULONG_MAX / 4,
> -			 rnp->gp_seq))
> +	if (ULONG_CMP_LT(rcu_seq_current(&rdp->gp_seq) + seq_gpwrap_lag,
> +			 rnp->gp_seq)) {
>  		WRITE_ONCE(rdp->gpwrap, true);
> +		WRITE_ONCE(rdp->gpwrap_count, READ_ONCE(rdp->gpwrap_count) + 1);
> +	}
>  	if (ULONG_CMP_LT(rdp->rcu_iw_gp_seq + ULONG_MAX / 4, rnp->gp_seq))
>  		rdp->rcu_iw_gp_seq = rnp->gp_seq + ULONG_MAX / 4;
>  }
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index a9a811d9d7a3..63bea388c243 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -183,6 +183,7 @@ struct rcu_data {
>  	bool		core_needs_qs;	/* Core waits for quiescent state. */
>  	bool		beenonline;	/* CPU online at least once. */
>  	bool		gpwrap;		/* Possible ->gp_seq wrap. */
> +	unsigned int	gpwrap_count;	/* Count of GP sequence wrap. */
>  	bool		cpu_started;	/* RCU watching this onlining CPU. */
>  	struct rcu_node *mynode;	/* This CPU's leaf of hierarchy */
>  	unsigned long grpmask;		/* Mask to apply to leaf qsmask. */
> -- 
> 2.43.0
>
Re: [PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Joel Fernandes 10 months ago
On 4/10/2025 2:29 PM, Paul E. McKenney wrote:
>> +static int rcu_gpwrap_lag_init(void)
>> +{
>> +	if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
>> +		pr_alert("rcu-torture: lag timing parameters must be positive\n");
>> +		return -EINVAL;
> When rcutorture is initiated by modprobe, this makes perfect sense.
> 
> But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
> testing and do other testing but splat so that the bogus scripting can
> be fixed, (2) Force default values and splat as before, (3) Splat and
> halt the system.
> 
> The usual approach has been #1, but what makes sense in this case?

If the user deliberately tries to prevent the test, I am Ok with #3 which I
believe is the current behavior. But otherwise #1 is also Ok with me but I don't
feel strongly about doing that.

If we want to do #3, it will just involve changing the "return -EINVAL" to
"return 0" but also may need to be doing so only if RCU torture is a built-in.

IMO the current behavior is reasonable than adding more complexity for an
unusual case for a built-in?

On the other hand if the issue is with providing the user with a way to disable
gpwrap testing, that should IMO be another parameter than setting the _mins
parameters to be 0. But I think we may not want this testing disabled since it
is already "self-disabled" for the first 25 miutes.

Thoughts?

Thanks!

 - Joel
Re: [PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 9 months, 4 weeks ago
On Mon, Apr 14, 2025 at 11:05:45AM -0400, Joel Fernandes wrote:
> On 4/10/2025 2:29 PM, Paul E. McKenney wrote:
> >> +static int rcu_gpwrap_lag_init(void)
> >> +{
> >> +	if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
> >> +		pr_alert("rcu-torture: lag timing parameters must be positive\n");
> >> +		return -EINVAL;
> > When rcutorture is initiated by modprobe, this makes perfect sense.
> > 
> > But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
> > testing and do other testing but splat so that the bogus scripting can
> > be fixed, (2) Force default values and splat as before, (3) Splat and
> > halt the system.
> > 
> > The usual approach has been #1, but what makes sense in this case?
> 
> If the user deliberately tries to prevent the test, I am Ok with #3 which I
> believe is the current behavior. But otherwise #1 is also Ok with me but I don't
> feel strongly about doing that.
> 
> If we want to do #3, it will just involve changing the "return -EINVAL" to
> "return 0" but also may need to be doing so only if RCU torture is a built-in.
> 
> IMO the current behavior is reasonable than adding more complexity for an
> unusual case for a built-in?

The danger is that someone adjusts a scenario, accidentally disables
*all* ->gpwrap testing during built-in tests (kvm.sh, kvm-remote,sh,
and torture.sh), and nobody notices.  This has tripped me up in the
past, hence the existing splats in rcutorture, but only for runs with
built-in rcutorture.

> On the other hand if the issue is with providing the user with a way to disable
> gpwrap testing, that should IMO be another parameter than setting the _mins
> parameters to be 0. But I think we may not want this testing disabled since it
> is already "self-disabled" for the first 25 miutes.

We do need a way of disabling the testing on long runs for fault-isolation
purposes.

For example, rcutorture.n_up_down=0 disables SRCU up/down testing.
Speaking of which, I am adding a section on that topic to this document:

https://docs.google.com/document/d/1RoYRrTsabdeTXcldzpoMnpmmCjGbJNWtDXN6ZNr_4H8/edit?usp=sharing

							Thanx, Paul
Re: [PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Joel Fernandes 9 months, 4 weeks ago

> On Apr 15, 2025, at 8:19 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> 
> On Mon, Apr 14, 2025 at 11:05:45AM -0400, Joel Fernandes wrote:
>> On 4/10/2025 2:29 PM, Paul E. McKenney wrote:
>>>> +static int rcu_gpwrap_lag_init(void)
>>>> +{
>>>> +    if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
>>>> +        pr_alert("rcu-torture: lag timing parameters must be positive\n");
>>>> +        return -EINVAL;
>>> When rcutorture is initiated by modprobe, this makes perfect sense.
>>> 
>>> But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
>>> testing and do other testing but splat so that the bogus scripting can
>>> be fixed, (2) Force default values and splat as before, (3) Splat and
>>> halt the system.
>>> 
>>> The usual approach has been #1, but what makes sense in this case?
>> 
>> If the user deliberately tries to prevent the test, I am Ok with #3 which I
>> believe is the current behavior. But otherwise #1 is also Ok with me but I don't
>> feel strongly about doing that.
>> 
>> If we want to do #3, it will just involve changing the "return -EINVAL" to
>> "return 0" but also may need to be doing so only if RCU torture is a built-in.
>> 
>> IMO the current behavior is reasonable than adding more complexity for an
>> unusual case for a built-in?
> 
> The danger is that someone adjusts a scenario, accidentally disables
> *all* ->gpwrap testing during built-in tests (kvm.sh, kvm-remote,sh,
> and torture.sh), and nobody notices.  This has tripped me up in the
> past, hence the existing splats in rcutorture, but only for runs with
> built-in rcutorture.

But in the code we are discussing, we will splat with an error if either parameter is set to 0?  Sorry if I missed something.

> 
>> On the other hand if the issue is with providing the user with a way to disable
>> gpwrap testing, that should IMO be another parameter than setting the _mins
>> parameters to be 0. But I think we may not want this testing disabled since it
>> is already "self-disabled" for the first 25 miutes.
> 
> We do need a way of disabling the testing on long runs for fault-isolation
> purposes.

Thanks, I will add an option for this.

> 
> For example, rcutorture.n_up_down=0 disables SRCU up/down testing.
> Speaking of which, I am adding a section on that topic to this document:
> 
> https://docs.google.com/document/d/1RoYRrTsabdeTXcldzpoMnpmmCjGbJNWtDXN6ZNr_4H8/edit?usp=sharing

Nice, thanks,

 - Joel


> 
>                            Thanx, Paul
Re: [PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 9 months, 3 weeks ago
On Wed, Apr 16, 2025 at 11:19:22AM +0000, Joel Fernandes wrote:
> 
> 
> > On Apr 15, 2025, at 8:19 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> > 
> > On Mon, Apr 14, 2025 at 11:05:45AM -0400, Joel Fernandes wrote:
> >> On 4/10/2025 2:29 PM, Paul E. McKenney wrote:
> >>>> +static int rcu_gpwrap_lag_init(void)
> >>>> +{
> >>>> +    if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
> >>>> +        pr_alert("rcu-torture: lag timing parameters must be positive\n");
> >>>> +        return -EINVAL;
> >>> When rcutorture is initiated by modprobe, this makes perfect sense.
> >>> 
> >>> But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
> >>> testing and do other testing but splat so that the bogus scripting can
> >>> be fixed, (2) Force default values and splat as before, (3) Splat and
> >>> halt the system.
> >>> 
> >>> The usual approach has been #1, but what makes sense in this case?
> >> 
> >> If the user deliberately tries to prevent the test, I am Ok with #3 which I
> >> believe is the current behavior. But otherwise #1 is also Ok with me but I don't
> >> feel strongly about doing that.
> >> 
> >> If we want to do #3, it will just involve changing the "return -EINVAL" to
> >> "return 0" but also may need to be doing so only if RCU torture is a built-in.
> >> 
> >> IMO the current behavior is reasonable than adding more complexity for an
> >> unusual case for a built-in?
> > 
> > The danger is that someone adjusts a scenario, accidentally disables
> > *all* ->gpwrap testing during built-in tests (kvm.sh, kvm-remote,sh,
> > and torture.sh), and nobody notices.  This has tripped me up in the
> > past, hence the existing splats in rcutorture, but only for runs with
> > built-in rcutorture.
> 
> But in the code we are discussing, we will splat with an error if either parameter is set to 0?  Sorry if I missed something.

The idea would be to instead splat if the user requested a given type of
testing, but that request conflicted with some other setting so that the
user's request had to be refused.  If the user did not request a given
type of testing (so that the corresponding parameter was zero), no splats.

Also, no splats of this type for modprobe (error return instead), rather,
modprobe gets an error code in this case.

Or am I missing the point of your question?

							Thanx, Paul

> >> On the other hand if the issue is with providing the user with a way to disable
> >> gpwrap testing, that should IMO be another parameter than setting the _mins
> >> parameters to be 0. But I think we may not want this testing disabled since it
> >> is already "self-disabled" for the first 25 miutes.
> > 
> > We do need a way of disabling the testing on long runs for fault-isolation
> > purposes.
> 
> Thanks, I will add an option for this.
> 
> > 
> > For example, rcutorture.n_up_down=0 disables SRCU up/down testing.
> > Speaking of which, I am adding a section on that topic to this document:
> > 
> > https://docs.google.com/document/d/1RoYRrTsabdeTXcldzpoMnpmmCjGbJNWtDXN6ZNr_4H8/edit?usp=sharing
> 
> Nice, thanks,
> 
>  - Joel
> 
> 
> > 
> >                            Thanx, Paul
Re: [v3,1/2] rcutorture: Perform more frequent testing of -&gt;gpwrap
Posted by Joel Fernandes 9 months, 3 weeks ago
Hello, Paul,

On April 20, 2025, 12:21 a.m. UTC Paul E. McKenney wrote:
> On Wed, Apr 16, 2025 at 11:19:22AM +0000, Joel Fernandes wrote:
> > 
> > 
> > > On Apr 15, 2025, at 8:19 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> > > 
> > > On Mon, Apr 14, 2025 at 11:05:45AM -0400, Joel Fernandes wrote:
> > >> On 4/10/2025 2:29 PM, Paul E. McKenney wrote:
> > >>>> +static int rcu_gpwrap_lag_init(void)
> > >>>> +{
> > >>>> +    if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
> > >>>> +        pr_alert("rcu-torture: lag timing parameters must be positive\n");
> > >>>> +        return -EINVAL;
> > >>> When rcutorture is initiated by modprobe, this makes perfect sense.
> > >>> 
> > >>> But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
> > >>> testing and do other testing but splat so that the bogus scripting can
> > >>> be fixed, (2) Force default values and splat as before, (3) Splat and
> > >>> halt the system.
> > >>> 
> > >>> The usual approach has been #1, but what makes sense in this case?
> > >> 
> > >> If the user deliberately tries to prevent the test, I am Ok with #3 which I
> > >> believe is the current behavior. But otherwise #1 is also Ok with me but I don't
> > >> feel strongly about doing that.
> > >> 
> > >> If we want to do #3, it will just involve changing the "return -EINVAL" to
> > >> "return 0" but also may need to be doing so only if RCU torture is a built-in.
> > >> 
> > >> IMO the current behavior is reasonable than adding more complexity for an
> > >> unusual case for a built-in?
> > > 
> > > The danger is that someone adjusts a scenario, accidentally disables
> > > *all* ->gpwrap testing during built-in tests (kvm.sh, kvm-remote,sh,
> > > and torture.sh), and nobody notices.  This has tripped me up in the
> > > past, hence the existing splats in rcutorture, but only for runs with
> > > built-in rcutorture.
> > 
> > But in the code we are discussing, we will splat with an error if either
> > parameter is set to 0?  Sorry if I missed something.
> 
> The idea would be to instead splat if the user requested a given type of
> testing, but that request conflicted with some other setting so that the
> user's request had to be refused.  If the user did not request a given
> type of testing (so that the corresponding parameter was zero), no splats.
> 
> Also, no splats of this type for modprobe (error return instead), rather,
> modprobe gets an error code in this case.
> 
> Or am I missing the point of your question?

No you are not missing anything. I just felt I already made the change you are
talking about because if user misconfigures the timing params, it will print an
error. But if you feel something is missing, I'd appreciate a prototype patch!

> > >> On the other hand if the issue is with providing the user with a way to disable
> > >> gpwrap testing, that should IMO be another parameter than setting the _mins
> > >> parameters to be 0. But I think we may not want this testing disabled since it
> > >> is already "self-disabled" for the first 25 miutes.
> > > 
> > > We do need a way of disabling the testing on long runs for fault-isolation
> > > purposes.
> > 
> > Thanks, I will add an option for this.

I still have to fix this, and will add it to the other fix we needed to make
because of the issue you found (kthread_should_stop() splat).

thanks,

 - Joel
Re: [v3,1/2] rcutorture: Perform more frequent testing of -&gt;gpwrap
Posted by Paul E. McKenney 9 months, 3 weeks ago
On Sun, Apr 20, 2025 at 02:40:20AM -0000, Joel Fernandes wrote:
> Hello, Paul,
> 
> On April 20, 2025, 12:21 a.m. UTC Paul E. McKenney wrote:
> > On Wed, Apr 16, 2025 at 11:19:22AM +0000, Joel Fernandes wrote:
> > > 
> > > 
> > > > On Apr 15, 2025, at 8:19 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > 
> > > > On Mon, Apr 14, 2025 at 11:05:45AM -0400, Joel Fernandes wrote:
> > > >> On 4/10/2025 2:29 PM, Paul E. McKenney wrote:
> > > >>>> +static int rcu_gpwrap_lag_init(void)
> > > >>>> +{
> > > >>>> +    if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
> > > >>>> +        pr_alert("rcu-torture: lag timing parameters must be positive\n");
> > > >>>> +        return -EINVAL;
> > > >>> When rcutorture is initiated by modprobe, this makes perfect sense.
> > > >>> 
> > > >>> But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
> > > >>> testing and do other testing but splat so that the bogus scripting can
> > > >>> be fixed, (2) Force default values and splat as before, (3) Splat and
> > > >>> halt the system.
> > > >>> 
> > > >>> The usual approach has been #1, but what makes sense in this case?
> > > >> 
> > > >> If the user deliberately tries to prevent the test, I am Ok with #3 which I
> > > >> believe is the current behavior. But otherwise #1 is also Ok with me but I don't
> > > >> feel strongly about doing that.
> > > >> 
> > > >> If we want to do #3, it will just involve changing the "return -EINVAL" to
> > > >> "return 0" but also may need to be doing so only if RCU torture is a built-in.
> > > >> 
> > > >> IMO the current behavior is reasonable than adding more complexity for an
> > > >> unusual case for a built-in?
> > > > 
> > > > The danger is that someone adjusts a scenario, accidentally disables
> > > > *all* ->gpwrap testing during built-in tests (kvm.sh, kvm-remote,sh,
> > > > and torture.sh), and nobody notices.  This has tripped me up in the
> > > > past, hence the existing splats in rcutorture, but only for runs with
> > > > built-in rcutorture.
> > > 
> > > But in the code we are discussing, we will splat with an error if either
> > > parameter is set to 0?  Sorry if I missed something.
> > 
> > The idea would be to instead splat if the user requested a given type of
> > testing, but that request conflicted with some other setting so that the
> > user's request had to be refused.  If the user did not request a given
> > type of testing (so that the corresponding parameter was zero), no splats.
> > 
> > Also, no splats of this type for modprobe (error return instead), rather,
> > modprobe gets an error code in this case.
> > 
> > Or am I missing the point of your question?
> 
> No you are not missing anything. I just felt I already made the change you are
> talking about because if user misconfigures the timing params, it will print an
> error. But if you feel something is missing, I'd appreciate a prototype patch!

OK, I see that you are relying on the splat after the "unwind" label
in rcu_torture_init(), which is perfectly legitimate.  Apologies for
my confusion!

> > > >> On the other hand if the issue is with providing the user with a way to disable
> > > >> gpwrap testing, that should IMO be another parameter than setting the _mins
> > > >> parameters to be 0. But I think we may not want this testing disabled since it
> > > >> is already "self-disabled" for the first 25 miutes.
> > > > 
> > > > We do need a way of disabling the testing on long runs for fault-isolation
> > > > purposes.
> > > 
> > > Thanks, I will add an option for this.
> 
> I still have to fix this, and will add it to the other fix we needed to make
> because of the issue you found (kthread_should_stop() splat).

Very good, and thank you!

							Thanx, Paul
Re: [PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 10 months ago
On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
> On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
> Currently, the ->gpwrap is not tested (at all per my testing) due to
> the > requirement of a large delta between a CPU's rdp->gp_seq and its
> node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
> set. This patch by default > adds 5 minutes of testing with ->gpwrap
> forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
> just 8 GPs. All of this is > configurable, including the active time for
> the setting and a full > testing cycle.  > > By default, the first 25
> minutes of a test will have the _default_ > behavior there is right now
> (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delta
> causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
> at least add a little bit of testing for > usecases where ->gpwrap is set.
> > > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> 
> Much better, thank you!
> 
> One potential nit below.  I will run some tests on this version.

And please feel free to apply the following to both:

Tested-by: Paul E. McKenney <paulmck@kernel.org>

> > ---
> >  kernel/rcu/rcu.h        |  4 +++
> >  kernel/rcu/rcutorture.c | 67 ++++++++++++++++++++++++++++++++++++++++-
> >  kernel/rcu/tree.c       | 34 +++++++++++++++++++--
> >  kernel/rcu/tree.h       |  1 +
> >  4 files changed, 103 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> > index eed2951a4962..516b26024a37 100644
> > --- a/kernel/rcu/rcu.h
> > +++ b/kernel/rcu/rcu.h
> > @@ -572,6 +572,8 @@ 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 rcu_set_gpwrap_lag(unsigned long lag);
> > +int rcu_get_gpwrap_count(int cpu);
> >  #else
> >  static inline void rcutorture_get_gp_data(int *flags, unsigned long *gp_seq)
> >  {
> > @@ -589,6 +591,8 @@ void do_trace_rcu_torture_read(const char *rcutorturename,
> >  	do { } while (0)
> >  #endif
> >  static inline void rcu_gp_set_torture_wait(int duration) { }
> > +static inline void rcu_set_gpwrap_lag(unsigned long lag) { }
> > +static inline int rcu_get_gpwrap_count(int cpu) { return 0; }
> >  #endif
> >  unsigned long long rcutorture_gather_gp_seqs(void);
> >  void rcutorture_format_gp_seqs(unsigned long long seqs, char *cp, size_t len);
> > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> > index 4fa7772be183..74de92c3a9ab 100644
> > --- a/kernel/rcu/rcutorture.c
> > +++ b/kernel/rcu/rcutorture.c
> > @@ -115,6 +115,9 @@ torture_param(int, nreaders, -1, "Number of RCU reader threads");
> >  torture_param(int, object_debug, 0, "Enable debug-object double call_rcu() testing");
> >  torture_param(int, onoff_holdoff, 0, "Time after boot before CPU hotplugs (s)");
> >  torture_param(int, onoff_interval, 0, "Time between CPU hotplugs (jiffies), 0=disable");
> > +torture_param(int, gpwrap_lag_cycle_mins, 30, "Total cycle duration for ovf lag testing (in minutes)");
> > +torture_param(int, gpwrap_lag_active_mins, 5, "Duration for which ovf lag is active within each cycle (in minutes)");
> > +torture_param(int, gpwrap_lag_gps, 8, "Value to set for set_gpwrap_lag during an active testing period.");
> >  torture_param(int, nocbs_nthreads, 0, "Number of NOCB toggle threads, 0 to disable");
> >  torture_param(int, nocbs_toggle, 1000, "Time between toggling nocb state (ms)");
> >  torture_param(int, preempt_duration, 0, "Preemption duration (ms), zero to disable");
> > @@ -413,6 +416,8 @@ struct rcu_torture_ops {
> >  	bool (*reader_blocked)(void);
> >  	unsigned long long (*gather_gp_seqs)(void);
> >  	void (*format_gp_seqs)(unsigned long long seqs, char *cp, size_t len);
> > +	void (*set_gpwrap_lag)(unsigned long lag);
> > +	int (*get_gpwrap_count)(int cpu);
> >  	long cbflood_max;
> >  	int irq_capable;
> >  	int can_boost;
> > @@ -619,6 +624,8 @@ static struct rcu_torture_ops rcu_ops = {
> >  				  : NULL,
> >  	.gather_gp_seqs		= rcutorture_gather_gp_seqs,
> >  	.format_gp_seqs		= rcutorture_format_gp_seqs,
> > +	.set_gpwrap_lag		= rcu_set_gpwrap_lag,
> > +	.get_gpwrap_count	= rcu_get_gpwrap_count,
> >  	.irq_capable		= 1,
> >  	.can_boost		= IS_ENABLED(CONFIG_RCU_BOOST),
> >  	.extendables		= RCUTORTURE_MAX_EXTEND,
> > @@ -2394,6 +2401,7 @@ rcu_torture_stats_print(void)
> >  	int i;
> >  	long pipesummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
> >  	long batchsummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
> > +	long n_gpwraps = 0;
> >  	struct rcu_torture *rtcp;
> >  	static unsigned long rtcv_snap = ULONG_MAX;
> >  	static bool splatted;
> > @@ -2404,6 +2412,7 @@ rcu_torture_stats_print(void)
> >  			pipesummary[i] += READ_ONCE(per_cpu(rcu_torture_count, cpu)[i]);
> >  			batchsummary[i] += READ_ONCE(per_cpu(rcu_torture_batch, cpu)[i]);
> >  		}
> > +		n_gpwraps += cur_ops->get_gpwrap_count(cpu);
> >  	}
> >  	for (i = RCU_TORTURE_PIPE_LEN; i >= 0; i--) {
> >  		if (pipesummary[i] != 0)
> > @@ -2435,8 +2444,9 @@ rcu_torture_stats_print(void)
> >  		data_race(n_barrier_attempts),
> >  		data_race(n_rcu_torture_barrier_error));
> >  	pr_cont("read-exits: %ld ", data_race(n_read_exits)); // Statistic.
> > -	pr_cont("nocb-toggles: %ld:%ld\n",
> > +	pr_cont("nocb-toggles: %ld:%ld ",
> >  		atomic_long_read(&n_nocb_offload), atomic_long_read(&n_nocb_deoffload));
> > +	pr_cont("gpwraps: %ld\n", n_gpwraps);
> >  
> >  	pr_alert("%s%s ", torture_type, TORTURE_FLAG);
> >  	if (atomic_read(&n_rcu_torture_mberror) ||
> > @@ -3607,6 +3617,54 @@ static int rcu_torture_preempt(void *unused)
> >  
> >  static enum cpuhp_state rcutor_hp;
> >  
> > +static struct hrtimer gpwrap_lag_timer;
> > +static bool gpwrap_lag_active;
> > +
> > +/* Timer handler for toggling RCU grace-period sequence overflow test lag value */
> > +static enum hrtimer_restart rcu_gpwrap_lag_timer(struct hrtimer *timer)
> > +{
> > +	ktime_t next_delay;
> > +
> > +	if (gpwrap_lag_active) {
> > +		pr_alert("rcu-torture: Disabling ovf lag (value=0)\n");
> > +		cur_ops->set_gpwrap_lag(0);
> > +		gpwrap_lag_active = false;
> > +		next_delay = ktime_set((gpwrap_lag_cycle_mins - gpwrap_lag_active_mins) * 60, 0);
> > +	} else {
> > +		pr_alert("rcu-torture: Enabling ovf lag (value=%d)\n", gpwrap_lag_gps);
> > +		cur_ops->set_gpwrap_lag(gpwrap_lag_gps);
> > +		gpwrap_lag_active = true;
> > +		next_delay = ktime_set(gpwrap_lag_active_mins * 60, 0);
> > +	}
> > +
> > +	if (torture_must_stop())
> > +		return HRTIMER_NORESTART;
> > +
> > +	hrtimer_forward_now(timer, next_delay);
> > +	return HRTIMER_RESTART;
> > +}
> > +
> > +static int rcu_gpwrap_lag_init(void)
> > +{
> > +	if (gpwrap_lag_cycle_mins <= 0 || gpwrap_lag_active_mins <= 0) {
> > +		pr_alert("rcu-torture: lag timing parameters must be positive\n");
> > +		return -EINVAL;
> 
> When rcutorture is initiated by modprobe, this makes perfect sense.
> 
> But if rcutorture is built in, we have other choices:  (1) Disable gpwrap
> testing and do other testing but splat so that the bogus scripting can
> be fixed, (2) Force default values and splat as before, (3) Splat and
> halt the system.
> 
> The usual approach has been #1, but what makes sense in this case?
> 
> > +	}
> > +
> > +	hrtimer_setup(&gpwrap_lag_timer, rcu_gpwrap_lag_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> > +	gpwrap_lag_active = false;
> > +	hrtimer_start(&gpwrap_lag_timer,
> > +		      ktime_set((gpwrap_lag_cycle_mins - gpwrap_lag_active_mins) * 60, 0), HRTIMER_MODE_REL);
> > +
> > +	return 0;
> > +}
> > +
> > +static void rcu_gpwrap_lag_cleanup(void)
> > +{
> > +	hrtimer_cancel(&gpwrap_lag_timer);
> > +	cur_ops->set_gpwrap_lag(0);
> > +	gpwrap_lag_active = false;
> > +}
> >  static void
> >  rcu_torture_cleanup(void)
> >  {
> > @@ -3776,6 +3834,9 @@ rcu_torture_cleanup(void)
> >  	torture_cleanup_end();
> >  	if (cur_ops->gp_slow_unregister)
> >  		cur_ops->gp_slow_unregister(NULL);
> > +
> > +	if (cur_ops->set_gpwrap_lag)
> > +		rcu_gpwrap_lag_cleanup();
> >  }
> >  
> >  static void rcu_torture_leak_cb(struct rcu_head *rhp)
> > @@ -4275,6 +4336,10 @@ rcu_torture_init(void)
> >  	torture_init_end();
> >  	if (cur_ops->gp_slow_register && !WARN_ON_ONCE(!cur_ops->gp_slow_unregister))
> >  		cur_ops->gp_slow_register(&rcu_fwd_cb_nodelay);
> > +
> > +	if (cur_ops->set_gpwrap_lag && rcu_gpwrap_lag_init())
> > +		goto unwind;
> > +
> >  	return 0;
> >  
> >  unwind:
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 659f83e71048..6ec30d07759d 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -80,6 +80,15 @@ static void rcu_sr_normal_gp_cleanup_work(struct work_struct *);
> >  static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, rcu_data) = {
> >  	.gpwrap = true,
> >  };
> > +
> > +int rcu_get_gpwrap_count(int cpu)
> > +{
> > +	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> > +
> > +	return READ_ONCE(rdp->gpwrap_count);
> > +}
> > +EXPORT_SYMBOL_GPL(rcu_get_gpwrap_count);
> > +
> >  static struct rcu_state rcu_state = {
> >  	.level = { &rcu_state.node[0] },
> >  	.gp_state = RCU_GP_IDLE,
> > @@ -757,6 +766,25 @@ void rcu_request_urgent_qs_task(struct task_struct *t)
> >  	smp_store_release(per_cpu_ptr(&rcu_data.rcu_urgent_qs, cpu), true);
> >  }
> >  
> > +/**
> > + * rcu_set_gpwrap_lag - Set RCU GP sequence overflow lag value.
> > + * @lag_gps: Set overflow lag to this many grace period worth of counters
> > + * which is used by rcutorture to quickly force a gpwrap situation.
> > + * @lag_gps = 0 means we reset it back to the boot-time value.
> > + */
> > +static unsigned long seq_gpwrap_lag = ULONG_MAX / 4;
> > +
> > +void rcu_set_gpwrap_lag(unsigned long lag_gps)
> > +{
> > +	unsigned long lag_seq_count;
> > +
> > +	lag_seq_count = (lag_gps == 0)
> > +			? ULONG_MAX / 4
> > +			: lag_gps << RCU_SEQ_CTR_SHIFT;
> > +	WRITE_ONCE(seq_gpwrap_lag, lag_seq_count);
> > +}
> > +EXPORT_SYMBOL_GPL(rcu_set_gpwrap_lag);
> > +
> >  /*
> >   * When trying to report a quiescent state on behalf of some other CPU,
> >   * it is our responsibility to check for and handle potential overflow
> > @@ -767,9 +795,11 @@ void rcu_request_urgent_qs_task(struct task_struct *t)
> >  static void rcu_gpnum_ovf(struct rcu_node *rnp, struct rcu_data *rdp)
> >  {
> >  	raw_lockdep_assert_held_rcu_node(rnp);
> > -	if (ULONG_CMP_LT(rcu_seq_current(&rdp->gp_seq) + ULONG_MAX / 4,
> > -			 rnp->gp_seq))
> > +	if (ULONG_CMP_LT(rcu_seq_current(&rdp->gp_seq) + seq_gpwrap_lag,
> > +			 rnp->gp_seq)) {
> >  		WRITE_ONCE(rdp->gpwrap, true);
> > +		WRITE_ONCE(rdp->gpwrap_count, READ_ONCE(rdp->gpwrap_count) + 1);
> > +	}
> >  	if (ULONG_CMP_LT(rdp->rcu_iw_gp_seq + ULONG_MAX / 4, rnp->gp_seq))
> >  		rdp->rcu_iw_gp_seq = rnp->gp_seq + ULONG_MAX / 4;
> >  }
> > diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> > index a9a811d9d7a3..63bea388c243 100644
> > --- a/kernel/rcu/tree.h
> > +++ b/kernel/rcu/tree.h
> > @@ -183,6 +183,7 @@ struct rcu_data {
> >  	bool		core_needs_qs;	/* Core waits for quiescent state. */
> >  	bool		beenonline;	/* CPU online at least once. */
> >  	bool		gpwrap;		/* Possible ->gp_seq wrap. */
> > +	unsigned int	gpwrap_count;	/* Count of GP sequence wrap. */
> >  	bool		cpu_started;	/* RCU watching this onlining CPU. */
> >  	struct rcu_node *mynode;	/* This CPU's leaf of hierarchy */
> >  	unsigned long grpmask;		/* Mask to apply to leaf qsmask. */
> > -- 
> > 2.43.0
> >
Re: [PATCH v3 1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 10 months ago
On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
> On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
> > Currently, the ->gpwrap is not tested (at all per my testing) due to
> > the > requirement of a large delta between a CPU's rdp->gp_seq and its
> > node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
> > set. This patch by default > adds 5 minutes of testing with ->gpwrap
> > forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
> > just 8 GPs. All of this is > configurable, including the active time for
> > the setting and a full > testing cycle.  > > By default, the first 25
> > minutes of a test will have the _default_ > behavior there is right now
> > (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delta
> > causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
> > at least add a little bit of testing for > usecases where ->gpwrap is set.
> > > > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > 
> > Much better, thank you!
> > 
> > One potential nit below.  I will run some tests on this version.
> 
> And please feel free to apply the following to both:
> 
> Tested-by: Paul E. McKenney <paulmck@kernel.org>

And this happy situation lasted only until I rebased onto v6.15-rc1 and
on top of this commit:

1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.16' and 'rcu/torture-for-6.16' into rcu/for-next")

This got me the splat shown below when running rcutorture scenario SRCU-N.
I reverted this commit and tests pass normally.

Your other commit (ARM64 images) continues working fine.

							Thanx, Paul

------------------------------------------------------------------------

[   15.911885] BUG: kernel NULL pointer dereference, address: 0000000000000000
[   15.912413] #PF: supervisor instruction fetch in kernel mode
[   15.912826] #PF: error_code(0x0010) - not-present page
[   15.913218] PGD 0 P4D 0 
[   15.913420] Oops: Oops: 0010 [#1] SMP PTI
[   15.913715] CPU: 3 UID: 0 PID: 62 Comm: rcu_torture_sta Not tainted 6.15.0-rc1-00047-g6e14cad86633 #19 PREEMPT(undef) 
[   15.914535] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
[   15.915147] RIP: 0010:0x0
[   15.915348] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
[   15.915856] RSP: 0000:ffffa0380021fdc8 EFLAGS: 00010246
[   15.916256] RAX: 0000000000000000 RBX: ffffffffb6b02cc0 RCX: 000000000000000a
[   15.916802] RDX: 0000000000000000 RSI: ffff9f121f418cc0 RDI: 0000000000000000
[   15.917305] RBP: 0000000000000000 R08: ffff9f121f418d20 R09: 0000000000000000
[   15.917789] R10: 0000000000000000 R11: 0000000000000005 R12: ffffffffb6b02d20
[   15.918293] R13: 0000000000000000 R14: ffffa0380021fe50 R15: ffffa0380021fdf8
[   15.918801] FS:  0000000000000000(0000) GS:ffff9f1268a96000(0000) knlGS:0000000000000000
[   15.919313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   15.919628] CR2: ffffffffffffffd6 CR3: 0000000017c32000 CR4: 00000000000006f0
[   15.920004] Call Trace:
[   15.920139]  <TASK>
[   15.920256]  rcu_torture_stats_print+0x16b/0x670
[   15.920514]  ? __switch_to_asm+0x39/0x70
[   15.920719]  ? finish_task_switch.isra.0+0x76/0x250
[   15.920982]  ? __pfx_rcu_torture_stats+0x10/0x10
[   15.921222]  rcu_torture_stats+0x25/0x70
[   15.921435]  kthread+0xf1/0x1e0
[   15.921602]  ? __pfx_kthread+0x10/0x10
[   15.921797]  ? __pfx_kthread+0x10/0x10
[   15.922000]  ret_from_fork+0x2f/0x50
[   15.922193]  ? __pfx_kthread+0x10/0x10
[   15.922395]  ret_from_fork_asm+0x1a/0x30
[   15.922605]  </TASK>
[   15.922723] Modules linked in:
[   15.922890] CR2: 0000000000000000
[   15.923072] ---[ end trace 0000000000000000 ]---
Re: [v3,1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Joel Fernandes 10 months ago
Hello, Paul,

On Fri, 11 Apr 2025 05:33:16 GMT, "Paul E. McKenney" wrote:
> On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
> > > On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
> > > Currently, the ->gpwrap is not tested (at all per my testing) due to
> > > the > requirement of a large delta between a CPU's rdp->gp_seq and its
> > > node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
> > > set. This patch by default > adds 5 minutes of testing with ->gpwrap
> > > forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
> > > just 8 GPs. All of this is > configurable, including the active time for
> > > the setting and a full > testing cycle.  > > By default, the first 25
> > > minutes of a test will have the _default_ > behavior there is right now
> > > (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delt
> a
> > > causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
> > > at least add a little bit of testing for > usecases where ->gpwrap is se
> t.
> > > > > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > > 
> > > Much better, thank you!
> > > 
> > > One potential nit below.  I will run some tests on this version.
> > 
> > And please feel free to apply the following to both:
> > 
> > Tested-by: Paul E. McKenney <paulmck@kernel.org>
> 
> And this happy situation lasted only until I rebased onto v6.15-rc1 and
> on top of this commit:
> 
> 1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.1
> 6' and 'rcu/torture-for-6.16' into rcu/for-next")
> 
> This got me the splat shown below when running rcutorture scenario SRCU-N.
> I reverted this commit and tests pass normally.
> 
> Your other commit (ARM64 images) continues working fine.

Interesting.. it seems to be crashing during statistics printing.

I am wondering if the test itself uncovered a bug or the bug is in the test
itself.

Looking forward to your test with the other patch and we could hold off on this
one till we have more data about what is going on.

thanks,

 - Joel




> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> [   15.911885] BUG: kernel NULL pointer dereference, address: 00000000000000
> 00
> [   15.912413] #PF: supervisor instruction fetch in kernel mode
> [   15.912826] #PF: error_code(0x0010) - not-present page
> [   15.913218] PGD 0 P4D 0 
> [   15.913420] Oops: Oops: 0010 [#1] SMP PTI
> [   15.913715] CPU: 3 UID: 0 PID: 62 Comm: rcu_torture_sta Not tainted 6.15.
> 0-rc1-00047-g6e14cad86633 #19 PREEMPT(undef) 
> [   15.914535] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15
> .0-1 04/01/2014
> [   15.915147] RIP: 0010:0x0
> [   15.915348] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
> [   15.915856] RSP: 0000:ffffa0380021fdc8 EFLAGS: 00010246
> [   15.916256] RAX: 0000000000000000 RBX: ffffffffb6b02cc0 RCX: 000000000000
> 000a
> [   15.916802] RDX: 0000000000000000 RSI: ffff9f121f418cc0 RDI: 000000000000
> 0000
> [   15.917305] RBP: 0000000000000000 R08: ffff9f121f418d20 R09: 000000000000
> 0000
> [   15.917789] R10: 0000000000000000 R11: 0000000000000005 R12: ffffffffb6b0
> 2d20
> [   15.918293] R13: 0000000000000000 R14: ffffa0380021fe50 R15: ffffa0380021
> fdf8
> [   15.918801] FS:  0000000000000000(0000) GS:ffff9f1268a96000(0000) knlGS:0
> 000000000000000
> [   15.919313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   15.919628] CR2: ffffffffffffffd6 CR3: 0000000017c32000 CR4: 000000000000
> 06f0
> [   15.920004] Call Trace:
> [   15.920139]  <TASK>
> [   15.920256]  rcu_torture_stats_print+0x16b/0x670
> [   15.920514]  ? __switch_to_asm+0x39/0x70
> [   15.920719]  ? finish_task_switch.isra.0+0x76/0x250
> [   15.920982]  ? __pfx_rcu_torture_stats+0x10/0x10
> [   15.921222]  rcu_torture_stats+0x25/0x70
> [   15.921435]  kthread+0xf1/0x1e0
> [   15.921602]  ? __pfx_kthread+0x10/0x10
> [   15.921797]  ? __pfx_kthread+0x10/0x10
> [   15.922000]  ret_from_fork+0x2f/0x50
> [   15.922193]  ? __pfx_kthread+0x10/0x10
> [   15.922395]  ret_from_fork_asm+0x1a/0x30
> [   15.922605]  </TASK>
> [   15.922723] Modules linked in:
> [   15.922890] CR2: 0000000000000000
> [   15.923072] ---[ end trace 0000000000000000 ]---
Re: [v3,1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 10 months ago
On Fri, Apr 11, 2025 at 05:36:32AM -0000, Joel Fernandes wrote:
> Hello, Paul,
> 
> On Fri, 11 Apr 2025 05:33:16 GMT, "Paul E. McKenney" wrote:
> > On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
> > > On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
> > > > On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
> > > > Currently, the ->gpwrap is not tested (at all per my testing) due to
> > > > the > requirement of a large delta between a CPU's rdp->gp_seq and its
> > > > node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
> > > > set. This patch by default > adds 5 minutes of testing with ->gpwrap
> > > > forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
> > > > just 8 GPs. All of this is > configurable, including the active time for
> > > > the setting and a full > testing cycle.  > > By default, the first 25
> > > > minutes of a test will have the _default_ > behavior there is right now
> > > > (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delt
> > a
> > > > causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
> > > > at least add a little bit of testing for > usecases where ->gpwrap is se
> > t.
> > > > > > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > > > 
> > > > Much better, thank you!
> > > > 
> > > > One potential nit below.  I will run some tests on this version.
> > > 
> > > And please feel free to apply the following to both:
> > > 
> > > Tested-by: Paul E. McKenney <paulmck@kernel.org>
> > 
> > And this happy situation lasted only until I rebased onto v6.15-rc1 and
> > on top of this commit:
> > 
> > 1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.1
> > 6' and 'rcu/torture-for-6.16' into rcu/for-next")
> > 
> > This got me the splat shown below when running rcutorture scenario SRCU-N.
> > I reverted this commit and tests pass normally.
> > 
> > Your other commit (ARM64 images) continues working fine.
> 
> Interesting.. it seems to be crashing during statistics printing.
> 
> I am wondering if the test itself uncovered a bug or the bug is in the test
> itself.

Both are quite possible, also a bug somewhere else entirely.

> Looking forward to your test with the other patch and we could hold off on this
> one till we have more data about what is going on.

This one got lot of OOMs when tests of RCU priority boosting overlapped
with testing of RCU callback flooding on TREE03, as in 13 of the 14
9-hour runs.  Back on v6.14-rc1, these were quite rare.

Ah, and I am carrying this as an experimental patch:

269b9b5be09d ("EXP sched: Disable DL server if sysctl_sched_rt_runtime is -1")

Just checking to see if this is still something I should be carrying.

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> 
> 
> > 
> > 							Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> > [   15.911885] BUG: kernel NULL pointer dereference, address: 00000000000000
> > 00
> > [   15.912413] #PF: supervisor instruction fetch in kernel mode
> > [   15.912826] #PF: error_code(0x0010) - not-present page
> > [   15.913218] PGD 0 P4D 0 
> > [   15.913420] Oops: Oops: 0010 [#1] SMP PTI
> > [   15.913715] CPU: 3 UID: 0 PID: 62 Comm: rcu_torture_sta Not tainted 6.15.
> > 0-rc1-00047-g6e14cad86633 #19 PREEMPT(undef) 
> > [   15.914535] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15
> > .0-1 04/01/2014
> > [   15.915147] RIP: 0010:0x0
> > [   15.915348] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
> > [   15.915856] RSP: 0000:ffffa0380021fdc8 EFLAGS: 00010246
> > [   15.916256] RAX: 0000000000000000 RBX: ffffffffb6b02cc0 RCX: 000000000000
> > 000a
> > [   15.916802] RDX: 0000000000000000 RSI: ffff9f121f418cc0 RDI: 000000000000
> > 0000
> > [   15.917305] RBP: 0000000000000000 R08: ffff9f121f418d20 R09: 000000000000
> > 0000
> > [   15.917789] R10: 0000000000000000 R11: 0000000000000005 R12: ffffffffb6b0
> > 2d20
> > [   15.918293] R13: 0000000000000000 R14: ffffa0380021fe50 R15: ffffa0380021
> > fdf8
> > [   15.918801] FS:  0000000000000000(0000) GS:ffff9f1268a96000(0000) knlGS:0
> > 000000000000000
> > [   15.919313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   15.919628] CR2: ffffffffffffffd6 CR3: 0000000017c32000 CR4: 000000000000
> > 06f0
> > [   15.920004] Call Trace:
> > [   15.920139]  <TASK>
> > [   15.920256]  rcu_torture_stats_print+0x16b/0x670
> > [   15.920514]  ? __switch_to_asm+0x39/0x70
> > [   15.920719]  ? finish_task_switch.isra.0+0x76/0x250
> > [   15.920982]  ? __pfx_rcu_torture_stats+0x10/0x10
> > [   15.921222]  rcu_torture_stats+0x25/0x70
> > [   15.921435]  kthread+0xf1/0x1e0
> > [   15.921602]  ? __pfx_kthread+0x10/0x10
> > [   15.921797]  ? __pfx_kthread+0x10/0x10
> > [   15.922000]  ret_from_fork+0x2f/0x50
> > [   15.922193]  ? __pfx_kthread+0x10/0x10
> > [   15.922395]  ret_from_fork_asm+0x1a/0x30
> > [   15.922605]  </TASK>
> > [   15.922723] Modules linked in:
> > [   15.922890] CR2: 0000000000000000
> > [   15.923072] ---[ end trace 0000000000000000 ]---
Re: [v3,1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Joel Fernandes 10 months ago

> On Apr 11, 2025, at 3:18 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> 
> On Fri, Apr 11, 2025 at 05:36:32AM -0000, Joel Fernandes wrote:
>> Hello, Paul,
>> 
>>> On Fri, 11 Apr 2025 05:33:16 GMT, "Paul E. McKenney" wrote:
>>> On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
>>>> On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
>>>>> On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
>>>>> Currently, the ->gpwrap is not tested (at all per my testing) due to
>>>>> the > requirement of a large delta between a CPU's rdp->gp_seq and its
>>>>> node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
>>>>> set. This patch by default > adds 5 minutes of testing with ->gpwrap
>>>>> forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
>>>>> just 8 GPs. All of this is > configurable, including the active time for
>>>>> the setting and a full > testing cycle.  > > By default, the first 25
>>>>> minutes of a test will have the _default_ > behavior there is right now
>>>>> (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delt
>>> a
>>>>> causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
>>>>> at least add a little bit of testing for > usecases where ->gpwrap is se
>>> t.
>>>>>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>>>> 
>>>>> Much better, thank you!
>>>>> 
>>>>> One potential nit below.  I will run some tests on this version.
>>>> 
>>>> And please feel free to apply the following to both:
>>>> 
>>>> Tested-by: Paul E. McKenney <paulmck@kernel.org>
>>> 
>>> And this happy situation lasted only until I rebased onto v6.15-rc1 and
>>> on top of this commit:
>>> 
>>> 1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.1
>>> 6' and 'rcu/torture-for-6.16' into rcu/for-next")
>>> 
>>> This got me the splat shown below when running rcutorture scenario SRCU-N.
>>> I reverted this commit and tests pass normally.
>>> 
>>> Your other commit (ARM64 images) continues working fine.
>> 
>> Interesting.. it seems to be crashing during statistics printing.
>> 
>> I am wondering if the test itself uncovered a bug or the bug is in the test
>> itself.
> 
> Both are quite possible, also a bug somewhere else entirely.

I may not get to debugging it for this merge window so I am leaning to defer it.

> 
>> Looking forward to your test with the other patch and we could hold off on this
>> one till we have more data about what is going on.
> 
> This one got lot of OOMs when tests of RCU priority boosting overlapped
> with testing of RCU callback flooding on TREE03, as in 13 of the 14
> 9-hour runs.  Back on v6.14-rc1, these were quite rare.
> 
> Ah, and I am carrying this as an experimental patch:
> 
> 269b9b5be09d ("EXP sched: Disable DL server if sysctl_sched_rt_runtime is -1")
> 
> Just checking to see if this is still something I should be carrying.

I think since it exposing boost issues, we should carry it! However since it is also noisy, maybe for short term we not carry it in any trees since we are getting close to posting the topic branches.

Do you see the same boost issues or frequency of them when carrying it on 6.15-rc1 without any of this merge windows changes?

By the way I have to rewrite that EXP patch at some point based on a review of it but functionally that patch is good.

Thanks,

- Joel 

> 
>                            Thanx, Paul
> 
>> thanks,
>> 
>> - Joel
>> 
>> 
>> 
>> 
>>> 
>>>                            Thanx, Paul
>>> 
>>> ------------------------------------------------------------------------
>>> 
>>> [   15.911885] BUG: kernel NULL pointer dereference, address: 00000000000000
>>> 00
>>> [   15.912413] #PF: supervisor instruction fetch in kernel mode
>>> [   15.912826] #PF: error_code(0x0010) - not-present page
>>> [   15.913218] PGD 0 P4D 0
>>> [   15.913420] Oops: Oops: 0010 [#1] SMP PTI
>>> [   15.913715] CPU: 3 UID: 0 PID: 62 Comm: rcu_torture_sta Not tainted 6.15.
>>> 0-rc1-00047-g6e14cad86633 #19 PREEMPT(undef)
>>> [   15.914535] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15
>>> .0-1 04/01/2014
>>> [   15.915147] RIP: 0010:0x0
>>> [   15.915348] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
>>> [   15.915856] RSP: 0000:ffffa0380021fdc8 EFLAGS: 00010246
>>> [   15.916256] RAX: 0000000000000000 RBX: ffffffffb6b02cc0 RCX: 000000000000
>>> 000a
>>> [   15.916802] RDX: 0000000000000000 RSI: ffff9f121f418cc0 RDI: 000000000000
>>> 0000
>>> [   15.917305] RBP: 0000000000000000 R08: ffff9f121f418d20 R09: 000000000000
>>> 0000
>>> [   15.917789] R10: 0000000000000000 R11: 0000000000000005 R12: ffffffffb6b0
>>> 2d20
>>> [   15.918293] R13: 0000000000000000 R14: ffffa0380021fe50 R15: ffffa0380021
>>> fdf8
>>> [   15.918801] FS:  0000000000000000(0000) GS:ffff9f1268a96000(0000) knlGS:0
>>> 000000000000000
>>> [   15.919313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [   15.919628] CR2: ffffffffffffffd6 CR3: 0000000017c32000 CR4: 000000000000
>>> 06f0
>>> [   15.920004] Call Trace:
>>> [   15.920139]  <TASK>
>>> [   15.920256]  rcu_torture_stats_print+0x16b/0x670
>>> [   15.920514]  ? __switch_to_asm+0x39/0x70
>>> [   15.920719]  ? finish_task_switch.isra.0+0x76/0x250
>>> [   15.920982]  ? __pfx_rcu_torture_stats+0x10/0x10
>>> [   15.921222]  rcu_torture_stats+0x25/0x70
>>> [   15.921435]  kthread+0xf1/0x1e0
>>> [   15.921602]  ? __pfx_kthread+0x10/0x10
>>> [   15.921797]  ? __pfx_kthread+0x10/0x10
>>> [   15.922000]  ret_from_fork+0x2f/0x50
>>> [   15.922193]  ? __pfx_kthread+0x10/0x10
>>> [   15.922395]  ret_from_fork_asm+0x1a/0x30
>>> [   15.922605]  </TASK>
>>> [   15.922723] Modules linked in:
>>> [   15.922890] CR2: 0000000000000000
>>> [   15.923072] ---[ end trace 0000000000000000 ]---
Re: [v3,1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 10 months ago
On Mon, Apr 14, 2025 at 08:07:24AM -0400, Joel Fernandes wrote:
> 
> 
> > On Apr 11, 2025, at 3:18 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> > 
> > On Fri, Apr 11, 2025 at 05:36:32AM -0000, Joel Fernandes wrote:
> >> Hello, Paul,
> >> 
> >>> On Fri, 11 Apr 2025 05:33:16 GMT, "Paul E. McKenney" wrote:
> >>> On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
> >>>> On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
> >>>>> On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
> >>>>> Currently, the ->gpwrap is not tested (at all per my testing) due to
> >>>>> the > requirement of a large delta between a CPU's rdp->gp_seq and its
> >>>>> node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
> >>>>> set. This patch by default > adds 5 minutes of testing with ->gpwrap
> >>>>> forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
> >>>>> just 8 GPs. All of this is > configurable, including the active time for
> >>>>> the setting and a full > testing cycle.  > > By default, the first 25
> >>>>> minutes of a test will have the _default_ > behavior there is right now
> >>>>> (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delt
> >>> a
> >>>>> causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
> >>>>> at least add a little bit of testing for > usecases where ->gpwrap is se
> >>> t.
> >>>>>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> >>>>> 
> >>>>> Much better, thank you!
> >>>>> 
> >>>>> One potential nit below.  I will run some tests on this version.
> >>>> 
> >>>> And please feel free to apply the following to both:
> >>>> 
> >>>> Tested-by: Paul E. McKenney <paulmck@kernel.org>
> >>> 
> >>> And this happy situation lasted only until I rebased onto v6.15-rc1 and
> >>> on top of this commit:
> >>> 
> >>> 1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.1
> >>> 6' and 'rcu/torture-for-6.16' into rcu/for-next")
> >>> 
> >>> This got me the splat shown below when running rcutorture scenario SRCU-N.
> >>> I reverted this commit and tests pass normally.
> >>> 
> >>> Your other commit (ARM64 images) continues working fine.
> >> 
> >> Interesting.. it seems to be crashing during statistics printing.
> >> 
> >> I am wondering if the test itself uncovered a bug or the bug is in the test
> >> itself.
> > 
> > Both are quite possible, also a bug somewhere else entirely.
> 
> I may not get to debugging it for this merge window so I am leaning to defer it.

The usual cause is use of an rcu_torture_ops function pointer without
having first checked that it is non-NULL.  But I suspect that you already
checked for this.

> >> Looking forward to your test with the other patch and we could hold off on this
> >> one till we have more data about what is going on.
> > 
> > This one got lot of OOMs when tests of RCU priority boosting overlapped
> > with testing of RCU callback flooding on TREE03, as in 13 of the 14
> > 9-hour runs.  Back on v6.14-rc1, these were quite rare.
> > 
> > Ah, and I am carrying this as an experimental patch:
> > 
> > 269b9b5be09d ("EXP sched: Disable DL server if sysctl_sched_rt_runtime is -1")
> > 
> > Just checking to see if this is still something I should be carrying.
> 
> I think since it exposing boost issues, we should carry it! However since it is also noisy, maybe for short term we not carry it in any trees since we are getting close to posting the topic branches.

I am carrying it in -rcu, but marked "EXP" so that I don't post it or
send it along in a pull request.

> Do you see the same boost issues or frequency of them when carrying it on 6.15-rc1 without any of this merge windows changes?
> 
> By the way I have to rewrite that EXP patch at some point based on a review of it but functionally that patch is good.

I just now started a short test with it reverted.

Oh, and yours and Boqun's latest passed overnight tests except for a
few Kconfig issues including the PREEMPT_RT pair:

75cf58ef310a ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.16' and 'rcu/torture-for-6.16' into rcu/for-next")

This is a known Kconfig issue in torture.sh, fixed by this -rcu commit:

2e26af16b7b6 ("torture.sh: Force CONFIG_RCU_NOCB_CPU=y for --do-rt configurations")

There are also Kconfig issues with a few of the KCSAN rcutorture scenarios
that I am looking into.  And torture.sh needs to be more aggressive about
reporting these...

							Thanx, Paul

> Thanks,
> 
> - Joel 
> 
> > 
> >                            Thanx, Paul
> > 
> >> thanks,
> >> 
> >> - Joel
> >> 
> >> 
> >> 
> >> 
> >>> 
> >>>                            Thanx, Paul
> >>> 
> >>> ------------------------------------------------------------------------
> >>> 
> >>> [   15.911885] BUG: kernel NULL pointer dereference, address: 00000000000000
> >>> 00
> >>> [   15.912413] #PF: supervisor instruction fetch in kernel mode
> >>> [   15.912826] #PF: error_code(0x0010) - not-present page
> >>> [   15.913218] PGD 0 P4D 0
> >>> [   15.913420] Oops: Oops: 0010 [#1] SMP PTI
> >>> [   15.913715] CPU: 3 UID: 0 PID: 62 Comm: rcu_torture_sta Not tainted 6.15.
> >>> 0-rc1-00047-g6e14cad86633 #19 PREEMPT(undef)
> >>> [   15.914535] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15
> >>> .0-1 04/01/2014
> >>> [   15.915147] RIP: 0010:0x0
> >>> [   15.915348] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
> >>> [   15.915856] RSP: 0000:ffffa0380021fdc8 EFLAGS: 00010246
> >>> [   15.916256] RAX: 0000000000000000 RBX: ffffffffb6b02cc0 RCX: 000000000000
> >>> 000a
> >>> [   15.916802] RDX: 0000000000000000 RSI: ffff9f121f418cc0 RDI: 000000000000
> >>> 0000
> >>> [   15.917305] RBP: 0000000000000000 R08: ffff9f121f418d20 R09: 000000000000
> >>> 0000
> >>> [   15.917789] R10: 0000000000000000 R11: 0000000000000005 R12: ffffffffb6b0
> >>> 2d20
> >>> [   15.918293] R13: 0000000000000000 R14: ffffa0380021fe50 R15: ffffa0380021
> >>> fdf8
> >>> [   15.918801] FS:  0000000000000000(0000) GS:ffff9f1268a96000(0000) knlGS:0
> >>> 000000000000000
> >>> [   15.919313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>> [   15.919628] CR2: ffffffffffffffd6 CR3: 0000000017c32000 CR4: 000000000000
> >>> 06f0
> >>> [   15.920004] Call Trace:
> >>> [   15.920139]  <TASK>
> >>> [   15.920256]  rcu_torture_stats_print+0x16b/0x670
> >>> [   15.920514]  ? __switch_to_asm+0x39/0x70
> >>> [   15.920719]  ? finish_task_switch.isra.0+0x76/0x250
> >>> [   15.920982]  ? __pfx_rcu_torture_stats+0x10/0x10
> >>> [   15.921222]  rcu_torture_stats+0x25/0x70
> >>> [   15.921435]  kthread+0xf1/0x1e0
> >>> [   15.921602]  ? __pfx_kthread+0x10/0x10
> >>> [   15.921797]  ? __pfx_kthread+0x10/0x10
> >>> [   15.922000]  ret_from_fork+0x2f/0x50
> >>> [   15.922193]  ? __pfx_kthread+0x10/0x10
> >>> [   15.922395]  ret_from_fork_asm+0x1a/0x30
> >>> [   15.922605]  </TASK>
> >>> [   15.922723] Modules linked in:
> >>> [   15.922890] CR2: 0000000000000000
> >>> [   15.923072] ---[ end trace 0000000000000000 ]---
Re: [v3,1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Joel Fernandes 10 months ago

On 4/14/2025 10:24 AM, Paul E. McKenney wrote:
> On Mon, Apr 14, 2025 at 08:07:24AM -0400, Joel Fernandes wrote:
>>
>>
>>> On Apr 11, 2025, at 3:18 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
>>>
>>> On Fri, Apr 11, 2025 at 05:36:32AM -0000, Joel Fernandes wrote:
>>>> Hello, Paul,
>>>>
>>>>> On Fri, 11 Apr 2025 05:33:16 GMT, "Paul E. McKenney" wrote:
>>>>> On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
>>>>>> On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
>>>>>>> On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
>>>>>>> Currently, the ->gpwrap is not tested (at all per my testing) due to
>>>>>>> the > requirement of a large delta between a CPU's rdp->gp_seq and its
>>>>>>> node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
>>>>>>> set. This patch by default > adds 5 minutes of testing with ->gpwrap
>>>>>>> forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
>>>>>>> just 8 GPs. All of this is > configurable, including the active time for
>>>>>>> the setting and a full > testing cycle.  > > By default, the first 25
>>>>>>> minutes of a test will have the _default_ > behavior there is right now
>>>>>>> (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delt
>>>>> a
>>>>>>> causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
>>>>>>> at least add a little bit of testing for > usecases where ->gpwrap is se
>>>>> t.
>>>>>>>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>>>>>>
>>>>>>> Much better, thank you!
>>>>>>>
>>>>>>> One potential nit below.  I will run some tests on this version.
>>>>>>
>>>>>> And please feel free to apply the following to both:
>>>>>>
>>>>>> Tested-by: Paul E. McKenney <paulmck@kernel.org>
>>>>>
>>>>> And this happy situation lasted only until I rebased onto v6.15-rc1 and
>>>>> on top of this commit:
>>>>>
>>>>> 1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.1
>>>>> 6' and 'rcu/torture-for-6.16' into rcu/for-next")
>>>>>
>>>>> This got me the splat shown below when running rcutorture scenario SRCU-N.
>>>>> I reverted this commit and tests pass normally.
>>>>>
>>>>> Your other commit (ARM64 images) continues working fine.
>>>>
>>>> Interesting.. it seems to be crashing during statistics printing.
>>>>
>>>> I am wondering if the test itself uncovered a bug or the bug is in the test
>>>> itself.
>>>
>>> Both are quite possible, also a bug somewhere else entirely.
>>
>> I may not get to debugging it for this merge window so I am leaning to defer it.
> 
> The usual cause is use of an rcu_torture_ops function pointer without
> having first checked that it is non-NULL.  But I suspect that you already
> checked for this.

Oops, I am not! You are right I think it broke since the movement into ops and
needs this which I missed for this call site (though I did it for the other). I
could repro SRCU-N without it and with the fix it passes:

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 74de92c3a9ab..df6504a855aa 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -2412,7 +2412,8 @@ rcu_torture_stats_print(void)
                        pipesummary[i] += READ_ONCE(per_cpu(rcu_torture_count,
cpu)[i]);
                        batchsummary[i] += READ_ONCE(per_cpu(rcu_torture_batch,
cpu)[i]);
                }
-               n_gpwraps += cur_ops->get_gpwrap_count(cpu);
+               if (cur_ops->get_gpwrap_count)
+                       n_gpwraps += cur_ops->get_gpwrap_count(cpu);
        }
        for (i = RCU_TORTURE_PIPE_LEN; i >= 0; i--) {
                if (pipesummary[i] != 0)

I will squash the fix into the patch and repost as v4.

> 
>>>> Looking forward to your test with the other patch and we could hold off on this
>>>> one till we have more data about what is going on.
>>>
>>> This one got lot of OOMs when tests of RCU priority boosting overlapped
>>> with testing of RCU callback flooding on TREE03, as in 13 of the 14
>>> 9-hour runs.  Back on v6.14-rc1, these were quite rare.
>>>
>>> Ah, and I am carrying this as an experimental patch:
>>>
>>> 269b9b5be09d ("EXP sched: Disable DL server if sysctl_sched_rt_runtime is -1")
>>>
>>> Just checking to see if this is still something I should be carrying.
>>
>> I think since it exposing boost issues, we should carry it! However since it is also noisy, maybe for short term we not carry it in any trees since we are getting close to posting the topic branches.
> 
> I am carrying it in -rcu, but marked "EXP" so that I don't post it or
> send it along in a pull request.

Sounds good to me.

>> Do you see the same boost issues or frequency of them when carrying it on 6.15-rc1 without any of this merge windows changes?
>>
>> By the way I have to rewrite that EXP patch at some point based on a review of it but functionally that patch is good.
> 
> I just now started a short test with it reverted.
> 
> Oh, and yours and Boqun's latest passed overnight tests except for a
> few Kconfig issues including the PREEMPT_RT pair:
> 
> 75cf58ef310a ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.16' and 'rcu/torture-for-6.16' into rcu/for-next")
> 
> This is a known Kconfig issue in torture.sh, fixed by this -rcu commit:
> 
> 2e26af16b7b6 ("torture.sh: Force CONFIG_RCU_NOCB_CPU=y for --do-rt configurations")

I already merged this change
https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/commit/?h=rcu/for-next&id=a3204f778cf7e37c7344404768398b4f9d43a368

But you saw issues in your testing even with this?

Could you rebase on top of my for-next branch so we are on the same page? Tag
next.2025.04.11a

I believe you said you were going to rebuild your tree, but were waiting on testing?

> 
> There are also Kconfig issues with a few of the KCSAN rcutorture scenarios
> that I am looking into.  And torture.sh needs to be more aggressive about
> reporting these...
Ok sounds good, happy to add these to my torture-for-6.16 topic branch once you
post them.

thanks,

 - Joel


Re: [v3,1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Paul E. McKenney 10 months ago
On Mon, Apr 14, 2025 at 10:56:24AM -0400, Joel Fernandes wrote:
> On 4/14/2025 10:24 AM, Paul E. McKenney wrote:
> > On Mon, Apr 14, 2025 at 08:07:24AM -0400, Joel Fernandes wrote:
> >>> On Apr 11, 2025, at 3:18 PM, Paul E. McKenney <paulmck@kernel.org> wrote:
> >>> On Fri, Apr 11, 2025 at 05:36:32AM -0000, Joel Fernandes wrote:
> >>>> Hello, Paul,
> >>>>
> >>>>> On Fri, 11 Apr 2025 05:33:16 GMT, "Paul E. McKenney" wrote:
> >>>>> On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
> >>>>>> On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
> >>>>>>> On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
> >>>>>>> Currently, the ->gpwrap is not tested (at all per my testing) due to
> >>>>>>> the > requirement of a large delta between a CPU's rdp->gp_seq and its
> >>>>>>> node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
> >>>>>>> set. This patch by default > adds 5 minutes of testing with ->gpwrap
> >>>>>>> forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
> >>>>>>> just 8 GPs. All of this is > configurable, including the active time for
> >>>>>>> the setting and a full > testing cycle.  > > By default, the first 25
> >>>>>>> minutes of a test will have the _default_ > behavior there is right now
> >>>>>>> (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delt
> >>>>> a
> >>>>>>> causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
> >>>>>>> at least add a little bit of testing for > usecases where ->gpwrap is se
> >>>>> t.
> >>>>>>>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> >>>>>>>
> >>>>>>> Much better, thank you!
> >>>>>>>
> >>>>>>> One potential nit below.  I will run some tests on this version.
> >>>>>>
> >>>>>> And please feel free to apply the following to both:
> >>>>>>
> >>>>>> Tested-by: Paul E. McKenney <paulmck@kernel.org>
> >>>>>
> >>>>> And this happy situation lasted only until I rebased onto v6.15-rc1 and
> >>>>> on top of this commit:
> >>>>>
> >>>>> 1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.1
> >>>>> 6' and 'rcu/torture-for-6.16' into rcu/for-next")
> >>>>>
> >>>>> This got me the splat shown below when running rcutorture scenario SRCU-N.
> >>>>> I reverted this commit and tests pass normally.
> >>>>>
> >>>>> Your other commit (ARM64 images) continues working fine.
> >>>>
> >>>> Interesting.. it seems to be crashing during statistics printing.
> >>>>
> >>>> I am wondering if the test itself uncovered a bug or the bug is in the test
> >>>> itself.
> >>>
> >>> Both are quite possible, also a bug somewhere else entirely.
> >>
> >> I may not get to debugging it for this merge window so I am leaning to defer it.
> > 
> > The usual cause is use of an rcu_torture_ops function pointer without
> > having first checked that it is non-NULL.  But I suspect that you already
> > checked for this.
> 
> Oops, I am not! You are right I think it broke since the movement into ops and
> needs this which I missed for this call site (though I did it for the other). I
> could repro SRCU-N without it and with the fix it passes:
> 
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index 74de92c3a9ab..df6504a855aa 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -2412,7 +2412,8 @@ rcu_torture_stats_print(void)
>                         pipesummary[i] += READ_ONCE(per_cpu(rcu_torture_count,
> cpu)[i]);
>                         batchsummary[i] += READ_ONCE(per_cpu(rcu_torture_batch,
> cpu)[i]);
>                 }
> -               n_gpwraps += cur_ops->get_gpwrap_count(cpu);
> +               if (cur_ops->get_gpwrap_count)
> +                       n_gpwraps += cur_ops->get_gpwrap_count(cpu);
>         }
>         for (i = RCU_TORTURE_PIPE_LEN; i >= 0; i--) {
>                 if (pipesummary[i] != 0)
> 
> I will squash the fix into the patch and repost as v4.

Been there, done that!  And looks like a proper fix to me.

							Thanx, Paul

> >>>> Looking forward to your test with the other patch and we could hold off on this
> >>>> one till we have more data about what is going on.
> >>>
> >>> This one got lot of OOMs when tests of RCU priority boosting overlapped
> >>> with testing of RCU callback flooding on TREE03, as in 13 of the 14
> >>> 9-hour runs.  Back on v6.14-rc1, these were quite rare.
> >>>
> >>> Ah, and I am carrying this as an experimental patch:
> >>>
> >>> 269b9b5be09d ("EXP sched: Disable DL server if sysctl_sched_rt_runtime is -1")
> >>>
> >>> Just checking to see if this is still something I should be carrying.
> >>
> >> I think since it exposing boost issues, we should carry it! However since it is also noisy, maybe for short term we not carry it in any trees since we are getting close to posting the topic branches.
> > 
> > I am carrying it in -rcu, but marked "EXP" so that I don't post it or
> > send it along in a pull request.
> 
> Sounds good to me.
> 
> >> Do you see the same boost issues or frequency of them when carrying it on 6.15-rc1 without any of this merge windows changes?
> >>
> >> By the way I have to rewrite that EXP patch at some point based on a review of it but functionally that patch is good.
> > 
> > I just now started a short test with it reverted.
> > 
> > Oh, and yours and Boqun's latest passed overnight tests except for a
> > few Kconfig issues including the PREEMPT_RT pair:
> > 
> > 75cf58ef310a ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.16' and 'rcu/torture-for-6.16' into rcu/for-next")
> > 
> > This is a known Kconfig issue in torture.sh, fixed by this -rcu commit:
> > 
> > 2e26af16b7b6 ("torture.sh: Force CONFIG_RCU_NOCB_CPU=y for --do-rt configurations")
> 
> I already merged this change
> https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/commit/?h=rcu/for-next&id=a3204f778cf7e37c7344404768398b4f9d43a368
> 
> But you saw issues in your testing even with this?
> 
> Could you rebase on top of my for-next branch so we are on the same page? Tag
> next.2025.04.11a
> 
> I believe you said you were going to rebuild your tree, but were waiting on testing?
> 
> > 
> > There are also Kconfig issues with a few of the KCSAN rcutorture scenarios
> > that I am looking into.  And torture.sh needs to be more aggressive about
> > reporting these...
> Ok sounds good, happy to add these to my torture-for-6.16 topic branch once you
> post them.
> 
> thanks,
> 
>  - Joel
> 
> 
Re: [v3,1/2] rcutorture: Perform more frequent testing of ->gpwrap
Posted by Joel Fernandes 10 months ago
Hello, Paul,

On Fri, 11 Apr 2025 05:33:16 GMT, "Paul E. McKenney" wrote:
> On Thu, Apr 10, 2025 at 11:54:13AM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 10, 2025 at 11:29:03AM -0700, Paul E. McKenney wrote:
> > > On Thu, Apr 10, 2025 at 11:03:27AM -0400, Joel Fernandes wrote: >
> > > Currently, the ->gpwrap is not tested (at all per my testing) due to
> > > the > requirement of a large delta between a CPU's rdp->gp_seq and its
> > > node's > rnp->gpseq.  > > This results in no testing of ->gpwrap being
> > > set. This patch by default > adds 5 minutes of testing with ->gpwrap
> > > forced by lowering the delta > between rdp->gp_seq and rnp->gp_seq to
> > > just 8 GPs. All of this is > configurable, including the active time for
> > > the setting and a full > testing cycle.  > > By default, the first 25
> > > minutes of a test will have the _default_ > behavior there is right now
> > > (ULONG_MAX / 4) delta. Then for 5 minutes, > we switch to a smaller delt
> a
> > > causing 1-2 wraps in 5 minutes. I believe > this is reasonable since we
> > > at least add a little bit of testing for > usecases where ->gpwrap is se
> t.
> > > > > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > > 
> > > Much better, thank you!
> > > 
> > > One potential nit below.  I will run some tests on this version.
> > 
> > And please feel free to apply the following to both:
> > 
> > Tested-by: Paul E. McKenney <paulmck@kernel.org>
> 
> And this happy situation lasted only until I rebased onto v6.15-rc1 and
> on top of this commit:
> 
> 1342aec2e442 ("Merge branches 'rcu/misc-for-6.16', 'rcu/seq-counters-for-6.1
> 6' and 'rcu/torture-for-6.16' into rcu/for-next")
> 
> This got me the splat shown below when running rcutorture scenario SRCU-N.
> I reverted this commit and tests pass normally.
> 
> Your other commit (ARM64 images) continues working fine.

Interesting.. it seems to be crashing during statistics printing.

I am wondering if the test itself uncovered a bug or the bug is in the test
itself.

Looking forward to your test with the other patch and we could hold off on this
one till we have more data about what is going on.

thanks,

 - Joel




> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> [   15.911885] BUG: kernel NULL pointer dereference, address: 00000000000000
> 00
> [   15.912413] #PF: supervisor instruction fetch in kernel mode
> [   15.912826] #PF: error_code(0x0010) - not-present page
> [   15.913218] PGD 0 P4D 0 
> [   15.913420] Oops: Oops: 0010 [#1] SMP PTI
> [   15.913715] CPU: 3 UID: 0 PID: 62 Comm: rcu_torture_sta Not tainted 6.15.
> 0-rc1-00047-g6e14cad86633 #19 PREEMPT(undef) 
> [   15.914535] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15
> .0-1 04/01/2014
> [   15.915147] RIP: 0010:0x0
> [   15.915348] Code: Unable to access opcode bytes at 0xffffffffffffffd6.
> [   15.915856] RSP: 0000:ffffa0380021fdc8 EFLAGS: 00010246
> [   15.916256] RAX: 0000000000000000 RBX: ffffffffb6b02cc0 RCX: 000000000000
> 000a
> [   15.916802] RDX: 0000000000000000 RSI: ffff9f121f418cc0 RDI: 000000000000
> 0000
> [   15.917305] RBP: 0000000000000000 R08: ffff9f121f418d20 R09: 000000000000
> 0000
> [   15.917789] R10: 0000000000000000 R11: 0000000000000005 R12: ffffffffb6b0
> 2d20
> [   15.918293] R13: 0000000000000000 R14: ffffa0380021fe50 R15: ffffa0380021
> fdf8
> [   15.918801] FS:  0000000000000000(0000) GS:ffff9f1268a96000(0000) knlGS:0
> 000000000000000
> [   15.919313] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   15.919628] CR2: ffffffffffffffd6 CR3: 0000000017c32000 CR4: 000000000000
> 06f0
> [   15.920004] Call Trace:
> [   15.920139]  <TASK>
> [   15.920256]  rcu_torture_stats_print+0x16b/0x670
> [   15.920514]  ? __switch_to_asm+0x39/0x70
> [   15.920719]  ? finish_task_switch.isra.0+0x76/0x250
> [   15.920982]  ? __pfx_rcu_torture_stats+0x10/0x10
> [   15.921222]  rcu_torture_stats+0x25/0x70
> [   15.921435]  kthread+0xf1/0x1e0
> [   15.921602]  ? __pfx_kthread+0x10/0x10
> [   15.921797]  ? __pfx_kthread+0x10/0x10
> [   15.922000]  ret_from_fork+0x2f/0x50
> [   15.922193]  ? __pfx_kthread+0x10/0x10
> [   15.922395]  ret_from_fork_asm+0x1a/0x30
> [   15.922605]  </TASK>
> [   15.922723] Modules linked in:
> [   15.922890] CR2: 0000000000000000
> [   15.923072] ---[ end trace 0000000000000000 ]---