Re: [PATCH RFC tip/core/rcu 11/12] rcu: fix race condition in synchronize_sched_expedited()

[Lai Jiangshan <laijs@cn.fujitsu.com>]

On 11/09/2010 09:26 PM, Tejun Heo wrote:
> Hello, Paul.
> 
> 
> How about something like the following?  It's slightly bigger but I
> think it's a bit easier to understand.  Thanks.

Hello, Paul, Tejun,

I think this approach is good and much better when several tasks
call synchronize_sched_expedited() at the same time.

Acked-by: Lai Jiangshan <laijs@cn.fujitsu.com>

> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index aa14a56..0069be5 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -9342,7 +9342,8 @@ EXPORT_SYMBOL_GPL(synchronize_sched_expedited);
> 
>  #else /* #ifndef CONFIG_SMP */
> 
> -static atomic_t synchronize_sched_expedited_count = ATOMIC_INIT(0);
> +static atomic_t sync_sched_expedited_token = ATOMIC_INIT(0);
> +static atomic_t sync_sched_expedited_done = ATOMIC_INIT(0);
> 
>  static int synchronize_sched_expedited_cpu_stop(void *data)
>  {
> @@ -9373,11 +9374,18 @@ static int synchronize_sched_expedited_cpu_stop(void *data)
>   */
>  void synchronize_sched_expedited(void)
>  {
> -	int snap, trycount = 0;
> +	int my_tok, tok, t, trycount = 0;
> +
> +	smp_mb();  /* ensure prior mod happens before getting token. */
> +
> +	/*
> +	 * Get a token.  This is used to coordinate with other
> +	 * concurrent syncers and consolidate multiple syncs.
> +	 */
> +	my_tok = tok = atomic_inc_return(&sync_sched_expedited_token);
> 
> -	smp_mb();  /* ensure prior mod happens before capturing snap. */
> -	snap = atomic_read(&synchronize_sched_expedited_count) + 1;
>  	get_online_cpus();
> +
>  	while (try_stop_cpus(cpu_online_mask,
>  			     synchronize_sched_expedited_cpu_stop,
>  			     NULL) == -EAGAIN) {
> @@ -9388,13 +9396,34 @@ void synchronize_sched_expedited(void)
>  			synchronize_sched();
>  			return;
>  		}
> -		if (atomic_read(&synchronize_sched_expedited_count) - snap > 0) {
> +
> +		/*
> +		 * If the done count reached @my_tok, we know at least
> +		 * one synchronization happened since we entered this
> +		 * function.
> +		 */
> +		if (atomic_read(&sync_sched_expedited_done) - my_tok >= 0) {
>  			smp_mb(); /* ensure test happens before caller kfree */
>  			return;
>  		}
> +
>  		get_online_cpus();
> +
> +		/* about to retry, get the latest token value */
> +		tok = atomic_read(&sync_sched_expedited_token);
>  	}
> -	atomic_inc(&synchronize_sched_expedited_count);
> +
> +	/*
> +	 * We now know that everything upto @tok is synchronized.
> +	 * Update done counter which should always monotonically
> +	 * increase (with wrapping considered).
> +	 */
> +	do {
> +		t = atomic_read(&sync_sched_expedited_done);
> +		if (t - tok >= 0)
> +			break;
> +	} while (atomic_cmpxchg(&sync_sched_expedited_done, t, tok) != t);
> +
>  	smp_mb__after_atomic_inc(); /* ensure post-GP actions seen after GP. */
>  	put_online_cpus();
>  }
>