From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f174.google.com (mail-pf0-f174.google.com [209.85.192.174]) by dpdk.org (Postfix) with ESMTP id 26D53952 for ; Fri, 10 Feb 2017 17:46:53 +0100 (CET) Received: by mail-pf0-f174.google.com with SMTP id e4so10440279pfg.1 for ; Fri, 10 Feb 2017 08:46:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Eyq6Tpki9tJ9RWE775JfJYZtt1FBsJXf/GotDvByg2o=; b=I6z34EPUcEmx1Pw0YlJGcDTkB7N/Nsaf8CloDmp2MxVfyEEPP0gPO843SXBAyLE2u0 ei2itrV23kOB5AuDWga+tC6yBAg41Bhz2lmEJVpEk4JP2pZubEN1fOev8UgxYZNlWPkb sW05cGTD2B8WOlahkAJ6jVaGikTQ3fVSyqNqCxDNpZMEmzWC8Q9uUI39b03smQhQzBEN YS9irdgvxWwPrAxM6uw8zoX+Da/OEFW2sD2gLX60qmLhOC8UZe5XddnoijOP9kJ9tAjv TTcwYQ6aZSstARyPZovUqlx+oYx9DjT99PeGOBpTRYIf5ARy10+GUQc0uuMZ+ZTLUg5p 4lDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Eyq6Tpki9tJ9RWE775JfJYZtt1FBsJXf/GotDvByg2o=; b=S0FPcCWjiOPZOVRNyvPI2L1tySPfXGzsnLcXiYhEfAVmBJIUpNnadEzXy07OTLKz6z B23Y/rD1MG8LV0GPDqKWZkEWdCkUWJ7Xx/fy0UFrWxJFgpkI378o+XyvDGBTY15NWYcG U7R1JbChxsZrgUCB8sm1rh3SYYiFuvwptEtWVgBesvTyuC/24gvNxHjttSh/XctwSWv8 5k/CRQZdlgx0TPvvkcTW1pgfm5EPnRkZ9v5LAZlxgJJ0vVwXAx4p6F+sS/Hy4Vrw6uRO Kw/RGJWYImgAttxXyB83ZE+9HpCG0+GysGUQLOX+4Jaa0XL7r1CCXpHq000DOnFx+0bi dqRw== X-Gm-Message-State: AMke39lbtAVqw5r1DA7LoeoAQzhAgGFzFv9fRim9nCloUAV1EkuvuifvshIENKTVkLBijQ== X-Received: by 10.99.65.1 with SMTP id o1mr11721817pga.93.1486745212426; Fri, 10 Feb 2017 08:46:52 -0800 (PST) Received: from xeon-e3 (204-195-18-65.wavecable.com. [204.195.18.65]) by smtp.gmail.com with ESMTPSA id t22sm429629pfa.114.2017.02.10.08.46.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 10 Feb 2017 08:46:52 -0800 (PST) Date: Fri, 10 Feb 2017 08:46:44 -0800 From: Stephen Hemminger To: Thomas Monjalon Cc: "Hunt, David" , Nikhil Rao , bruce.richardson@intel.com, Konstantin Ananyev , dev@dpdk.org Message-ID: <20170210084644.52651be6@xeon-e3> In-Reply-To: <2295899.BebvH11edl@xps13> References: <1475184293-18298-1-git-send-email-nikhil.rao@intel.com> <1919498.PKpEFfz702@xps13> <2295899.BebvH11edl@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] eal: fix bug in x86 cmpset X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Feb 2017 16:46:53 -0000 On Fri, 10 Feb 2017 11:53:06 +0100 Thomas Monjalon wrote: > 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. A bigger issue is why there are a huge number of copies of the same link code in drivers. Definitely should be common code. Also why is cmpset used here when a simple atomic_set would work as well for what was intended.