[v1] Further shrink srcu_struct to promote cache locality

[PATCH rcu 01/20] rcu-tasks: Fix warning for unused tasks_rcu_exit_srcu

Posted by Paul E. McKenney 2 years, 10 months ago

The tasks_rcu_exit_srcu variable is used only by kernels built
with CONFIG_TASKS_RCU=y, but is defined for all kernesl with
CONFIG_TASKS_RCU_GENERIC=y.  Therefore, in kernels built with
CONFIG_TASKS_RCU_GENERIC=y but CONFIG_TASKS_RCU=n, this gives
a "defined but not used" warning.

This commit therefore moves this variable under CONFIG_TASKS_RCU.

Link: https://lore.kernel.org/oe-kbuild-all/202303191536.XzMSyzTl-lkp@intel.com/
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/tasks.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index bfb5e1549f2b..185358c2f08d 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -136,8 +136,10 @@ static struct rcu_tasks rt_name =							\
 	.kname = #rt_name,								\
 }
 
+#ifdef CONFIG_TASKS_RCU
 /* Track exiting tasks in order to allow them to be waited for. */
 DEFINE_STATIC_SRCU(tasks_rcu_exit_srcu);
+#endif
 
 /* Avoid IPIing CPUs early in the grace period. */
 #define RCU_TASK_IPI_DELAY (IS_ENABLED(CONFIG_TASKS_TRACE_RCU_READ_MB) ? HZ / 2 : 0)
-- 
2.40.0.rc2

Re: [PATCH rcu 01/20] rcu-tasks: Fix warning for unused tasks_rcu_exit_srcu

Posted by Frederic Weisbecker 2 years, 10 months ago

On Thu, Mar 30, 2023 at 03:47:07PM -0700, Paul E. McKenney wrote:
> The tasks_rcu_exit_srcu variable is used only by kernels built
> with CONFIG_TASKS_RCU=y, but is defined for all kernesl with
> CONFIG_TASKS_RCU_GENERIC=y.  Therefore, in kernels built with
> CONFIG_TASKS_RCU_GENERIC=y but CONFIG_TASKS_RCU=n, this gives
> a "defined but not used" warning.
> 
> This commit therefore moves this variable under CONFIG_TASKS_RCU.
> 
> Link: https://lore.kernel.org/oe-kbuild-all/202303191536.XzMSyzTl-lkp@intel.com/
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  kernel/rcu/tasks.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index bfb5e1549f2b..185358c2f08d 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -136,8 +136,10 @@ static struct rcu_tasks rt_name =							\
>  	.kname = #rt_name,								\
>  }
>  
> +#ifdef CONFIG_TASKS_RCU
>  /* Track exiting tasks in order to allow them to be waited for. */
>  DEFINE_STATIC_SRCU(tasks_rcu_exit_srcu);
> +#endif

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

BTW should  exit_tasks_rcu_start/stop() be defined as static inline stubs
whenever !CONFIG_TASKS_RCU ? Currently:

* if CONFIG_TASKS_RCU_GENERIC=n, the stubs are as usual (static inline)

* if CONFIG_TASKS_RCU_GENERIC=y && CONFIG_TASKS_RCU=n, then the stubs are
  defined as empty linked function (is the compiler smart enough to remove
  the empty call?)

Re: [PATCH rcu 01/20] rcu-tasks: Fix warning for unused tasks_rcu_exit_srcu

Posted by Paul E. McKenney 2 years, 10 months ago

On Fri, Mar 31, 2023 at 01:58:38PM +0200, Frederic Weisbecker wrote:
> On Thu, Mar 30, 2023 at 03:47:07PM -0700, Paul E. McKenney wrote:
> > The tasks_rcu_exit_srcu variable is used only by kernels built
> > with CONFIG_TASKS_RCU=y, but is defined for all kernesl with
> > CONFIG_TASKS_RCU_GENERIC=y.  Therefore, in kernels built with
> > CONFIG_TASKS_RCU_GENERIC=y but CONFIG_TASKS_RCU=n, this gives
> > a "defined but not used" warning.
> > 
> > This commit therefore moves this variable under CONFIG_TASKS_RCU.
> > 
> > Link: https://lore.kernel.org/oe-kbuild-all/202303191536.XzMSyzTl-lkp@intel.com/
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> >  kernel/rcu/tasks.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index bfb5e1549f2b..185358c2f08d 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -136,8 +136,10 @@ static struct rcu_tasks rt_name =							\
> >  	.kname = #rt_name,								\
> >  }
> >  
> > +#ifdef CONFIG_TASKS_RCU
> >  /* Track exiting tasks in order to allow them to be waited for. */
> >  DEFINE_STATIC_SRCU(tasks_rcu_exit_srcu);
> > +#endif
> 
> Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

Thank you!  I will apply this on my next rebase.

> BTW should  exit_tasks_rcu_start/stop() be defined as static inline stubs
> whenever !CONFIG_TASKS_RCU ? Currently:
> 
> * if CONFIG_TASKS_RCU_GENERIC=n, the stubs are as usual (static inline)
> 
> * if CONFIG_TASKS_RCU_GENERIC=y && CONFIG_TASKS_RCU=n, then the stubs are
>   defined as empty linked function (is the compiler smart enough to remove
>   the empty call?)

Good point!  They are currently defined as static inline only if there
are no RCU Tasks flavors built into the kernel, so if there was (say)
RCU Tasks Trace but no RCU Tasks, these functions would needlessly be
actual calls to empty functions.

I am not sure that there are any kernels built like this, and this is not
on a fastpath, but I would welcome a patch that made this more precise.
Testing will require a few kernel builds, though.  ;-)

							Thanx, Paul