From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH] eal: fix bug in x86 cmpset Date: Fri, 10 Feb 2017 11:53:06 +0100 Message-ID: <2295899.BebvH11edl@xps13> References: <1475184293-18298-1-git-send-email-nikhil.rao@intel.com> <1919498.PKpEFfz702@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: Nikhil Rao , bruce.richardson@intel.com, Konstantin Ananyev , dev@dpdk.org To: "Hunt, David" Return-path: Received: from mail-wr0-f182.google.com (mail-wr0-f182.google.com [209.85.128.182]) by dpdk.org (Postfix) with ESMTP id 9B23F952 for ; Fri, 10 Feb 2017 11:53:09 +0100 (CET) Received: by mail-wr0-f182.google.com with SMTP id 89so104895542wrr.2 for ; Fri, 10 Feb 2017 02:53:09 -0800 (PST) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2017-02-10 10:39, Hunt, David: > > On 9/2/2017 4:53 PM, Thomas Monjalon wrote: > > 2016-11-06 22:09, Thomas Monjalon: > >> 2016-09-29 18:34, Thomas Monjalon: > >>> 2016-09-30 02:54, Nikhil Rao: > >>>> The original code used movl instead of xchgl, this caused > >>>> rte_atomic64_cmpset to use ebx as the lower dword of the source > >>>> to cmpxchg8b instead of the lower dword of function argument "src". > >>> Could you please start the explanation with a statement of > >>> what is wrong from an user point of view? > >>> It could help to understand how severe it is. > >> Please, we need a clear explanation of the bug, and an acknowledgement. > > Should we close this bug? > > I took a few minutes to look at this, and the issue can easily be > reproduced with a small snippet of code. > With the 'mov', the lower dword of the result is incorrect. This is > resolved by using 'xchgl'. > > void main() > { > uint64_t a = 0xff000000ff; > > rte_atomic64_cmpset( &a, 0xff000000ff, 0xfa000000fa); > printf("0x%lx\n", a); > } > > When using 'mov', the result is 0xfa00000000 > When using 'xchgl', the result is 0xfa000000fa, as expected. This operation is used a lot in drivers for link status. I think we need to clearly explain what was the consequence of this bug.