Re: [PATCH] driver core: fix shutdown races with probe/remove

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Wed, Jun 06, 2012 at 11:21:52AM -0400, Alan Stern wrote:
> On Wed, 6 Jun 2012, Paul E. McKenney wrote:
> 
> > No sane compiler would change it to a byte-at-a-time store, but the
> > compiler would nevertheless be within its rights to do so.  And a quick
> > review of certain LKML threads could easily cause anyone to question gcc's
> > sanity.  Furthermore, the compiler is permitted to make transformations
> > like the following, which it might well do to save a branch:
> > 
> > 	if (b)				a = 0;
> > 		a = 1;			if (b)
> > 	else					a = 1;
> > 		a = 0;
> 
> The compiler would be forbidden if the original code were
> 
> 	if (b)
> 		ACCESS_ONCE(a) = 1;
> 	else
> 		ACCESS_ONCE(a) = 0;
> 
> But if I remember correctly, the code snippet we were talking was more 
> like:
> 
> 	if (ACCESS_ONCE(b))
> 		a = 1;
> 
> Isn't this use of ACCESS_ONCE unnecessary?

That would depend on what else is nearby.

> > In short, without ACCESS_ONCE(), the compiler is within its rights to
> > assume that the code is single-threaded.  There are a large number of
> > non-obvious optimizations that are safe in single-threaded code but
> > destructive in multithreaded code.
> 
> Followed to its logical conclusion, this means that virtually every
> access to a shared variable that isn't protected by some sort of lock
> or isn't one of the special atomic operations needs to use
> ACCESS_ONCE.  The kernel must be riddled with places that don't do 
> this.

Agreed, we are relying on the compiler being unaggressive about
optimizations, which it usually is.  But it will continue becoming
more aggressive over time, so we should at least apply ACCESS_ONCE()
to new code.

> Besides, how sure are you that even in the presence of ACCESS_ONCE, the
> compiler will not make any unsafe transformations?  For example, is the
> compiler forbidden from transforming
> 
> 	if (a)
> 		ACCESS_ONCE(b) = 5;
> 
> into
> 
> 	tmp = c;
> 	c = 999;
> 	if (a)
> 		ACCESS_ONCE(b) = 5;
> 	c = tmp;
> 
> ?  After all, the c variable isn't protected by ACCESS_ONCE in the 
> original code.  And yet this transformation is clearly _unsafe_ for 
> multi-threaded operation.

There are some limitations because volatile accesses are not allowed to
move past "sequence points", but it would be possible to come up with
similar examples.  This sort of thing is why C1x has a memory model and
why it allows variables to be designated as needing to be SMP-safe.

In the meantime, things like ACCESS_ONCE() are what is available, limited
though they are.

							Thanx, Paul

> > In addition, and perhaps more important, ACCESS_ONCE() serves as useful
> > documentation of the fact that the variable is accessed outside of locks.
> 
> True.
> 
> Alan Stern
> 
> 
>