Hi Paul,
On Fri, Nov 6, 2020 at 12:40 AM <paulmck@xxxxxxxxxx> wrote:
>
> From: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
>
> At the end of the test and after rcu_torture_writer() stalls, rcutorture
> invokes show_rcu_gp_kthreads() in order to dump out information on the
> RCU grace-period kthread. This makes a lot of sense when testing vanilla
> RCU, but not so much for the other flavors. This commit therefore allows
> per-flavor kthread-dump functions to be specified.
>
> [ paulmck: Apply feedback from kernel test robot <lkp@xxxxxxxxx>. ]
> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
Thanks for your patch, which is now commit 27c0f1448389baf7
("rcutorture: Make grace-period kthread report match RCU flavor being
tested").
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -533,4 +533,20 @@ static inline bool rcu_is_nocb_cpu(int cpu) { return false; }
> static inline void rcu_bind_current_to_nocb(void) { }
> #endif
>
> +#if !defined(CONFIG_TINY_RCU) && defined(CONFIG_TASKS_RCU)
> +void show_rcu_tasks_classic_gp_kthread(void);
> +#else
> +static inline void show_rcu_tasks_classic_gp_kthread(void) {}
> +#endif
> +#if !defined(CONFIG_TINY_RCU) && defined(CONFIG_TASKS_RUDE_RCU)
> +void show_rcu_tasks_rude_gp_kthread(void);
> +#else
> +static inline void show_rcu_tasks_rude_gp_kthread(void) {}
> +#endif
The #ifdef expression does not match the one for the implementation
below.
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -762,6 +765,7 @@ static struct rcu_torture_ops tasks_rude_ops = {
> .exp_sync = synchronize_rcu_tasks_rude,
> .call = call_rcu_tasks_rude,
> .cb_barrier = rcu_barrier_tasks_rude,
> + .gp_kthread_dbg = show_rcu_tasks_rude_gp_kthread,
Perhaps you just want to have a NULL pointer for the dummy case, instead
of instantiating a dummy static inline function and taking its address?
> .fqs = NULL,
> .stats = NULL,
> .irq_capable = 1,
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -696,16 +696,14 @@ static int __init rcu_spawn_tasks_rude_kthread(void)
> }
> core_initcall(rcu_spawn_tasks_rude_kthread);
>
> -#ifndef CONFIG_TINY_RCU
> -static void show_rcu_tasks_rude_gp_kthread(void)
> +#if !defined(CONFIG_TINY_RCU)
Different #ifdef expression.
> +void show_rcu_tasks_rude_gp_kthread(void)
Do you really want to define a non-static function...
> {
> show_rcu_tasks_generic_gp_kthread(&rcu_tasks_rude, "");
> }
> -#endif /* #ifndef CONFIG_TINY_RCU */
> -
> -#else /* #ifdef CONFIG_TASKS_RUDE_RCU */
> -static void show_rcu_tasks_rude_gp_kthread(void) {}
> -#endif /* #else #ifdef CONFIG_TASKS_RUDE_RCU */
> +EXPORT_SYMBOL_GPL(show_rcu_tasks_rude_gp_kthread);
... and export its symbol, from a header file?
I know the file is included only once.
> +#endif // !defined(CONFIG_TINY_RCU)
> +#endif /* #ifdef CONFIG_TASKS_RUDE_RCU */
>
> ////////////////////////////////////////////////////////////////////////
> //
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
