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  linux-cve-announce  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:	Tue, 5 Aug 2008 12:48:41 -0400 (EDT)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
cc:	linux-kernel@...r.kernel.org, mingo@...e.hu,
	akpm@...ux-foundation.org, oleg@...sign.ru,
	manfred@...orfullife.com, dipankar@...ibm.com, dvhltc@...ibm.com,
	niv@...ibm.com
Subject: Re: [PATCH tip/core/rcu] classic RCU locking and memory-barrier
 cleanups


On Tue, 5 Aug 2008, Paul E. McKenney wrote:
> ---
> 
>  rcuclassic.c |   51 +++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/rcuclassic.c b/kernel/rcuclassic.c
> index d3553ee..e6f42ef 100644
> --- a/kernel/rcuclassic.c
> +++ b/kernel/rcuclassic.c
> @@ -86,6 +86,7 @@ static void force_quiescent_state(struct rcu_data *rdp,
>  	int cpu;
>  	cpumask_t cpumask;
>  	set_need_resched();
> +	spin_lock(&rcp->lock);
>  	if (unlikely(!rcp->signaled)) {
>  		rcp->signaled = 1;
>  		/*
> @@ -111,6 +112,7 @@ static void force_quiescent_state(struct rcu_data *rdp,
>  		for_each_cpu_mask(cpu, cpumask)
>  			smp_send_reschedule(cpu);
>  	}
> +	spin_unlock(&rcp->lock);
>  }
>  #else
>  static inline void force_quiescent_state(struct rcu_data *rdp,
> @@ -124,7 +126,9 @@ static void __call_rcu(struct rcu_head *head, struct rcu_ctrlblk *rcp,
>  		struct rcu_data *rdp)
>  {
>  	long batch;
> -	smp_mb(); /* reads the most recently updated value of rcu->cur. */
> +
> +	head->next = NULL;
> +	smp_mb(); /* Read of rcu->cur must happen after any change by caller. */
>  
>  	/*
>  	 * Determine the batch number of this callback.
> @@ -174,7 +178,6 @@ void call_rcu(struct rcu_head *head,
>  	unsigned long flags;
>  
>  	head->func = func;
> -	head->next = NULL;
>  	local_irq_save(flags);
>  	__call_rcu(head, &rcu_ctrlblk, &__get_cpu_var(rcu_data));
>  	local_irq_restore(flags);
> @@ -203,7 +206,6 @@ void call_rcu_bh(struct rcu_head *head,
>  	unsigned long flags;
>  
>  	head->func = func;
> -	head->next = NULL;
>  	local_irq_save(flags);
>  	__call_rcu(head, &rcu_bh_ctrlblk, &__get_cpu_var(rcu_bh_data));
>  	local_irq_restore(flags);
> @@ -389,17 +391,17 @@ static void rcu_move_batch(struct rcu_data *this_rdp, struct rcu_head *list,
>  static void __rcu_offline_cpu(struct rcu_data *this_rdp,
>  				struct rcu_ctrlblk *rcp, struct rcu_data *rdp)
>  {
> -	/* if the cpu going offline owns the grace period
> +	/*
> +	 * if the cpu going offline owns the grace period
>  	 * we can block indefinitely waiting for it, so flush
>  	 * it here
>  	 */
>  	spin_lock_bh(&rcp->lock);
>  	if (rcp->cur != rcp->completed)
>  		cpu_quiet(rdp->cpu, rcp);
> -	spin_unlock_bh(&rcp->lock);
> -	/* spin_lock implies smp_mb() */

The spin_unlock is being removed here. Was that the smp_mb that was 
implied or is it the above spin_lock_bh?

>  	rcu_move_batch(this_rdp, rdp->donelist, rdp->donetail, rcp->cur + 1);
>  	rcu_move_batch(this_rdp, rdp->nxtlist, rdp->nxttail[2], rcp->cur + 1);
> +	spin_unlock_bh(&rcp->lock);
>  
>  	local_irq_disable();
>  	this_rdp->qlen += rdp->qlen;
> @@ -433,16 +435,19 @@ static void rcu_offline_cpu(int cpu)
>  static void __rcu_process_callbacks(struct rcu_ctrlblk *rcp,
>  					struct rcu_data *rdp)
>  {
> +	long completed_snap;
> +
>  	if (rdp->nxtlist) {
>  		local_irq_disable();
> +		completed_snap = ACCESS_ONCE(rcp->completed);
>  
>  		/*
>  		 * move the other grace-period-completed entries to
>  		 * [rdp->nxtlist, *rdp->nxttail[0]) temporarily
>  		 */
> -		if (!rcu_batch_before(rcp->completed, rdp->batch))
> +		if (!rcu_batch_before(completed_snap, rdp->batch))
>  			rdp->nxttail[0] = rdp->nxttail[1] = rdp->nxttail[2];
> -		else if (!rcu_batch_before(rcp->completed, rdp->batch - 1))
> +		else if (!rcu_batch_before(completed_snap, rdp->batch - 1))
>  			rdp->nxttail[0] = rdp->nxttail[1];
>  
>  		/*
> @@ -483,20 +488,38 @@ static void __rcu_process_callbacks(struct rcu_ctrlblk *rcp,
>  
>  static void rcu_process_callbacks(struct softirq_action *unused)
>  {
> +	/*
> +	 * Memory references from any prior RCU read-side critical sections
> +	 * executed by the interrupted code must be see before any RCU
> +	 * grace-period manupulations below.
> +	 */
> +
> +	smp_mb(); /* See above block comment. */
> +
>  	__rcu_process_callbacks(&rcu_ctrlblk, &__get_cpu_var(rcu_data));
>  	__rcu_process_callbacks(&rcu_bh_ctrlblk, &__get_cpu_var(rcu_bh_data));
> +
> +	/*
> +	 * Memory references from any later RCU read-side critical sections
> +	 * executed by the interrupted code must be see after any RCU
> +	 * grace-period manupulations above.
> +	 */
> +
> +	smp_mb(); /* See above block comment. */
>  }
>  
>  static int __rcu_pending(struct rcu_ctrlblk *rcp, struct rcu_data *rdp)
>  {
>  	if (rdp->nxtlist) {
> +		long completed_snap = ACCESS_ONCE(rcp->completed);
> +
>  		/*
>  		 * This cpu has pending rcu entries and the grace period
>  		 * for them has completed.
>  		 */
> -		if (!rcu_batch_before(rcp->completed, rdp->batch))
> +		if (!rcu_batch_before(completed_snap, rdp->batch))
>  			return 1;
> -		if (!rcu_batch_before(rcp->completed, rdp->batch - 1) &&
> +		if (!rcu_batch_before(completed_snap, rdp->batch - 1) &&
>  				rdp->nxttail[0] != rdp->nxttail[1])
>  			return 1;
>  		if (rdp->nxttail[0] != &rdp->nxtlist)
> @@ -547,6 +570,12 @@ int rcu_needs_cpu(int cpu)
>  	return !!rdp->nxtlist || !!rdp_bh->nxtlist || rcu_pending(cpu);
>  }
>  
> +/*
> + * Top-level function driving RCU grace-period detection, normally
> + * invoked from the scheduler-clock interrupt.  This function simply
> + * increments counters that are read only from softirq by this same
> + * CPU, so there are no memory barriers required.
> + */
>  void rcu_check_callbacks(int cpu, int user)
>  {
>  	if (user ||
> @@ -590,6 +619,7 @@ void rcu_check_callbacks(int cpu, int user)
>  static void rcu_init_percpu_data(int cpu, struct rcu_ctrlblk *rcp,
>  						struct rcu_data *rdp)
>  {
> +	spin_lock(&rcp->lock);
>  	memset(rdp, 0, sizeof(*rdp));
>  	rdp->nxttail[0] = rdp->nxttail[1] = rdp->nxttail[2] = &rdp->nxtlist;
>  	rdp->donetail = &rdp->donelist;
> @@ -597,6 +627,7 @@ static void rcu_init_percpu_data(int cpu, struct rcu_ctrlblk *rcp,
>  	rdp->qs_pending = 0;
>  	rdp->cpu = cpu;
>  	rdp->blimit = blimit;
> +	spin_unlock(&rcp->lock);
>  }
>  
>  static void __cpuinit rcu_online_cpu(int cpu)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ