[PATCH 01/11] usb: chipidea: Add power management support

Russell King - ARM Linux linux at arm.linux.org.uk
Tue Oct 15 07:15:13 EDT 2013


On Tue, Oct 15, 2013 at 10:18:15AM +0800, Peter Chen wrote:
> So, the lessons for this topic are:
> 
> - If one atomic variable's operation only includes one instruction like
> atomic_read and atomic_set, it is not meaningful for using atomic
> operation, we can just use bool instead of it.

The lesson here is that these are 100% equivalent as far as safety from
races is concerned:

	a = atomic_read(&v);		a = v->counter;

	atomic_set(&v, b);		v->counter = b;

and in general, whenever atomic_read() gets used it's almost certainly
a sign of a bug.

Consider this (similar has been submitted):

	a = atomic_read(&v);
	if (a != 0)
		a += 1;

	atomic_set(&v, a);

and people have thought that somehow this is magically safe from races
because they're using atomic_t, and somehow that saves the universe.
The above is in fact no safer than:

	a = *v;
	if (a != 0)
		a += 1;
	*v = a;

The only thing that using atomic_* does is add a false sense of security
and a level of obfuscation to catch the unwary reviewer.

The reason is quite simple: a single access read in itself is atomic.
Either it has read the value, or it hasn't.  A single access store is
itself atomic.  Either the data has been written, or it hasn't.  The
issue is _always_ what you do around it.



More information about the linux-arm-kernel mailing list