Re: [PATCH v2] rcutorture: Create nocb tasks only for CONFIG_RCU_NOCB_CPU=y kernels

[Joel Fernandes]

> On Feb 2, 2023, at 1:57 AM, Zqiang <qiang1.zhang@xxxxxxxxx> wrote:
> 
> When setting nocbs_nthreads to start rcutorture test with a non-zero value,
> the nocb tasks will be created and invoke rcu_nocb_cpu_offload/deoffload()
> to toggle CPU's callback-offload state, but for CONFIG_RCU_NOCB_CPU=n
> kernel, the rcu_nocb_cpu_offload/deoffload() is a no-op and this is also
> meaningless for torture_type is non-rcu.
> 
> This commit therefore add member can_nocbs_toggle to rcu_torture_ops
> structure to avoid unnecessary nocb tasks creation.
> 
> Signed-off-by: Zqiang <qiang1.zhang@xxxxxxxxx>
> ---
> kernel/rcu/rcutorture.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)

Sorry if I am missing something but what is the point of adding more lines of code and complexity to handle this? Does it improve the test coverage or reduce overhead?

This is test code. I see no problem with cost of an extra unused task with positive trade off of keeping the code simple…

thanks,

 - Joel 


> 
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index 297da28ce92d..ced0a8e1d765 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -383,6 +383,7 @@ struct rcu_torture_ops {
>    long cbflood_max;
>    int irq_capable;
>    int can_boost;
> +    int can_nocbs_toggle;
>    int extendables;
>    int slow_gps;
>    int no_pi_lock;
> @@ -569,6 +570,7 @@ static struct rcu_torture_ops rcu_ops = {
>    .stall_dur        = rcu_jiffies_till_stall_check,
>    .irq_capable        = 1,
>    .can_boost        = IS_ENABLED(CONFIG_RCU_BOOST),
> +    .can_nocbs_toggle    = IS_ENABLED(CONFIG_RCU_NOCB_CPU),
>    .extendables        = RCUTORTURE_MAX_EXTEND,
>    .name            = "rcu"
> };
> @@ -2356,7 +2358,7 @@ rcu_torture_print_module_parms(struct rcu_torture_ops *cur_ops, const char *tag)
>         "n_barrier_cbs=%d "
>         "onoff_interval=%d onoff_holdoff=%d "
>         "read_exit_delay=%d read_exit_burst=%d "
> -         "nocbs_nthreads=%d nocbs_toggle=%d "
> +         "nocbs_nthreads=%d/%d nocbs_toggle=%d "
>         "test_nmis=%d\n",
>         torture_type, tag, nrealreaders, nfakewriters,
>         stat_interval, verbose, test_no_idle_hz, shuffle_interval,
> @@ -2368,7 +2370,7 @@ rcu_torture_print_module_parms(struct rcu_torture_ops *cur_ops, const char *tag)
>         n_barrier_cbs,
>         onoff_interval, onoff_holdoff,
>         read_exit_delay, read_exit_burst,
> -         nocbs_nthreads, nocbs_toggle,
> +         nocbs_nthreads, cur_ops->can_nocbs_toggle, nocbs_toggle,
>         test_nmis);
> }
> 
> @@ -3708,6 +3710,10 @@ rcu_torture_init(void)
>        pr_alert("rcu-torture: ->fqs NULL and non-zero fqs_duration, fqs disabled.\n");
>        fqs_duration = 0;
>    }
> +    if (!cur_ops->can_nocbs_toggle && nocbs_nthreads != 0) {
> +        pr_alert("rcu-torture: ->can_nocbs_toggle false and non-zero nocbs_nthreads, nocbs_toggle disabled.\n");
> +        nocbs_nthreads = 0;
> +    }
>    if (cur_ops->init)
>        cur_ops->init();
> 
> -- 
> 2.25.1
>