Received-SPF: pass (google.com: domain of linux-kernel+bounces-137573-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 9 Apr 2024 13:05:17 -0700
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Zqiang <qiang.zhang1211@gmail.com>
Cc: frederic@kernel.org, neeraj.upadhyay@kernel.org, joel@joelfernandes.org,
	rcu@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] rcutorture: Make rcutorture support srcu double call
 test
Message-ID: <a511e1a5-9218-4b21-9bb4-72b285397492@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <20240407112714.24460-1-qiang.zhang1211@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240407112714.24460-1-qiang.zhang1211@gmail.com>

On Sun, Apr 07, 2024 at 07:27:14PM +0800, Zqiang wrote:
> This commit also allows rcutorture to support srcu double call test
> with CONFIG_DEBUG_OBJECTS_RCU_HEAD option enabled. the non-raw sdp
> structure's->spinlock will be acquired in call_srcu(), in Preempt-RT
> kernels, this spinlock is sleepable, therefore remove disable-irq and
> disable-preempt protection.
> 
> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com>
> ---
>  kernel/rcu/rcutorture.c | 46 +++++++++++++++++++++++------------------
>  1 file changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index 807fbf6123a7..44cc455e1b61 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -390,6 +390,7 @@ struct rcu_torture_ops {
>  	int extendables;
>  	int slow_gps;
>  	int no_pi_lock;
> +	int debug_objects;
>  	const char *name;
>  };
>  
> @@ -577,6 +578,7 @@ static struct rcu_torture_ops rcu_ops = {
>  	.irq_capable		= 1,
>  	.can_boost		= IS_ENABLED(CONFIG_RCU_BOOST),
>  	.extendables		= RCUTORTURE_MAX_EXTEND,
> +	.debug_objects		= 1,
>  	.name			= "rcu"
>  };
>  
> @@ -747,6 +749,7 @@ static struct rcu_torture_ops srcu_ops = {
>  	.cbflood_max	= 50000,
>  	.irq_capable	= 1,
>  	.no_pi_lock	= IS_ENABLED(CONFIG_TINY_SRCU),
> +	.debug_objects	= 1,
>  	.name		= "srcu"
>  };
>  
> @@ -786,6 +789,7 @@ static struct rcu_torture_ops srcud_ops = {
>  	.cbflood_max	= 50000,
>  	.irq_capable	= 1,
>  	.no_pi_lock	= IS_ENABLED(CONFIG_TINY_SRCU),
> +	.debug_objects	= 1,
>  	.name		= "srcud"
>  };
>  
> @@ -3455,7 +3459,6 @@ rcu_torture_cleanup(void)
>  		cur_ops->gp_slow_unregister(NULL);
>  }
>  
> -#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
>  static void rcu_torture_leak_cb(struct rcu_head *rhp)
>  {
>  }
> @@ -3473,7 +3476,6 @@ static void rcu_torture_err_cb(struct rcu_head *rhp)
>  	 */
>  	pr_alert("%s: duplicated callback was invoked.\n", KBUILD_MODNAME);
>  }
> -#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>  
>  /*
>   * Verify that double-free causes debug-objects to complain, but only
> @@ -3482,39 +3484,43 @@ static void rcu_torture_err_cb(struct rcu_head *rhp)
>   */
>  static void rcu_test_debug_objects(void)
>  {
> -#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
>  	struct rcu_head rh1;
>  	struct rcu_head rh2;
> +	int idx;
> +
> +	if (!IS_ENABLED(CONFIG_DEBUG_OBJECTS_RCU_HEAD)) {
> +		pr_alert("%s: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_%s()\n",
> +					KBUILD_MODNAME, cur_ops->name);
> +		return;
> +	}
> +
> +	if (WARN_ON_ONCE(cur_ops->debug_objects &&
> +			(!cur_ops->call || !cur_ops->cb_barrier)))
> +		return;
> +
>  	struct rcu_head *rhp = kmalloc(sizeof(*rhp), GFP_KERNEL);
>  
>  	init_rcu_head_on_stack(&rh1);
>  	init_rcu_head_on_stack(&rh2);
> -	pr_alert("%s: WARN: Duplicate call_rcu() test starting.\n", KBUILD_MODNAME);
> +	pr_alert("%s: WARN: Duplicate call_%s() test starting.\n", KBUILD_MODNAME, cur_ops->name);
>  
>  	/* Try to queue the rh2 pair of callbacks for the same grace period. */
> -	preempt_disable(); /* Prevent preemption from interrupting test. */
> -	rcu_read_lock(); /* Make it impossible to finish a grace period. */
> -	call_rcu_hurry(&rh1, rcu_torture_leak_cb); /* Start grace period. */
> -	local_irq_disable(); /* Make it harder to start a new grace period. */
> -	call_rcu_hurry(&rh2, rcu_torture_leak_cb);
> -	call_rcu_hurry(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
> +	idx = cur_ops->readlock(); /* Make it impossible to finish a grace period. */
> +	cur_ops->call(&rh1, rcu_torture_leak_cb); /* Start grace period. */
> +	cur_ops->call(&rh2, rcu_torture_leak_cb);
> +	cur_ops->call(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
>  	if (rhp) {
> -		call_rcu_hurry(rhp, rcu_torture_leak_cb);
> -		call_rcu_hurry(rhp, rcu_torture_err_cb); /* Another duplicate callback. */
> +		cur_ops->call(rhp, rcu_torture_leak_cb);
> +		cur_ops->call(rhp, rcu_torture_err_cb); /* Another duplicate callback. */
>  	}
> -	local_irq_enable();
> -	rcu_read_unlock();
> -	preempt_enable();
> +	cur_ops->readunlock(idx);
>  
>  	/* Wait for them all to get done so we can safely return. */
> -	rcu_barrier();
> -	pr_alert("%s: WARN: Duplicate call_rcu() test complete.\n", KBUILD_MODNAME);
> +	cur_ops->cb_barrier();
> +	pr_alert("%s: WARN: Duplicate call_%s() test complete.\n", KBUILD_MODNAME, cur_ops->name);

This will get us "call_srcud() test", but if there are complaints yet
another string can be added to the nrcu_torture_ops structure.

So I have queued this for testing and further review.

							Thanx, Paul

>  	destroy_rcu_head_on_stack(&rh1);
>  	destroy_rcu_head_on_stack(&rh2);
>  	kfree(rhp);
> -#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
> -	pr_alert("%s: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n", KBUILD_MODNAME);
> -#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
>  }
>  
>  static void rcutorture_sync(void)
> -- 
> 2.17.1
>