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

[Mark Rutland]

Hi,

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.

> +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,
Mark.