From mboxrd@z Thu Jan 1 00:00:00 1970 From: Antoine Tenart Date: Thu, 27 Oct 2016 15:28:45 +0200 Subject: [U-Boot] [PATCH v3 01/10] arm: add atomic functions with return support In-Reply-To: <20161026151354.GA22713@leverpostej> References: <20161026121033.6339-1-antoine.tenart@free-electrons.com> <20161026121033.6339-2-antoine.tenart@free-electrons.com> <20161026151354.GA22713@leverpostej> Message-ID: <20161027132845.2i3dn4iqnb72epp4@kwain> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > 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: