From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mark Rutland <mark.rutland@arm.com>
Date: Wed, 26 Oct 2016 16:14:31 +0100
Subject: [U-Boot] [PATCH v3 01/10] arm: add atomic functions with return
 support
In-Reply-To: <20161026121033.6339-2-antoine.tenart@free-electrons.com>
References: <20161026121033.6339-1-antoine.tenart@free-electrons.com>
 <20161026121033.6339-2-antoine.tenart@free-electrons.com>
Message-ID: <20161026151354.GA22713@leverpostej>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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.