[U-Boot] [PATCH v3 01/10] arm: add atomic functions with return support

[Antoine Tenart]

HI Mark,

On Wed, Oct 26, 2016 at 04:14:31PM +0100, Mark Rutland wrote:
> On Wed, Oct 26, 2016 at 02:10:24PM +0200, Antoine Tenart wrote:
> > Implement three atomic functions to allow making an atomic operation
> > that returns the value. Adds: atomic_add_return(), atomic_sub_return(),
> > atomic_inc_return() and atomic_dec_return().
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart at free-electrons.com>
> 
> In the cover letter, you mentioned that these are needed for SMP
> systems (for managing common suspend entry/exit management).
> 
> The below operations are *not* atomic in SMP systems, and can only work
> in UP. The same is true of all the existing code in the same file.

That's what I feared...

> > +static inline int atomic_add_return(int i, volatile atomic_t *v)
> > +{
> > +	unsigned long flags = 0;
> > +	int ret;
> > +
> > +	local_irq_save(flags);
> > +	ret = (v->counter += i);
> > +	local_irq_restore(flags);
> > +
> > +	return ret;
> > +}
> 
> local_irq_{save,restore}() won't serialize two CPUs. Consider two CPUs
> executing this in parallel (assuming the compiler chooses a register rX
> as a temporary):
> 
> 	CPU0				CPU1
> 
> 	local_irq_save()		local_irq_save()
> 	rX = v->counter			rX = v->counter
> 	rX += i				rX += i
> 	v->counter = rX
> 					v->counter = rX
> 	local_irq_restore()		local_irq_restore()
> 
> At the end of this, CPU0's increment of v->counter is lost.
> 
> If you need atomics on SMP, you'll need to use the
> {load,store}-exclusive instructions, which come with a number of
> additional requirements (e.g. the memory being operated on must be
> mapped as write-back cacheable, the entire retry loop needs to be
> writtten in asm, etc).

Thanks a lot for the review and for the explanation.

So I need to do something like what's done in the kernel, at
http://lxr.free-electrons.com/source/arch/arm/include/asm/atomic.h#L60
for ARM; by using ldrex and strex.

Thanks,

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161027/baec227b/attachment.sig>