Re: [PATCH tip/core/rcu 3/4] rcutorture: Make grace-period kthread report match RCU flavor being tested

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: "Paul E . McKenney" <paulmck@kernel.org>
Cc: rcu@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kernel Team <kernel-team@fb.com>, Ingo Molnar <mingo@kernel.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	David Howells <dhowells@redhat.com>,
	Eric Dumazet <edumazet@google.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [PATCH tip/core/rcu 3/4] rcutorture: Make grace-period kthread report match RCU flavor being tested
Date: Tue, 15 Dec 2020 09:40:26 +0100	[thread overview]
Message-ID: <CAMuHMdXjUxfp0h=TiwNoZJUHrSD4sDwYEbuqNR4rcWSRFCjUtw@mail.gmail.com> (raw)
In-Reply-To: <20201105233933.20748-3-paulmck@kernel.org>

Hi Paul,

On Fri, Nov 6, 2020 at 12:40 AM <paulmck@kernel.org> wrote:
>
> From: "Paul E. McKenney" <paulmck@kernel.org>
>
> 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@intel.com>. ]
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

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@linux-m68k.org

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