linux-kernel - Re: [PATCH RFC tip/core/rcu 11/12] rcu: fix race condition in synchronize_sched

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:	Wed, 10 Nov 2010 20:20:14 -0800
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	Tejun Heo <tj@...nel.org>, linux-kernel@...r.kernel.org,
	mingo@...e.hu, dipankar@...ibm.com, akpm@...ux-foundation.org,
	mathieu.desnoyers@...ymtl.ca, josh@...htriplett.org,
	niv@...ibm.com, tglx@...utronix.de, peterz@...radead.org,
	rostedt@...dmis.org, Valdis.Kletnieks@...edu, dhowells@...hat.com,
	eric.dumazet@...il.com, darren@...art.com
Subject: Re: [PATCH RFC tip/core/rcu 11/12] rcu: fix race condition in
 synchronize_sched_expedited()

On Wed, Nov 10, 2010 at 04:56:32PM +0800, Lai Jiangshan wrote:
> 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.

I am becoming more comfortable with it as well.  Tejun, what kind of
testing did you do?  Lai, could you please run it on your systems?

							Thanx, Paul

> Acked-by: Lai Jiangshan <laijs@...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();
> >  }
> > 
> 
--
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/