| Message ID | 20191204151916.12607-1-xiaoyun.li@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | raw/ntb: fix write memory barrier issue | expand |
| Context | Check | Description |
|---|---|---|
| ci/travis-robot | warning | Travis build: failed |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/iol-compilation | success | Compile Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/checkpatch | success | coding style OK |
Hi Xiaoyun, > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Xiaoyun Li > Sent: Wednesday, December 4, 2019 11:19 PM > To: jingjing.wu@intel.com > Cc: dev@dpdk.org; Xiaoyun Li <xiaoyun.li@intel.com>; stable@dpdk.org > Subject: [dpdk-dev] [PATCH] 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> > --- > 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..dd0b72f8c 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); > - *txq->used_cnt = txq->last_used; > rte_wmb(); > + *txq->used_cnt = txq->last_used; I am ok with the re-location of the barrier, but why not the rte_io_wmb instead of rte_wmb? Rte_io_wmb is sufficient to guarantee the preceding stores are visible to the device, rte_wmb is overkill. https://code.dpdk.org/dpdk/latest/source/lib/librte_eal/common/include/generic/rte_atomic.h#L92 > > /* 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); > - *rxq->avail_cnt = rxq->last_avail; > rte_wmb(); > + *rxq->avail_cnt = rxq->last_avail; > > /* update queue stats */ > off = NTB_XSTATS_NUM * ((size_t)context + 1); > -- > 2.17.1
Didn't notice that. Will fix it in v2. Thanks! > -----Original Message----- > From: Gavin Hu (Arm Technology China) [mailto:Gavin.Hu@arm.com] > Sent: Saturday, December 14, 2019 23:30 > To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing <jingjing.wu@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org; nd <nd@arm.com> > Subject: RE: [dpdk-dev] [PATCH] raw/ntb: fix write memory barrier issue > > Hi Xiaoyun, > > > -----Original Message----- > > From: dev <dev-bounces@dpdk.org> On Behalf Of Xiaoyun Li > > Sent: Wednesday, December 4, 2019 11:19 PM > > To: jingjing.wu@intel.com > > Cc: dev@dpdk.org; Xiaoyun Li <xiaoyun.li@intel.com>; stable@dpdk.org > > Subject: [dpdk-dev] [PATCH] 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> > > --- > > 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..dd0b72f8c 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); > > - *txq->used_cnt = txq->last_used; > > rte_wmb(); > > + *txq->used_cnt = txq->last_used; > I am ok with the re-location of the barrier, but why not the rte_io_wmb instead > of rte_wmb? > Rte_io_wmb is sufficient to guarantee the preceding stores are visible to the > device, rte_wmb is overkill. > https://code.dpdk.org/dpdk/latest/source/lib/librte_eal/common/include/gener > ic/rte_atomic.h#L92 > > > > /* 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); > > - *rxq->avail_cnt = rxq->last_avail; > > rte_wmb(); > > + *rxq->avail_cnt = rxq->last_avail; > > > > /* update queue stats */ > > off = NTB_XSTATS_NUM * ((size_t)context + 1); > > -- > > 2.17.1
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
> -----Original Message----- > From: Li, Xiaoyun <xiaoyun.li@intel.com> > Sent: Wednesday, December 4, 2019 11:19 PM > To: Wu, Jingjing <jingjing.wu@intel.com> > Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org > Subject: [PATCH] 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> Acked-by: Jingjing Wu <jingjing.wu@intel.com>
26/12/2019 02:46, Wu, Jingjing: > From: Li, Xiaoyun <xiaoyun.li@intel.com> > > 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> > Acked-by: Jingjing Wu <jingjing.wu@intel.com> v1 applied, thanks
diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index ad7f6abfd..dd0b72f8c 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); - *txq->used_cnt = txq->last_used; rte_wmb(); + *txq->used_cnt = txq->last_used; /* 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); - *rxq->avail_cnt = rxq->last_avail; rte_wmb(); + *rxq->avail_cnt = rxq->last_avail; /* update queue stats */ off = NTB_XSTATS_NUM * ((size_t)context + 1);
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> --- drivers/raw/ntb/ntb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)