| Message ID | 1458303833-14815-1-git-send-email-i.maximets@samsung.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Thomas Monjalon |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id D905E5962; Fri, 18 Mar 2016 13:24:01 +0100 (CET) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id D96E65949 for <dev@dpdk.org>; Fri, 18 Mar 2016 13:24:00 +0100 (CET) Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0O4800IRDIFY3E70@mailout2.w1.samsung.com> for dev@dpdk.org; Fri, 18 Mar 2016 12:23:58 +0000 (GMT) X-AuditID: cbfec7f5-f79b16d000005389-73-56ebf35e6d7b Received: from eusync2.samsung.com ( [203.254.199.212]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id E7.A0.21385.E53FBE65; Fri, 18 Mar 2016 12:23:58 +0000 (GMT) Received: from imaximets.rnd.samsung.ru ([106.109.129.180]) by eusync2.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0O4800CPRIFU1B30@eusync2.samsung.com>; Fri, 18 Mar 2016 12:23:58 +0000 (GMT) From: Ilya Maximets <i.maximets@samsung.com> To: dev@dpdk.org, Huawei Xie <huawei.xie@intel.com> Cc: Dyasly Sergey <s.dyasly@samsung.com>, Yuanhan Liu <yuanhan.liu@linux.intel.com>, Ilya Maximets <i.maximets@samsung.com> Date: Fri, 18 Mar 2016 15:23:53 +0300 Message-id: <1458303833-14815-1-git-send-email-i.maximets@samsung.com> X-Mailer: git-send-email 2.5.0 In-reply-to: <1456314438-4021-2-git-send-email-i.maximets@samsung.com> References: <1456314438-4021-2-git-send-email-i.maximets@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpkluLIzCtJLcpLzFFi42I5/e/4Fd24z6/DDO59U7B492k7k0X7zLNM Flfaf7JbTJ4tZXF9wgVWB1aPXwuWsnos3vOSyWPeyUCPvi2rGANYorhsUlJzMstSi/TtErgy jm8UKTjEXXFz4zr2BsYDnF2MnBwSAiYSjZ3r2CBsMYkL99YD2VwcQgJLGSVub7nABOG0Mkm8 vPsdrIpNQEfi1OojjCC2iICxxLSDO4BsDg5mgXqJ9VsyQMLCAq4S2+6vBCtnEVCV2LrlHFg5 r4CbRPe+2Swg5RICchILLqSDhDmBwgfed7GA2EJArbf+nmWawMi7gJFhFaNoamlyQXFSeq6R XnFibnFpXrpecn7uJkZIwHzdwbj0mNUhRgEORiUe3hdVr8OEWBPLiitzDzFKcDArifBKfAIK 8aYkVlalFuXHF5XmpBYfYpTmYFES5525632IkEB6YklqdmpqQWoRTJaJg1OqgfEsj0dj6xsz /iuPlhm5x318bbLxgvK05LA/3eJMbJ3fJm94v0zBXk6fzXBD1zeOtO3RdyMTmlguXWFT3eyy OHhHDOOfDQ4v9aXaIjeqnHs5q3hJqUmQ76TcVxnsH3d93egV2ZeTvZ/v93sbnj6BNzJBSUfF xb+aTGF5ZXpvGZ90mNiVW0V3HyixFGckGmoxFxUnAgA6wwHRFAIAAA== Subject: [dpdk-dev] [PATCH v4] vhost: use SMP barriers instead of compiler ones. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Ilya Maximets
March 18, 2016, 12:23 p.m. UTC
Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio
uses architecture dependent SMP barriers. vHost should use them too.
Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
lib/librte_vhost/vhost_rxtx.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
Comments
On Fri, Mar 18, 2016 at 03:23:53PM +0300, Ilya Maximets wrote: > Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio > uses architecture dependent SMP barriers. vHost should use them too. > > Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers") > > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > --- > lib/librte_vhost/vhost_rxtx.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c > index b4da665..859c669 100644 > --- a/lib/librte_vhost/vhost_rxtx.c > +++ b/lib/librte_vhost/vhost_rxtx.c > @@ -315,7 +315,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, > rte_prefetch0(&vq->desc[desc_indexes[i+1]]); > } > > - rte_compiler_barrier(); > + rte_smp_wmb(); > > /* Wait until it's our turn to add our buffer to the used ring. */ > while (unlikely(vq->last_used_idx != res_start_idx)) > @@ -565,7 +565,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, > > nr_used = copy_mbuf_to_desc_mergeable(dev, vq, start, end, > pkts[pkt_idx]); > - rte_compiler_barrier(); > + rte_smp_wmb(); > > /* > * Wait until it's our turn to add our buffer > @@ -923,7 +923,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, > sizeof(vq->used->ring[used_idx])); > } > > - rte_compiler_barrier(); > + rte_smp_wmb(); > + rte_smp_rmb(); rte_smp_mb? --yliu
On 18.03.2016 15:41, Yuanhan Liu wrote: > On Fri, Mar 18, 2016 at 03:23:53PM +0300, Ilya Maximets wrote: >> Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio >> uses architecture dependent SMP barriers. vHost should use them too. >> >> Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers") >> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> >> --- >> lib/librte_vhost/vhost_rxtx.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c >> index b4da665..859c669 100644 >> --- a/lib/librte_vhost/vhost_rxtx.c >> +++ b/lib/librte_vhost/vhost_rxtx.c >> @@ -315,7 +315,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, >> rte_prefetch0(&vq->desc[desc_indexes[i+1]]); >> } >> >> - rte_compiler_barrier(); >> + rte_smp_wmb(); >> >> /* Wait until it's our turn to add our buffer to the used ring. */ >> while (unlikely(vq->last_used_idx != res_start_idx)) >> @@ -565,7 +565,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, >> >> nr_used = copy_mbuf_to_desc_mergeable(dev, vq, start, end, >> pkts[pkt_idx]); >> - rte_compiler_barrier(); >> + rte_smp_wmb(); >> >> /* >> * Wait until it's our turn to add our buffer >> @@ -923,7 +923,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, >> sizeof(vq->used->ring[used_idx])); >> } >> >> - rte_compiler_barrier(); >> + rte_smp_wmb(); >> + rte_smp_rmb(); > > rte_smp_mb? rte_smp_mb() is a real mm_fence() on x86. And we don't need to synchronize reads with writes here, only reads with reads and writes with writes. It is enough because next increment uses read and write. Pair of barriers is better because it will not impact on performance on x86. Best regards, Ilya Maximets.
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ilya Maximets > Sent: Monday, March 21, 2016 4:50 AM > To: Yuanhan Liu > Cc: dev@dpdk.org; Xie, Huawei; Dyasly Sergey > Subject: Re: [dpdk-dev] [PATCH v4] vhost: use SMP barriers instead of compiler ones. > > > > On 18.03.2016 15:41, Yuanhan Liu wrote: > > On Fri, Mar 18, 2016 at 03:23:53PM +0300, Ilya Maximets wrote: > >> Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio > >> uses architecture dependent SMP barriers. vHost should use them too. > >> > >> Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers") > >> > >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > >> --- > >> lib/librte_vhost/vhost_rxtx.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c > >> index b4da665..859c669 100644 > >> --- a/lib/librte_vhost/vhost_rxtx.c > >> +++ b/lib/librte_vhost/vhost_rxtx.c > >> @@ -315,7 +315,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, > >> rte_prefetch0(&vq->desc[desc_indexes[i+1]]); > >> } > >> > >> - rte_compiler_barrier(); > >> + rte_smp_wmb(); > >> > >> /* Wait until it's our turn to add our buffer to the used ring. */ > >> while (unlikely(vq->last_used_idx != res_start_idx)) > >> @@ -565,7 +565,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, > >> > >> nr_used = copy_mbuf_to_desc_mergeable(dev, vq, start, end, > >> pkts[pkt_idx]); > >> - rte_compiler_barrier(); > >> + rte_smp_wmb(); > >> > >> /* > >> * Wait until it's our turn to add our buffer > >> @@ -923,7 +923,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, > >> sizeof(vq->used->ring[used_idx])); > >> } > >> > >> - rte_compiler_barrier(); > >> + rte_smp_wmb(); > >> + rte_smp_rmb(); > > > > rte_smp_mb? > > rte_smp_mb() is a real mm_fence() on x86. And we don't need to synchronize reads with > writes here, only reads with reads and writes with writes. It is enough because next > increment uses read and write. Pair of barriers is better because it will not impact > on performance on x86. Not arguing about that particular patch, just a question: Why do we have: #define rte_smp_mb() rte_mb() for x86? Why not just: #define rte_smp_mb() rte_compiler_barrier() here? I meant for situations when we do need real mfence, there is an 'rte_mb' to use. Konstantin > > Best regards, Ilya Maximets.
On 3/21/2016 10:07 PM, Ananyev, Konstantin wrote: > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ilya Maximets >> Sent: Monday, March 21, 2016 4:50 AM >> To: Yuanhan Liu >> Cc: dev@dpdk.org; Xie, Huawei; Dyasly Sergey >> Subject: Re: [dpdk-dev] [PATCH v4] vhost: use SMP barriers instead of compiler ones. >> >> >> >> On 18.03.2016 15:41, Yuanhan Liu wrote: >>> On Fri, Mar 18, 2016 at 03:23:53PM +0300, Ilya Maximets wrote: >>>> Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio >>>> uses architecture dependent SMP barriers. vHost should use them too. >>>> >>>> Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers") >>>> >>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> >>>> --- >>>> lib/librte_vhost/vhost_rxtx.c | 7 ++++--- >>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c >>>> index b4da665..859c669 100644 >>>> --- a/lib/librte_vhost/vhost_rxtx.c >>>> +++ b/lib/librte_vhost/vhost_rxtx.c >>>> @@ -315,7 +315,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, >>>> rte_prefetch0(&vq->desc[desc_indexes[i+1]]); >>>> } >>>> >>>> - rte_compiler_barrier(); >>>> + rte_smp_wmb(); >>>> >>>> /* Wait until it's our turn to add our buffer to the used ring. */ >>>> while (unlikely(vq->last_used_idx != res_start_idx)) >>>> @@ -565,7 +565,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, >>>> >>>> nr_used = copy_mbuf_to_desc_mergeable(dev, vq, start, end, >>>> pkts[pkt_idx]); >>>> - rte_compiler_barrier(); >>>> + rte_smp_wmb(); >>>> >>>> /* >>>> * Wait until it's our turn to add our buffer >>>> @@ -923,7 +923,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, >>>> sizeof(vq->used->ring[used_idx])); >>>> } >>>> >>>> - rte_compiler_barrier(); >>>> + rte_smp_wmb(); >>>> + rte_smp_rmb(); >>> rte_smp_mb? >> rte_smp_mb() is a real mm_fence() on x86. And we don't need to synchronize reads with >> writes here, only reads with reads and writes with writes. It is enough because next >> increment uses read and write. Pair of barriers is better because it will not impact >> on performance on x86. > Not arguing about that particular patch, just a question: > Why do we have: > #define rte_smp_mb() rte_mb() Konstantine, actually smp_mb is defined as mfence while smp_r/wmb are defined as barrier in kernel_src/arch/x86/include/asm/barrier.h. > for x86? > Why not just: > #define rte_smp_mb() rte_compiler_barrier() > here? > I meant for situations when we do need real mfence, there is an 'rte_mb' to use. > Konstantin > >> Best regards, Ilya Maximets.
> -----Original Message----- > From: Xie, Huawei > Sent: Monday, March 21, 2016 5:26 PM > To: Ananyev, Konstantin; Ilya Maximets; Yuanhan Liu > Cc: dev@dpdk.org; Dyasly Sergey > Subject: Re: [dpdk-dev] [PATCH v4] vhost: use SMP barriers instead of compiler ones. > > On 3/21/2016 10:07 PM, Ananyev, Konstantin wrote: > > > >> -----Original Message----- > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ilya Maximets > >> Sent: Monday, March 21, 2016 4:50 AM > >> To: Yuanhan Liu > >> Cc: dev@dpdk.org; Xie, Huawei; Dyasly Sergey > >> Subject: Re: [dpdk-dev] [PATCH v4] vhost: use SMP barriers instead of compiler ones. > >> > >> > >> > >> On 18.03.2016 15:41, Yuanhan Liu wrote: > >>> On Fri, Mar 18, 2016 at 03:23:53PM +0300, Ilya Maximets wrote: > >>>> Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio > >>>> uses architecture dependent SMP barriers. vHost should use them too. > >>>> > >>>> Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers") > >>>> > >>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > >>>> --- > >>>> lib/librte_vhost/vhost_rxtx.c | 7 ++++--- > >>>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c > >>>> index b4da665..859c669 100644 > >>>> --- a/lib/librte_vhost/vhost_rxtx.c > >>>> +++ b/lib/librte_vhost/vhost_rxtx.c > >>>> @@ -315,7 +315,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, > >>>> rte_prefetch0(&vq->desc[desc_indexes[i+1]]); > >>>> } > >>>> > >>>> - rte_compiler_barrier(); > >>>> + rte_smp_wmb(); > >>>> > >>>> /* Wait until it's our turn to add our buffer to the used ring. */ > >>>> while (unlikely(vq->last_used_idx != res_start_idx)) > >>>> @@ -565,7 +565,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, > >>>> > >>>> nr_used = copy_mbuf_to_desc_mergeable(dev, vq, start, end, > >>>> pkts[pkt_idx]); > >>>> - rte_compiler_barrier(); > >>>> + rte_smp_wmb(); > >>>> > >>>> /* > >>>> * Wait until it's our turn to add our buffer > >>>> @@ -923,7 +923,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, > >>>> sizeof(vq->used->ring[used_idx])); > >>>> } > >>>> > >>>> - rte_compiler_barrier(); > >>>> + rte_smp_wmb(); > >>>> + rte_smp_rmb(); > >>> rte_smp_mb? > >> rte_smp_mb() is a real mm_fence() on x86. And we don't need to synchronize reads with > >> writes here, only reads with reads and writes with writes. It is enough because next > >> increment uses read and write. Pair of barriers is better because it will not impact > >> on performance on x86. > > Not arguing about that particular patch, just a question: > > Why do we have: > > #define rte_smp_mb() rte_mb() > > Konstantine, actually smp_mb is defined as mfence while smp_r/wmb are > defined as barrier in kernel_src/arch/x86/include/asm/barrier.h. I am aware of that, but who said that we should do the same? Konstantin > > > for x86? > > Why not just: > > #define rte_smp_mb() rte_compiler_barrier() > > here? > > I meant for situations when we do need real mfence, there is an 'rte_mb' to use. > > Konstantin > > > >> Best regards, Ilya Maximets.
On 3/18/2016 8:24 PM, Ilya Maximets wrote: > Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio > uses architecture dependent SMP barriers. vHost should use them too. > > Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers") > > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Huawei Xie <huawei.xie@intel.com> It fixes the issue for other archs. Will recheck in future.
2016-03-23 14:07, Xie, Huawei: > On 3/18/2016 8:24 PM, Ilya Maximets wrote: > > Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio > > uses architecture dependent SMP barriers. vHost should use them too. > > > > Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers") > > > > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > > Acked-by: Huawei Xie <huawei.xie@intel.com> > > It fixes the issue for other archs. Will recheck in future. Applied, thanks
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c index b4da665..859c669 100644 --- a/lib/librte_vhost/vhost_rxtx.c +++ b/lib/librte_vhost/vhost_rxtx.c @@ -315,7 +315,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, rte_prefetch0(&vq->desc[desc_indexes[i+1]]); } - rte_compiler_barrier(); + rte_smp_wmb(); /* Wait until it's our turn to add our buffer to the used ring. */ while (unlikely(vq->last_used_idx != res_start_idx)) @@ -565,7 +565,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, nr_used = copy_mbuf_to_desc_mergeable(dev, vq, start, end, pkts[pkt_idx]); - rte_compiler_barrier(); + rte_smp_wmb(); /* * Wait until it's our turn to add our buffer @@ -923,7 +923,8 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id, sizeof(vq->used->ring[used_idx])); } - rte_compiler_barrier(); + rte_smp_wmb(); + rte_smp_rmb(); vq->used->idx += i; vhost_log_used_vring(dev, vq, offsetof(struct vring_used, idx), sizeof(vq->used->idx));