From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f44.google.com (mail-pg0-f44.google.com [74.125.83.44]) by dpdk.org (Postfix) with ESMTP id EF9421B346 for ; Thu, 2 Nov 2017 02:09:34 +0100 (CET) Received: by mail-pg0-f44.google.com with SMTP id s75so3609979pgs.0 for ; Wed, 01 Nov 2017 18:09:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=wZ7FiPGHiCWz4vGDdXx/OitMjU/MAOhpOPZYQqwjz4k=; b=fZIv3lloFH1wtNXn8NKw5kOYTFrPghy39H6Q9d8sgtk6jNA0Wm1RYwj9PtMFaSz/ff lESI0ycpDlIwSVpLHvQfKyvRLK+hZlWvYF2rQRm61DMuBniVwKGoJ0UiwbXmQVtVoE+q AN84S/6/eszVku4EWU2yT0s8OsgBB57LGhfUszuVtRC871trkpSfiJaGYTMZ+PUNraPI 416B7DhbNXVFb68k5ympC1KaBRUI/GXeGsC4vgTDV4UtnCqsO3EoxVLpuwIOHd9CiHxv BmwPluIPoa1NsIQw90uYuBtTYH4fwoDJPQlGtTjMskV35Axeeg63T+6CXxyyASbMPfjX l/gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=wZ7FiPGHiCWz4vGDdXx/OitMjU/MAOhpOPZYQqwjz4k=; b=ptey1NoV0UN/+sJHEivRCuCFgJ5jcwL6Fw2hWtHOe8mOPdQ0M0SrfpS255LjUh2dMv TG02HbvmSlhJYf4P4tm2kfKnBNkO1v0Hx5kao8+E4GUNxYpJX08q/tgfCpieEwecy4vE 0oMcZ2VmiVTDqs18D0hVMrjCZGt4RXfXjc3S+MWK4P+2lDefuOtO+Bv+bzO2LbQJKTjw yU1NHEGKnto+MERQ2gxj8ZR3ILuiDBE6/pJxcrDi3RwHBYvQoPH7Gw3huL/Gda0xqvdz 9Cci3uqIAXQ1nuFlOoUObohcpL+HzjFzGZ+cGGLitv1y/WMfDHFZdiyMXITydKvmUHMS 03Fw== X-Gm-Message-State: AMCzsaXibJkSKWSsPP2ssPwvjfbhJTWgP8/4ou5iCeNIwF9y2DNGACDy ooI4blRTGN0fexDx1WDVLBs= X-Google-Smtp-Source: ABhQp+QBZgTuqiOAtFejoIx/Z87nAQDOezNuRtcmoakzYVMZopboqStDBr//MXVpqLSCTzUre7OuKA== X-Received: by 10.98.16.66 with SMTP id y63mr1800949pfi.192.1509584974012; Wed, 01 Nov 2017 18:09:34 -0700 (PDT) Received: from [0.0.0.0] (67.209.179.165.16clouds.com. [67.209.179.165]) by smtp.gmail.com with ESMTPSA id y30sm3502203pge.27.2017.11.01.18.09.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Nov 2017 18:09:33 -0700 (PDT) To: Jerin Jacob Cc: "Ananyev, Konstantin" , "Zhao, Bing" , Olivier MATZ , "dev@dpdk.org" , "jia.he@hxt-semitech.com" , "jie2.liu@hxt-semitech.com" , "bing.zhao@hxt-semitech.com" , "Richardson, Bruce" , jianbo.liu@arm.com, hemant.agrawal@nxp.com References: <2601191342CEEE43887BDE71AB9772585FAAB570@IRSMSX103.ger.corp.intel.com> <3e580cd7-2854-d855-be9c-7c4ce06e3ed5@gmail.com> <20171020054319.GA4249@jerin> <20171023100617.GA17957@jerin> <20171025132642.GA13977@jerin> <20171031111433.GA21742@jerin> <69adfb00-4582-b362-0540-d1d9d6bcf6aa@gmail.com> <20171101190420.GA21407@jerin> From: Jia He Message-ID: <3c4b96f3-ca6c-c792-7a78-b44e4a7cef12@gmail.com> Date: Thu, 2 Nov 2017 09:09:24 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171101190420.GA21407@jerin> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] ring: guarantee ordering of cons/prod loading when doing enqueue/dequeue 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: Thu, 02 Nov 2017 01:09:35 -0000 Hi Jerin On 11/2/2017 3:04 AM, Jerin Jacob Wrote: > Date: Thu, 2 Nov 2017 00:27:46 +0530 > From: Jerin Jacob > To: Jia He > Cc: "Ananyev, Konstantin" , > "Zhao, Bing" , > Olivier MATZ , > "dev@dpdk.org" , > "jia.he@hxt-semitech.com" , > "jie2.liu@hxt-semitech.com" , > "bing.zhao@hxt-semitech.com" , > "Richardson, Bruce" , > jianbo.liu@arm.com, hemant.agrawal@nxp.com > Subject: Re: [dpdk-dev] [PATCH] ring: guarantee ordering of cons/prod loading > when doing enqueue/dequeue > Message-ID: <20171101185723.GA18759@jerin> > References: <2601191342CEEE43887BDE71AB9772585FAAB570@IRSMSX103.ger.corp.intel.com> > <3e580cd7-2854-d855-be9c-7c4ce06e3ed5@gmail.com> > <20171020054319.GA4249@jerin> > > <20171023100617.GA17957@jerin> > > <20171025132642.GA13977@jerin> > > <20171031111433.GA21742@jerin> > <69adfb00-4582-b362-0540-d1d9d6bcf6aa@gmail.com> > MIME-Version: 1.0 > Content-Type: text/plain; charset=iso-8859-1 > Content-Disposition: inline > Content-Transfer-Encoding: 8bit > In-Reply-To: <69adfb00-4582-b362-0540-d1d9d6bcf6aa@gmail.com> > User-Agent: Mutt/1.9.1 (2017-09-22) > > -----Original Message----- >> Date: Wed, 1 Nov 2017 10:53:12 +0800 >> From: Jia He >> To: Jerin Jacob >> Cc: "Ananyev, Konstantin" , "Zhao, Bing" >> , Olivier MATZ , >> "dev@dpdk.org" , "jia.he@hxt-semitech.com" >> , "jie2.liu@hxt-semitech.com" >> , "bing.zhao@hxt-semitech.com" >> , "Richardson, Bruce" >> , jianbo.liu@arm.com, hemant.agrawal@nxp.com >> Subject: Re: [dpdk-dev] [PATCH] ring: guarantee ordering of cons/prod >> loading when doing enqueue/dequeue >> User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 >> Thunderbird/52.4.0 >> >> Hi Jerin >> >> Thanks for your suggestions. I will try to use config macro to let it be >> chosen by user. > It is better, if we avoid #ifdef's in the common code. I think, you can > do the scheme like examples/l3fwd/l3fwd_em_hlm_neon.h. Where, > the common code will have all generic routine, difference will be > moved to a separate files to reduce #ifdef clutter. > >> I need to point out one possible issue in your load_acq/store_rel patch >> >> at https://github.com/jerinjacobk/mytests/blob/master/ring/0001-ring-using-c11-memory-model.patch >> >> @@ -516,8 +541,13 @@ __rte_ring_move_cons_head(struct rte_ring *r, int >> is_sc, >>          /* Restore n as it may change every loop */ >>          n = max; >> >> +#if 0 >>          *old_head = r->cons.head; >>          const uint32_t prod_tail = r->prod.tail; >> +#else >> +        *old_head = __atomic_load_n(&r->cons.head, __ATOMIC_RELAXED); >>                --[1] >> +        const uint32_t prod_tail = __atomic_load_n(&r->prod.tail, >> __ATOMIC_ACQUIRE);   --[2] >> +#endif >> >> line [1] __ATOMIC_RELAXED is not enough for this case(tested in our ARM64 >> server). >> >> line [2] __ATOMIC_ACQUIRE guarantee the 2nd load will not be reorded before >> the 1st load, but will not >> >> guarantee the 1st load will not be reordered after the 2nd load. Please also > For me it looks same. [1] can not cross [2] is same as [2] cannot cross > [1], if [1] are [2] at back to back. No ? No, IIUC, load_acquire(a) is equal to the pseudo codes: load(a) one-direction-barrier() instead of one-direction-barrier() load(a) Thus, in below cases, load(a) and load(b) can still be reordered, this is not the semantic violation of the load_acquire: load(a) load_acquire(b) IIUC, the orginal implementation in https://github.com/freebsd/freebsd/blob/master/sys/sys/buf_ring.h#L170 is not optimal. I tested the changes as follow and it works fine: +        *old_head = __atomic_load_n(&r->cons.head, __ATOMIC_ACQUIRE);         const uint32_t prod_tail = r->prod.tail; i.e. load_acquire(a) load(b) Cheers, Jia