Re: [PATCH tip/core/rcu 10/20] rcu: Add functions to test for trivial grace periods

[Josh Triplett]

On Sat, Jan 14, 2017 at 01:13:11AM -0800, Paul E. McKenney wrote:
> Under some circumstances, RCU grace periods are zero cost.  For
> RCU-preempt, this is the case during boot, and for RCU-bh and RCU-sched,
> this is the case if there is only one CPU.  This means that RCU users
> might wish to dispense with grace-period-avoidance strategies when
> grace periods are zero cost, so this commit adds rcu_trivial_gp(),
> rcu_bh_trivial_gp(), and rcu_sched_trivial_gp() to test for these
> conditions.  Because the conditions leading to zero-cost grace periods
> can change at any time (for example, when a second CPU is onlined), these
> functions should be used as performance hints, and must not be relied
> on for correctness.  For example, even if rcu_trivial_gp() returns true,
> you are required to invoke synchronize_rcu().
> 
> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

Do you have anything planned that uses these functions?

Exposing this information, rather than just letting callers call
synchronize_rcu() and sometimes get "free" grace periods, seems
potentially error-prone.

If you keep these functions, please expand the comments above them to
explicitly include the explanation in the commit message, and
specifically that you must always call the corresponding synchronize
function even if these functions return true.

>  include/linux/rcupdate.h |  8 ++++++++
>  include/linux/rcutiny.h  | 10 ++++++++++
>  include/linux/rcutree.h  |  2 ++
>  kernel/rcu/tree.c        | 24 ++++++++++++++++++++++++
>  kernel/rcu/tree_plugin.h | 11 +++++++++++
>  5 files changed, 55 insertions(+)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 01f71e1d2e94..a6222478b87d 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -293,6 +293,7 @@ void __rcu_read_lock(void);
>  void __rcu_read_unlock(void);
>  void rcu_read_unlock_special(struct task_struct *t);
>  void synchronize_rcu(void);
> +bool rcu_trivial_gp(void);
>  
>  /*
>   * Defined as a macro as it is a very low level header included from
> @@ -448,6 +449,13 @@ bool __rcu_is_watching(void);
>  #define RCU_SCHEDULER_INIT	1
>  #define RCU_SCHEDULER_RUNNING	2
>  
> +#ifndef CONFIG_PREEMPT_RCU
> +static inline bool rcu_trivial_gp(void)
> +{
> +	return rcu_sched_trivial_gp();
> +}
> +#endif /* #ifndef CONFIG_PREEMPT_RCU */
> +
>  /*
>   * init_rcu_head_on_stack()/destroy_rcu_head_on_stack() are needed for dynamic
>   * initialization and destruction of rcu_head on the stack. rcu_head structures
> diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> index ac81e4063b40..a77dafe79813 100644
> --- a/include/linux/rcutiny.h
> +++ b/include/linux/rcutiny.h
> @@ -82,6 +82,16 @@ static inline void synchronize_sched_expedited(void)
>  	synchronize_sched();
>  }
>  
> +static inline bool rcu_sched_trivial_gp(void)
> +{
> +	return true;
> +}
> +
> +static inline bool rcu_bh_trivial_gp(void)
> +{
> +	return true;
> +}
> +
>  static inline void kfree_call_rcu(struct rcu_head *head,
>  				  rcu_callback_t func)
>  {
> diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> index 63a4e4cf40a5..fcd61cb08851 100644
> --- a/include/linux/rcutree.h
> +++ b/include/linux/rcutree.h
> @@ -47,6 +47,8 @@ static inline void rcu_virt_note_context_switch(int cpu)
>  void synchronize_rcu_bh(void);
>  void synchronize_sched_expedited(void);
>  void synchronize_rcu_expedited(void);
> +bool rcu_sched_trivial_gp(void);
> +bool rcu_bh_trivial_gp(void);
>  
>  void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
>  
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index d7b63b88434b..ed5a17aca281 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3293,6 +3293,18 @@ void synchronize_sched(void)
>  EXPORT_SYMBOL_GPL(synchronize_sched);
>  
>  /**
> + * rcu_sched_trivial_gp - Are RCU-sched grace periods trivially zero cost?
> + *
> + * Returns true if RCU-sched grace periods are currently zero cost, which
> + * they are if there is only one CPU.  Note that unless you take steps to
> + * prevent it, the number of CPUs might change at any time.
> + */
> +bool rcu_sched_trivial_gp(void)
> +{
> +	return rcu_blocking_is_gp();
> +}
> +
> +/**
>   * synchronize_rcu_bh - wait until an rcu_bh grace period has elapsed.
>   *
>   * Control will return to the caller some time after a full rcu_bh grace
> @@ -3320,6 +3332,18 @@ void synchronize_rcu_bh(void)
>  EXPORT_SYMBOL_GPL(synchronize_rcu_bh);
>  
>  /**
> + * rcu_bh_trivial_gp - Are RCU-bh grace periods trivially zero cost?
> + *
> + * Returns true if RCU-bh grace periods are currently zero cost, which
> + * they are if there is only one CPU.  Note that unless you take steps to
> + * prevent it, the number of CPUs might change at any time.
> + */
> +bool rcu_bh_trivial_gp(void)
> +{
> +	return rcu_blocking_is_gp();
> +}
> +
> +/**
>   * get_state_synchronize_rcu - Snapshot current RCU state
>   *
>   * Returns a cookie that is used by a later call to cond_synchronize_rcu()
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 56583e764ebf..e92d67a3fad2 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -680,6 +680,17 @@ void synchronize_rcu(void)
>  EXPORT_SYMBOL_GPL(synchronize_rcu);
>  
>  /**
> + * rcu_trivial_gp - Are RCU grace periods trivially zero cost?
> + *
> + * Returns true if RCU grace periods are currently zero cost, which
> + * they are during boot.
> + */
> +bool rcu_trivial_gp(void)
> +{
> +	return rcu_scheduler_active == RCU_SCHEDULER_INACTIVE;
> +}
> +
> +/**
>   * rcu_barrier - Wait until all in-flight call_rcu() callbacks complete.
>   *
>   * Note that this primitive does not necessarily wait for an RCU grace period
> -- 
> 2.5.2
>