patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Gavin Hu <Gavin.Hu@arm.com>
To: "Li, Xiaoyun" <xiaoyun.li@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Maslekar, Omkar" <omkar.maslekar@intel.com>,
	"stable@dpdk.org" <stable@dpdk.org>, nd <nd@arm.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
	nd <nd@arm.com>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v2] raw/ntb: fix write memory barrier issue
Date: Mon, 23 Dec 2019 08:38:08 +0000
Message-ID: <VI1PR08MB537694B24564C8E262BCC1328F2E0@VI1PR08MB5376.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <B9E724F4CB7543449049E7AE7669D82F0B35B9BC@SHSMSX101.ccr.corp.intel.com>

Hi Xiaoyun,

> -----Original Message-----
> From: Li, Xiaoyun <xiaoyun.li@intel.com>
> Sent: Monday, December 23, 2019 3:52 PM
> To: Gavin Hu <Gavin.Hu@arm.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Maslekar, Omkar <omkar.maslekar@intel.com>;
> stable@dpdk.org; nd <nd@arm.com>
> Subject: RE: [dpdk-dev] [PATCH v2] raw/ntb: fix write memory barrier issue
> 
> Hi
> I reconsidered and retested about this issue.
> I still need to use rte_wmb instead of using rte_io_wmb.
> 
> Because to achieve high performance, ntb needs to turn on WC(write
> combining) feature. The perf difference with and without WC enabled is
> more than 20X.
> And when WC enabled, rte_io_wmb cannot make sure the instructions are
> in order only rte_wmb can make sure that.
> 
> And in my retest, when sending 64 bytes packets, using rte_io_wmb will
> cause out-of-order issue and cause memory corruption on rx side.
> And using rte_wmb is fine.
That's true, as it is declared as 'write combine' region, even x86 is known as strong ordered, it is the interconnect or PCI RC may do the reordering, 'write combine', 'write coalescing', which caused this problem.
IMO, rte_io_*mb barriers on x86 should be promoted to stronger is WC is involved(but that will sap performance for non-WC memories?). 
https://code.dpdk.org/dpdk/latest/source/lib/librte_eal/common/include/arch/x86/rte_atomic.h#L78 

Using rte_wmb will hurt performance for aarch64 also, as pci device memory accesses to a single device are strongly ordered therefore the strongest rte_wmb is not necessary.  
> So I can only use v1 patch and suspend v2 patch in patchwork.
> 
> Best Regards
> Xiaoyun Li
> 
> > -----Original Message-----
> > From: Gavin Hu (Arm Technology China) [mailto:Gavin.Hu@arm.com]
> > Sent: Monday, December 16, 2019 18:50
> > To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; Maslekar, Omkar <omkar.maslekar@intel.com>;
> > stable@dpdk.org; nd <nd@arm.com>
> > Subject: RE: [dpdk-dev] [PATCH v2] raw/ntb: fix write memory barrier
> issue
> >
> >
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Xiaoyun Li
> > > Sent: Monday, December 16, 2019 9:59 AM
> > > To: jingjing.wu@intel.com
> > > Cc: dev@dpdk.org; omkar.maslekar@intel.com; Xiaoyun Li
> > > <xiaoyun.li@intel.com>; stable@dpdk.org
> > > Subject: [dpdk-dev] [PATCH v2] raw/ntb: fix write memory barrier issue
> > >
> > > All buffers and ring info should be written before tail register update.
> > > This patch relocates the write memory barrier before updating tail
> > > register to avoid potential issues.
> > >
> > > Fixes: 11b5c7daf019 ("raw/ntb: add enqueue and dequeue functions")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > > ---
> > > v2:
> > >  * Replaced rte_wmb with rte_io_wmb since rte_io_wmb is enough.
> > > ---
> > >  drivers/raw/ntb/ntb.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index
> > > ad7f6abfd..c7de86f36 100644
> > > --- a/drivers/raw/ntb/ntb.c
> > > +++ b/drivers/raw/ntb/ntb.c
> > > @@ -683,8 +683,8 @@ ntb_enqueue_bufs(struct rte_rawdev *dev,
> > >  			   sizeof(struct ntb_used) * nb1);
> > >  		rte_memcpy(txq->tx_used_ring, tx_used + nb1,
> > >  			   sizeof(struct ntb_used) * nb2);
> > > +		rte_io_wmb();
> > As both txq->tx_used_ring and *txq->used_cnt are physically reside in the
> PCI
> > device side, rte_io_wmb is correct to ensure the ordering.
> >
> > >  		*txq->used_cnt = txq->last_used;
> > > -		rte_wmb();
> > >
> > >  		/* update queue stats */
> > >  		hw->ntb_xstats[NTB_TX_BYTES_ID + off] += bytes; @@ -
> 789,8
> > +789,8 @@
> > > ntb_dequeue_bufs(struct rte_rawdev *dev,
> > >  			   sizeof(struct ntb_desc) * nb1);
> > >  		rte_memcpy(rxq->rx_desc_ring, rx_desc + nb1,
> > >  			   sizeof(struct ntb_desc) * nb2);
> > > +		rte_io_wmb();
> > >  		*rxq->avail_cnt = rxq->last_avail;
> > > -		rte_wmb();
> > >
> > >  		/* update queue stats */
> > >  		off = NTB_XSTATS_NUM * ((size_t)context + 1);
> > > --
> > > 2.17.1
> >
> > Reviewed-by: Gavin Hu <gavin.hu@arm.com>


  reply	other threads:[~2019-12-23  8:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-04 15:19 [dpdk-stable] [PATCH] " Xiaoyun Li
2019-12-14 15:29 ` [dpdk-stable] [dpdk-dev] " Gavin Hu (Arm Technology China)
2019-12-16  1:57   ` Li, Xiaoyun
2019-12-25  3:52     ` Gavin Hu
2019-12-16  1:58 ` [dpdk-stable] [PATCH v2] " Xiaoyun Li
2019-12-16 10:49   ` [dpdk-stable] [dpdk-dev] " Gavin Hu (Arm Technology China)
2019-12-23  7:52     ` Li, Xiaoyun
2019-12-23  8:38       ` Gavin Hu [this message]
2019-12-23  9:35         ` Li, Xiaoyun
2019-12-24 15:46           ` Gavin Hu
2019-12-23  2:38   ` [dpdk-stable] " Wu, Jingjing
2019-12-26  1:46 ` [dpdk-stable] [PATCH] " Wu, Jingjing
2020-01-20  9:04   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon

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=VI1PR08MB537694B24564C8E262BCC1328F2E0@VI1PR08MB5376.eurprd08.prod.outlook.com \
    --to=gavin.hu@arm.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=jingjing.wu@intel.com \
    --cc=nd@arm.com \
    --cc=omkar.maslekar@intel.com \
    --cc=stable@dpdk.org \
    --cc=xiaoyun.li@intel.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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git