From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <viktorin@rehivetech.com>
Received: from wes1-so1.wedos.net (wes1-so1.wedos.net [46.28.106.15])
 by dpdk.org (Postfix) with ESMTP id 6ED9F37A4
 for <dev@dpdk.org>; Mon,  2 Nov 2015 14:02:36 +0100 (CET)
Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz
 [147.229.13.147])
 by wes1-so1.wedos.net (Postfix) with ESMTPSA id 3nqDtN0lbtz4Nb;
 Mon,  2 Nov 2015 14:02:36 +0100 (CET)
Date: Mon, 2 Nov 2015 14:00:54 +0100
From: Jan Viktorin <viktorin@rehivetech.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Message-ID: <20151102140054.3cbe88c2@pcviktorin.fit.vutbr.cz>
In-Reply-To: <20151102055304.GB17659@localhost.localdomain>
References: <5289220.BuGOiUSaJq@xps13>
 <1446164742-29546-1-git-send-email-viktorin@rehivetech.com>
 <1446164742-29546-2-git-send-email-viktorin@rehivetech.com>
 <20151102055304.GB17659@localhost.localdomain>
Organization: RehiveTech
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: Vlastimil Kosar <kosar@rehivetech.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v5 01/15] eal/arm: atomic operations for ARM
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 02 Nov 2015 13:02:36 -0000

On Mon, 2 Nov 2015 11:23:05 +0530
Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote:

--snip--
> > +/*------------------------- 16 bit atomic operations -------------------------*/
> > +
> > +#ifndef RTE_FORCE_INTRINSICS
> > +static inline int
> > +rte_atomic16_cmpset(volatile uint16_t *dst, uint16_t exp, uint16_t src)
> > +{
> > +	return __atomic_compare_exchange(dst, &exp, &src, 0, __ATOMIC_ACQUIRE,
> > +		__ATOMIC_ACQUIRE) ? 1 : 0;
> > +}  
> 
> IMO, it should be __ATOMIC_SEQ_CST be instead of __ATOMIC_ACQUIRE.
> __ATOMIC_ACQUIRE works in conjunction with __ATOMIC_RELEASE.
> AFAIK, DPDK atomic api expects full barrier. C11 memory model not yet
> used.

Seems to be reasonable, thanks.

> So why can't we use RTE_FORCE_INTRINSICS based generic
> implementation. Same holds true for spinlock implementation too(i.e using
> RTE_FORCE_INTRINSICS). Am I missing something here ?

True. This was done with the intention to rewrite as a platform-specific
assembly. But it's never been done yet... If you mean to set
RTE_FORCE_INTRINSICS=y in the defconfig and remove this code entirely
(at least for ARMv7), I would agree.

> 
> 
> 
> > +
> > +static inline int rte_atomic16_test_and_set(rte_atomic16_t *v)
> > +{
> > +	return rte_atomic16_cmpset((volatile uint16_t *)&v->cnt, 0, 1);
> > +}
--snip--

-- 
   Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic