From: Jianbo Liu <jianbo.liu@arm.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Cc: Jia He <hejianet@gmail.com>,
dev@dpdk.org, olivier.matz@6wind.com,
konstantin.ananyev@intel.com, bruce.richardson@intel.com,
hemant.agrawal@nxp.com, Jia He <jia.he@hxt-semitech.com>,
jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com
Subject: Re: [dpdk-dev] [PATCH v5 1/1] ring: guarantee load/load order in enqueue and dequeue
Date: Fri, 10 Nov 2017 11:12:58 +0800 [thread overview]
Message-ID: <20171110031257.GD27104@arm.com> (raw)
In-Reply-To: <20171110024609.GA5295@jerin>
The 11/10/2017 08:16, Jerin Jacob wrote:
> -----Original Message-----
> > Date: Fri, 10 Nov 2017 01:51:09 +0000
> > From: Jia He <hejianet@gmail.com>
> > To: jerin.jacob@caviumnetworks.com, dev@dpdk.org, olivier.matz@6wind.com
> > Cc: konstantin.ananyev@intel.com, bruce.richardson@intel.com,
> > jianbo.liu@arm.com, hemant.agrawal@nxp.com, Jia He <hejianet@gmail.com>,
> > Jia He <jia.he@hxt-semitech.com>, jie2.liu@hxt-semitech.com,
> > bing.zhao@hxt-semitech.com
> > Subject: [PATCH v5 1/1] ring: guarantee load/load order in enqueue and
> > dequeue
> > X-Mailer: git-send-email 2.7.4
> >
> > We watched a rte panic of mbuf_autotest in our qualcomm arm64 server.
>
> I think, the above text can be improved. Something like below.
>
>
> Fix for the rte_panic() in mbuf_autotest on qualcomm arm64 server(...SoC
> name...)
>
> Root cause:
> > In __rte_ring_move_cons_head()
> > ...
> > do {
> > /* Restore n as it may change every loop */
> > n = max;
> >
> > *old_head = r->cons.head; //1st load
> > const uint32_t prod_tail = r->prod.tail; //2nd load
> >
> > cpu1(producer) cpu2(consumer) cpu3(consumer)
> > load r->prod.tail
> > in enqueue:
> > load r->cons.tail
> > load r->prod.head
> >
> > store r->prod.tail
> >
> > load r->cons.head
> > load r->prod.tail
> > ...
> > store r->cons.{head,tail}
> > load r->cons.head
> >
> > In weak memory order architectures(powerpc,arm), the 2nd load might be
> > reodered before the 1st load, that makes *entries is bigger than we
> > wanted. This nasty reording messed enque/deque up. Then, r->cons.head
> > will be bigger than prod_tail, then make *entries very big and the
> > consumer will go forward incorrectly.
> >
> > After this patch, even with above context switches, the old cons.head
> > will be recaculated after failure of rte_atomic32_cmpset. So no race
> > conditions left.
> >
> > There is no such issue on X86, because X86 is strong memory order model.
> > But rte_smp_rmb() doesn't have impact on runtime performance on X86, so
> > keep the same code without architectures specific concerns.
> >
> > Signed-off-by: Jia He <jia.he@hxt-semitech.com>
> > Signed-off-by: jie2.liu@hxt-semitech.com
> > Signed-off-by: bing.zhao@hxt-semitech.com
>
> ➜ [master][dpdk.org] $ ./devtools/checkpatches.sh
>
> ### ring: guarantee load/load order in enqueue and dequeue
>
> WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a
> separate line
> #58: FILE: lib/librte_ring/rte_ring.h:414:
> + * memory model. It is noop on x86 */
>
> WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a
> separate line
> #70: FILE: lib/librte_ring/rte_ring.h:527:
> + * memory model. It is noop on x86 */
>
> total: 0 errors, 2 warnings, 22 lines checked
>
> 0/1 valid patch
> ➜ [master][dpdk.org] $ ./devtools/check-git-log.sh
> Wrong tag:
> Signed-off-by: jie2.liu@hxt-semitech.com
> Signed-off-by: bing.zhao@hxt-semitech.com
>
>
> With above fixes:
> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>
>
>
Acked-by: Jianbo Liu <jianbo.liu@arm.com>
And add it to stable.
Cc: stable@dpdk.org
--
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
next prev parent reply other threads:[~2017-11-10 3:14 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1510118764-29697-1-git-send-email-hejianet@gmail.com>
2017-11-08 9:54 ` [dpdk-dev] [PATCH v4 0/4] fix race condition in enqueue/dequeue because of cpu reorder Jia He
2017-11-08 9:54 ` [dpdk-dev] [PATCH v4 1/4] eal/arm64: remove the braces {} for dmb() and dsb() Jia He
2017-11-08 9:54 ` [dpdk-dev] [PATCH v4 2/4] ring: guarantee load/load order in enqueue and dequeue Jia He
2017-11-08 9:54 ` [dpdk-dev] [PATCH v4 3/4] ring: introduce new header file to include common functions Jia He
2017-11-08 9:54 ` [dpdk-dev] [PATCH v4 4/4] ring: introduce new header file to support C11 memory model Jia He
2017-11-08 12:15 ` [dpdk-dev] [PATCH v4 0/4] fix race condition in enqueue/dequeue because of cpu reorder Bruce Richardson
2017-11-08 15:11 ` Jia He
2017-11-08 16:29 ` Jerin Jacob
2017-11-08 18:36 ` Ananyev, Konstantin
[not found] ` <2459a535-920e-9ac5-2f46-1d1dd00e275b@gmail.com>
2017-11-24 9:24 ` Bruce Richardson
2017-11-10 1:51 ` [dpdk-dev] [PATCH v5 0/1] " Jia He
2017-11-10 1:51 ` [dpdk-dev] [PATCH v5 1/1] ring: guarantee load/load order in enqueue and dequeue Jia He
2017-11-10 2:46 ` Jerin Jacob
2017-11-10 3:12 ` Jianbo Liu [this message]
2017-11-10 9:59 ` Ananyev, Konstantin
2017-11-10 3:30 ` [dpdk-dev] [PATCH v6] " Jia He
2017-11-10 3:30 ` [dpdk-dev] [PATCH v6] ring: " Jia He
2017-11-12 17:51 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2017-11-10 5:23 ` [dpdk-dev] [PATCH v5 0/3] support c11 memory model barrier in librte_ring Jia He
2017-11-10 5:23 ` [dpdk-dev] [PATCH v5 1/3] eal/arm64: remove the braces {} for dmb() and dsb() Jia He
2017-11-10 5:23 ` [dpdk-dev] [PATCH v5 2/3] ring: introduce new header file to include common functions Jia He
2017-11-10 5:23 ` [dpdk-dev] [PATCH v6 3/3] ring: introduce new header file to support C11 memory model Jia He
2017-11-27 2:00 ` [dpdk-dev] [PATCH V6 0/3] support c11 memory model barrier in librte_ring Jia He
2017-11-27 2:00 ` [dpdk-dev] [PATCH V6 1/3] eal/arm64: remove the braces {} for dmb() and dsb() Jia He
2017-12-03 11:11 ` Jerin Jacob
2017-11-27 2:00 ` [dpdk-dev] [PATCH V6 2/3] ring: introduce new header file to include common functions Jia He
2017-12-03 12:13 ` Jerin Jacob
2017-11-27 2:00 ` [dpdk-dev] [PATCH V6 3/3] ring: introduce new header file to support C11 memory model Jia He
2017-12-03 12:14 ` Jerin Jacob
2017-12-04 1:50 ` [dpdk-dev] [PATCH V7 0/3] support c11 memory model barrier in librte_ring Jia He
2017-12-04 1:50 ` [dpdk-dev] [PATCH V7 1/3] eal/arm64: remove the braces {} for dmb() and dsb() Jia He
2017-12-04 1:50 ` [dpdk-dev] [PATCH V7 2/3] ring: introduce new header file to include common functions Jia He
2018-01-12 17:09 ` Thomas Monjalon
2018-01-16 2:06 ` Jia He
2018-01-16 15:19 ` Olivier Matz
2017-12-04 1:50 ` [dpdk-dev] [PATCH V7 3/3] ring: introduce new header file to support C11 memory model Jia He
2017-12-04 8:05 ` Jianbo Liu
2018-01-12 17:18 ` Thomas Monjalon
2018-01-16 15:18 ` Olivier Matz
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=20171110031257.GD27104@arm.com \
--to=jianbo.liu@arm.com \
--cc=bing.zhao@hxt-semitech.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=hejianet@gmail.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=jia.he@hxt-semitech.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).