From: Jia He <hejianet@gmail.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Zhao, Bing" <ilovethull@163.com>,
Olivier MATZ <olivier.matz@6wind.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"jia.he@hxt-semitech.com" <jia.he@hxt-semitech.com>,
"jie2.liu@hxt-semitech.com" <jie2.liu@hxt-semitech.com>,
"bing.zhao@hxt-semitech.com" <bing.zhao@hxt-semitech.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
jianbo.liu@arm.com, hemant.agrawal@nxp.com
Subject: Re: [dpdk-dev] [PATCH] ring: guarantee ordering of cons/prod loading when doing enqueue/dequeue
Date: Thu, 2 Nov 2017 09:09:24 +0800 [thread overview]
Message-ID: <3c4b96f3-ca6c-c792-7a78-b44e4a7cef12@gmail.com> (raw)
In-Reply-To: <20171101190420.GA21407@jerin>
Hi Jerin
On 11/2/2017 3:04 AM, Jerin Jacob Wrote:
> Date: Thu, 2 Nov 2017 00:27:46 +0530
> From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> To: Jia He <hejianet@gmail.com>
> Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
> "Zhao, Bing" <ilovethull@163.com>,
> Olivier MATZ <olivier.matz@6wind.com>,
> "dev@dpdk.org" <dev@dpdk.org>,
> "jia.he@hxt-semitech.com" <jia.he@hxt-semitech.com>,
> "jie2.liu@hxt-semitech.com" <jie2.liu@hxt-semitech.com>,
> "bing.zhao@hxt-semitech.com" <bing.zhao@hxt-semitech.com>,
> "Richardson, Bruce" <bruce.richardson@intel.com>,
> 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>
> <ab7154a2-a9f8-f12e-b6a0-2805c2065e2e@gmail.com>
> <20171023100617.GA17957@jerin>
> <b0e6eae2-e61b-1946-ac44-d781225778e5@gmail.com>
> <20171025132642.GA13977@jerin>
> <b54ce545-b75d-9813-209f-4125bd76c7db@gmail.com>
> <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 <hejianet@gmail.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "Zhao, Bing"
>> <ilovethull@163.com>, Olivier MATZ <olivier.matz@6wind.com>,
>> "dev@dpdk.org" <dev@dpdk.org>, "jia.he@hxt-semitech.com"
>> <jia.he@hxt-semitech.com>, "jie2.liu@hxt-semitech.com"
>> <jie2.liu@hxt-semitech.com>, "bing.zhao@hxt-semitech.com"
>> <bing.zhao@hxt-semitech.com>, "Richardson, Bruce"
>> <bruce.richardson@intel.com>, 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
next prev parent reply other threads:[~2017-11-02 1:09 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-10 9:56 Jia He
2017-10-12 15:53 ` Olivier MATZ
2017-10-12 16:15 ` Stephen Hemminger
2017-10-12 17:05 ` Ananyev, Konstantin
2017-10-12 17:23 ` Jerin Jacob
2017-10-13 1:02 ` Jia He
2017-10-13 1:15 ` Jia He
2017-10-13 1:16 ` Jia He
2017-10-13 1:49 ` Jerin Jacob
2017-10-13 3:23 ` Jia He
2017-10-13 5:57 ` Zhao, Bing
2017-10-13 7:33 ` Jianbo Liu
2017-10-13 8:20 ` Jia He
2017-10-19 10:02 ` Ananyev, Konstantin
2017-10-19 11:18 ` Zhao, Bing
2017-10-19 14:15 ` Ananyev, Konstantin
2017-10-19 20:02 ` Ananyev, Konstantin
2017-10-20 1:57 ` Jia He
2017-10-20 5:43 ` Jerin Jacob
2017-10-23 8:49 ` Jia He
2017-10-23 9:05 ` Kuusisaari, Juhamatti
2017-10-23 9:10 ` Bruce Richardson
2017-10-23 10:06 ` Jerin Jacob
2017-10-24 2:04 ` Jia He
2017-10-25 13:26 ` Jerin Jacob
2017-10-26 2:27 ` Jia He
2017-10-31 2:55 ` Jia He
2017-10-31 11:14 ` Jerin Jacob
2017-11-01 2:53 ` Jia He
2017-11-01 19:04 ` Jerin Jacob
2017-11-02 1:09 ` Jia He [this message]
2017-11-02 8:57 ` Jia He
2017-11-03 2:55 ` Jia He
2017-11-03 12:47 ` Jerin Jacob
2017-11-01 4:48 ` Jia He
2017-11-01 19:10 ` Jerin Jacob
2017-10-20 7:03 ` Ananyev, Konstantin
2017-10-13 0:24 ` Liu, Jie2
2017-10-13 2:12 ` Zhao, Bing
2017-10-13 2:34 ` Jerin Jacob
2017-10-16 10:51 ` Kuusisaari, Juhamatti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3c4b96f3-ca6c-c792-7a78-b44e4a7cef12@gmail.com \
--to=hejianet@gmail.com \
--cc=bing.zhao@hxt-semitech.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=ilovethull@163.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=jia.he@hxt-semitech.com \
--cc=jianbo.liu@arm.com \
--cc=jie2.liu@hxt-semitech.com \
--cc=konstantin.ananyev@intel.com \
--cc=olivier.matz@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).