lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 16 Jan 2017 00:01:29 -0800
From:   Josh Triplett <josh@...htriplett.org>
To:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:     linux-kernel@...r.kernel.org, mingo@...nel.org,
        jiangshanlai@...il.com, dipankar@...ibm.com,
        akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
        tglx@...utronix.de, peterz@...radead.org, rostedt@...dmis.org,
        dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com,
        fweisbec@...il.com, oleg@...hat.com, bobby.prani@...il.com
Subject: Re: [PATCH tip/core/rcu 10/20] rcu: Add functions to test for
 trivial grace periods

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@...ux.vnet.ibm.com>

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
> 

Powered by blists - more mailing lists