* [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags @ 2015-04-22 16:33 Huawei Xie 2015-04-23 13:57 ` Luke Gorrie ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Huawei Xie @ 2015-04-22 16:33 UTC (permalink / raw) To: dev; +Cc: mst update of used->idx and read of avail->flags could be reordered. memory fence should be used to ensure the order, otherwise guest could see a stale used->idx value after it toggles the interrupt suppression flag. Signed-off-by: Huawei Xie <huawei.xie@intel.com> --- lib/librte_vhost/vhost_rxtx.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c index 510ffe8..6afba35 100644 --- a/lib/librte_vhost/vhost_rxtx.c +++ b/lib/librte_vhost/vhost_rxtx.c @@ -178,6 +178,9 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, *(volatile uint16_t *)&vq->used->idx += count; vq->last_used_idx = res_end_idx; + /* flush used->idx update before we read avail->flags. */ + rte_mb(); + /* Kick the guest if necessary. */ if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) eventfd_write((int)vq->callfd, 1); -- 1.8.1.4 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-04-22 16:33 [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags Huawei Xie @ 2015-04-23 13:57 ` Luke Gorrie 2015-04-24 1:01 ` Linhaifeng 2015-04-29 11:11 ` [dpdk-dev] [PATCH v2] " Huawei Xie 2 siblings, 0 replies; 17+ messages in thread From: Luke Gorrie @ 2015-04-23 13:57 UTC (permalink / raw) To: Huawei Xie; +Cc: dev, Michael S. Tsirkin On 22 April 2015 at 18:33, Huawei Xie <huawei.xie@intel.com> wrote: > update of used->idx and read of avail->flags could be reordered. > memory fence should be used to ensure the order, otherwise guest could see > a stale used->idx value after it toggles the interrupt suppression flag. > This patch looks right to me. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-04-22 16:33 [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags Huawei Xie 2015-04-23 13:57 ` Luke Gorrie @ 2015-04-24 1:01 ` Linhaifeng 2015-04-24 2:40 ` Xie, Huawei 2015-04-24 7:27 ` Luke Gorrie 2015-04-29 11:11 ` [dpdk-dev] [PATCH v2] " Huawei Xie 2 siblings, 2 replies; 17+ messages in thread From: Linhaifeng @ 2015-04-24 1:01 UTC (permalink / raw) To: Huawei Xie, dev; +Cc: mst On 2015/4/23 0:33, Huawei Xie wrote: > update of used->idx and read of avail->flags could be reordered. > memory fence should be used to ensure the order, otherwise guest could see a stale used->idx value after it toggles the interrupt suppression flag. > > Signed-off-by: Huawei Xie <huawei.xie@intel.com> > --- > lib/librte_vhost/vhost_rxtx.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c > index 510ffe8..6afba35 100644 > --- a/lib/librte_vhost/vhost_rxtx.c > +++ b/lib/librte_vhost/vhost_rxtx.c > @@ -178,6 +178,9 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, > *(volatile uint16_t *)&vq->used->idx += count; > vq->last_used_idx = res_end_idx; > > + /* flush used->idx update before we read avail->flags. */ > + rte_mb(); > + > /* Kick the guest if necessary. */ > if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) > eventfd_write((int)vq->callfd, 1); > If not add memory fence what would happen? Packets loss or interrupt loss?How to test it ? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-04-24 1:01 ` Linhaifeng @ 2015-04-24 2:40 ` Xie, Huawei 2015-04-24 7:27 ` Luke Gorrie 1 sibling, 0 replies; 17+ messages in thread From: Xie, Huawei @ 2015-04-24 2:40 UTC (permalink / raw) To: Linhaifeng, dev; +Cc: mst > -----Original Message----- > From: Linhaifeng [mailto:haifeng.lin@huawei.com] > Sent: Friday, April 24, 2015 9:01 AM > To: Xie, Huawei; dev@dpdk.org > Cc: luke@snabb.co; mst@redhat.com > Subject: Re: [PATCH] vhost: flush used->idx update before reading avail- > >flags > > > > On 2015/4/23 0:33, Huawei Xie wrote: > > update of used->idx and read of avail->flags could be reordered. > > memory fence should be used to ensure the order, otherwise guest could > see a stale used->idx value after it toggles the interrupt suppression flag. > > > > Signed-off-by: Huawei Xie <huawei.xie@intel.com> > > --- > > lib/librte_vhost/vhost_rxtx.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c > > index 510ffe8..6afba35 100644 > > --- a/lib/librte_vhost/vhost_rxtx.c > > +++ b/lib/librte_vhost/vhost_rxtx.c > > @@ -178,6 +178,9 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t > queue_id, > > *(volatile uint16_t *)&vq->used->idx += count; > > vq->last_used_idx = res_end_idx; > > > > + /* flush used->idx update before we read avail->flags. */ > > + rte_mb(); > > + > > /* Kick the guest if necessary. */ > > if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) > > eventfd_write((int)vq->callfd, 1); > > > > If not add memory fence what would happen? Packets loss or interrupt > loss?How to test it ? If those two store and load are reordered, guest toggles the interrupt suppression flag, then it checks that there is no more work to do. Actually there is. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-04-24 1:01 ` Linhaifeng 2015-04-24 2:40 ` Xie, Huawei @ 2015-04-24 7:27 ` Luke Gorrie 2015-06-09 7:04 ` Linhaifeng 1 sibling, 1 reply; 17+ messages in thread From: Luke Gorrie @ 2015-04-24 7:27 UTC (permalink / raw) To: Linhaifeng; +Cc: dev, Michael S. Tsirkin On 24 April 2015 at 03:01, Linhaifeng <haifeng.lin@huawei.com> wrote: > If not add memory fence what would happen? Packets loss or interrupt > loss?How to test it ? > You should be able to test it like this: 1. Boot two Linux kernel (e.g. 3.13) guests. 2. Connect them via vhost switch. 3. Run continuous traffic between them (e.g. iperf). I would expect that within a reasonable timeframe (< 1 hour) one of the guests' network interfaces will hang indefinitely due to a missed interrupt. You won't be able to reproduce this using DPDK guests because they are not using the same interrupt suppression method. This is a serious real-world problem. I wouldn't deploy the vhost implementation without this fix. Cheers, -Luke ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-04-24 7:27 ` Luke Gorrie @ 2015-06-09 7:04 ` Linhaifeng 2015-06-09 7:45 ` Luke Gorrie 2015-06-09 8:46 ` Michael S. Tsirkin 0 siblings, 2 replies; 17+ messages in thread From: Linhaifeng @ 2015-06-09 7:04 UTC (permalink / raw) To: Luke Gorrie; +Cc: dev, Michael S. Tsirkin On 2015/4/24 15:27, Luke Gorrie wrote: > On 24 April 2015 at 03:01, Linhaifeng <haifeng.lin@huawei.com> wrote: > >> If not add memory fence what would happen? Packets loss or interrupt >> loss?How to test it ? >> > > You should be able to test it like this: > > 1. Boot two Linux kernel (e.g. 3.13) guests. > 2. Connect them via vhost switch. > 3. Run continuous traffic between them (e.g. iperf). > > I would expect that within a reasonable timeframe (< 1 hour) one of the > guests' network interfaces will hang indefinitely due to a missed interrupt. > > You won't be able to reproduce this using DPDK guests because they are not > using the same interrupt suppression method. > > This is a serious real-world problem. I wouldn't deploy the vhost > implementation without this fix. > > Cheers, > -Luke > I think this patch can't resole this problem. On the other hand we still would miss interrupt. After add rte_mb() function the we want the case is : 1.write used->idx. ring is full or empty. 2.virtio_net open interrupt. 3.read avail->flags. but this case(miss interrupt) would happen too: 1.write used->idx. ring is full or empty. 2.read avail->flags. 3.virtio_net open interrupt. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-06-09 7:04 ` Linhaifeng @ 2015-06-09 7:45 ` Luke Gorrie 2015-06-09 8:46 ` Michael S. Tsirkin 1 sibling, 0 replies; 17+ messages in thread From: Luke Gorrie @ 2015-06-09 7:45 UTC (permalink / raw) To: Linhaifeng; +Cc: dev, Michael S. Tsirkin On 9 June 2015 at 09:04, Linhaifeng <haifeng.lin@huawei.com> wrote: > On 2015/4/24 15:27, Luke Gorrie wrote: > > You should be able to test it like this: > > > > 1. Boot two Linux kernel (e.g. 3.13) guests. > > 2. Connect them via vhost switch. > > 3. Run continuous traffic between them (e.g. iperf). > > > > I would expect that within a reasonable timeframe (< 1 hour) one of the > > guests' network interfaces will hang indefinitely due to a missed > interrupt. > > > > You won't be able to reproduce this using DPDK guests because they are > not > > using the same interrupt suppression method. > > I think this patch can't resole this problem. On the other hand we still > would miss interrupt. > For what it is worth, we were able to reproduce the problem as described above with older Snabb Switch releases and we were also able to verify that inserting a memory barrier fixes this problem. This is the relevant commit in the snabbswitch repo for reference: https://github.com/SnabbCo/snabbswitch/commit/c33cdd8704246887e11d7c353f773f7b488a47f2 In a nutshell, we added an MFENCE instruction after writing used->idx and before checking VRING_F_NO_INTERRUPT. I have not tested this case under DPDK myself and so I am not really certain which memory barrier operations are sufficient/insufficient in that context. I hope that our experience is relevant/helpful though and I am happy to explain more about that if I have missed any important details. Cheers, -Luke ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-06-09 7:04 ` Linhaifeng 2015-06-09 7:45 ` Luke Gorrie @ 2015-06-09 8:46 ` Michael S. Tsirkin 2015-06-09 13:34 ` Xie, Huawei 2015-06-10 8:30 ` Luke Gorrie 1 sibling, 2 replies; 17+ messages in thread From: Michael S. Tsirkin @ 2015-06-09 8:46 UTC (permalink / raw) To: Linhaifeng; +Cc: dev On Tue, Jun 09, 2015 at 03:04:02PM +0800, Linhaifeng wrote: > > > On 2015/4/24 15:27, Luke Gorrie wrote: > > On 24 April 2015 at 03:01, Linhaifeng <haifeng.lin@huawei.com> wrote: > > > >> If not add memory fence what would happen? Packets loss or interrupt > >> loss?How to test it ? > >> > > > > You should be able to test it like this: > > > > 1. Boot two Linux kernel (e.g. 3.13) guests. > > 2. Connect them via vhost switch. > > 3. Run continuous traffic between them (e.g. iperf). > > > > I would expect that within a reasonable timeframe (< 1 hour) one of the > > guests' network interfaces will hang indefinitely due to a missed interrupt. > > > > You won't be able to reproduce this using DPDK guests because they are not > > using the same interrupt suppression method. > > > > This is a serious real-world problem. I wouldn't deploy the vhost > > implementation without this fix. > > > > Cheers, > > -Luke > > > > I think this patch can't resole this problem. On the other hand we still would miss interrupt. > > After add rte_mb() function the we want the case is : > 1.write used->idx. ring is full or empty. > 2.virtio_net open interrupt. > 3.read avail->flags. > > but this case(miss interrupt) would happen too: > 1.write used->idx. ring is full or empty. > 2.read avail->flags. > 3.virtio_net open interrupt. > That's why a correct guest, after detecting an empty used ring, must always re-check used idx at least once after writing avail->flags. By the way, similarly, host side must re-check avail idx after writing used flags. I don't see where snabbswitch does it - is that a bug in snabbswitch? -- MST ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-06-09 8:46 ` Michael S. Tsirkin @ 2015-06-09 13:34 ` Xie, Huawei 2015-06-10 2:36 ` Linhaifeng 2015-06-10 8:30 ` Luke Gorrie 1 sibling, 1 reply; 17+ messages in thread From: Xie, Huawei @ 2015-06-09 13:34 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: dev On 6/9/2015 4:47 PM, Michael S. Tsirkin wrote: > On Tue, Jun 09, 2015 at 03:04:02PM +0800, Linhaifeng wrote: >> >> On 2015/4/24 15:27, Luke Gorrie wrote: >>> On 24 April 2015 at 03:01, Linhaifeng <haifeng.lin@huawei.com> wrote: >>> >>>> If not add memory fence what would happen? Packets loss or interrupt >>>> loss?How to test it ? >>>> >>> You should be able to test it like this: >>> >>> 1. Boot two Linux kernel (e.g. 3.13) guests. >>> 2. Connect them via vhost switch. >>> 3. Run continuous traffic between them (e.g. iperf). >>> >>> I would expect that within a reasonable timeframe (< 1 hour) one of the >>> guests' network interfaces will hang indefinitely due to a missed interrupt. >>> >>> You won't be able to reproduce this using DPDK guests because they are not >>> using the same interrupt suppression method. >>> >>> This is a serious real-world problem. I wouldn't deploy the vhost >>> implementation without this fix. >>> >>> Cheers, >>> -Luke >>> >> I think this patch can't resole this problem. On the other hand we still would miss interrupt. >> >> After add rte_mb() function the we want the case is : >> 1.write used->idx. ring is full or empty. >> 2.virtio_net open interrupt. >> 3.read avail->flags. >> >> but this case(miss interrupt) would happen too: >> 1.write used->idx. ring is full or empty. >> 2.read avail->flags. >> 3.virtio_net open interrupt. >> > That's why a correct guest, after detecting an empty used ring, must always > re-check used idx at least once after writing avail->flags. > > By the way, similarly, host side must re-check avail idx after writing > used flags. I don't see where snabbswitch does it - is that a bug > in snabbswitch? > yes, both host and guest should recheck if there is more work added after they toggle the flag. For DPDK vHost, as it runs in polling mode, we will recheck avail idx soon, so we don't need recheck. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-06-09 13:34 ` Xie, Huawei @ 2015-06-10 2:36 ` Linhaifeng 0 siblings, 0 replies; 17+ messages in thread From: Linhaifeng @ 2015-06-10 2:36 UTC (permalink / raw) To: Xie, Huawei, Michael S. Tsirkin; +Cc: dev On 2015/6/9 21:34, Xie, Huawei wrote: > On 6/9/2015 4:47 PM, Michael S. Tsirkin wrote: >> On Tue, Jun 09, 2015 at 03:04:02PM +0800, Linhaifeng wrote: >>> >>> On 2015/4/24 15:27, Luke Gorrie wrote: >>>> On 24 April 2015 at 03:01, Linhaifeng <haifeng.lin@huawei.com> wrote: >>>> >>>>> If not add memory fence what would happen? Packets loss or interrupt >>>>> loss?How to test it ? >>>>> >>>> You should be able to test it like this: >>>> >>>> 1. Boot two Linux kernel (e.g. 3.13) guests. >>>> 2. Connect them via vhost switch. >>>> 3. Run continuous traffic between them (e.g. iperf). >>>> >>>> I would expect that within a reasonable timeframe (< 1 hour) one of the >>>> guests' network interfaces will hang indefinitely due to a missed interrupt. >>>> >>>> You won't be able to reproduce this using DPDK guests because they are not >>>> using the same interrupt suppression method. >>>> >>>> This is a serious real-world problem. I wouldn't deploy the vhost >>>> implementation without this fix. >>>> >>>> Cheers, >>>> -Luke >>>> >>> I think this patch can't resole this problem. On the other hand we still would miss interrupt. >>> >>> After add rte_mb() function the we want the case is : >>> 1.write used->idx. ring is full or empty. >>> 2.virtio_net open interrupt. >>> 3.read avail->flags. >>> >>> but this case(miss interrupt) would happen too: >>> 1.write used->idx. ring is full or empty. >>> 2.read avail->flags. >>> 3.virtio_net open interrupt. >>> >> That's why a correct guest, after detecting an empty used ring, must always >> re-check used idx at least once after writing avail->flags. >> >> By the way, similarly, host side must re-check avail idx after writing >> used flags. I don't see where snabbswitch does it - is that a bug >> in snabbswitch? >> > yes, both host and guest should recheck if there is more work added > after they toggle the flag. > For DPDK vHost, as it runs in polling mode, we will recheck avail idx > soon, so we don't need recheck. > > DPDK does check the avail idx but does nothing like this: if (vq->last_used_idx == avail_idx) { return; } If we miss an interrupt after calling rte_mb(), (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) is False; while (vq->last_used_idx == avail_idx) is True, then the guest will miss the interrupt forever and virtio-net would stop working. Would this case happen? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-06-09 8:46 ` Michael S. Tsirkin 2015-06-09 13:34 ` Xie, Huawei @ 2015-06-10 8:30 ` Luke Gorrie 2015-06-11 0:50 ` Linhaifeng 1 sibling, 1 reply; 17+ messages in thread From: Luke Gorrie @ 2015-06-10 8:30 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: dev On 9 June 2015 at 10:46, Michael S. Tsirkin <mst@redhat.com> wrote: > By the way, similarly, host side must re-check avail idx after writing > used flags. I don't see where snabbswitch does it - is that a bug > in snabbswitch? Good question. Snabb Switch does not use interrupts from the guest. We always set VRING_F_NO_NOTIFY to tell the guest that it need not interrupt us. Then we run in poll mode and in practice check the avail ring for new descriptors every 20us or so. So the argument for not needing this check in both Snabb Switch and DPDK is that we are running poll mode and don't notice whether interrupts are being sent or not. Is that a solid argument or do I misunderstand what the race condition is? Cheers, -Luke ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags 2015-06-10 8:30 ` Luke Gorrie @ 2015-06-11 0:50 ` Linhaifeng 0 siblings, 0 replies; 17+ messages in thread From: Linhaifeng @ 2015-06-11 0:50 UTC (permalink / raw) To: Luke Gorrie, Michael S. Tsirkin; +Cc: dev On 2015/6/10 16:30, Luke Gorrie wrote: > On 9 June 2015 at 10:46, Michael S. Tsirkin <mst@redhat.com> wrote: > >> By the way, similarly, host side must re-check avail idx after writing >> used flags. I don't see where snabbswitch does it - is that a bug >> in snabbswitch? > > > Good question. > > Snabb Switch does not use interrupts from the guest. We always set > VRING_F_NO_NOTIFY to tell the guest that it need not interrupt us. Then we > run in poll mode and in practice check the avail ring for new descriptors > every 20us or so. Yes, host not need guest to notify when in poll mode but host also need to notify guest who use virtio_net when vring is full or emtpy.If host loss this notification guest would stop working. > > So the argument for not needing this check in both Snabb Switch and DPDK is > that we are running poll mode and don't notice whether interrupts are being > sent or not. > > Is that a solid argument or do I misunderstand what the race condition is? > > Cheers, > -Luke > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v2] vhost: flush used->idx update before reading avail->flags 2015-04-22 16:33 [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags Huawei Xie 2015-04-23 13:57 ` Luke Gorrie 2015-04-24 1:01 ` Linhaifeng @ 2015-04-29 11:11 ` Huawei Xie 2015-05-13 10:46 ` Thomas Monjalon 2 siblings, 1 reply; 17+ messages in thread From: Huawei Xie @ 2015-04-29 11:11 UTC (permalink / raw) To: dev; +Cc: mst update of used->idx and read of avail->flags could be reordered. memory fence should be used to ensure the order, otherwise guest could see a stale used->idx value after it toggles the interrupt suppression flag. After guest sets the interrupt suppression flag, it will check if there is more buffer to process through used->idx. If it sees a stale value, it will exit the processing while host willn't send interrupt to guest. Signed-off-by: Huawei Xie <huawei.xie@intel.com> --- lib/librte_vhost/vhost_rxtx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c index 510ffe8..4809d32 100644 --- a/lib/librte_vhost/vhost_rxtx.c +++ b/lib/librte_vhost/vhost_rxtx.c @@ -178,6 +178,9 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, *(volatile uint16_t *)&vq->used->idx += count; vq->last_used_idx = res_end_idx; + /* flush used->idx update before we read avail->flags. */ + rte_mb(); + /* Kick the guest if necessary. */ if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) eventfd_write((int)vq->callfd, 1); @@ -505,6 +508,9 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, *(volatile uint16_t *)&vq->used->idx += entry_success; vq->last_used_idx = res_end_idx; + /* flush used->idx update before we read avail->flags. */ + rte_mb(); + /* Kick the guest if necessary. */ if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) eventfd_write((int)vq->callfd, 1); -- 1.8.1.4 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: flush used->idx update before reading avail->flags 2015-04-29 11:11 ` [dpdk-dev] [PATCH v2] " Huawei Xie @ 2015-05-13 10:46 ` Thomas Monjalon 2015-05-15 13:43 ` Nikita Kalyazin 0 siblings, 1 reply; 17+ messages in thread From: Thomas Monjalon @ 2015-05-13 10:46 UTC (permalink / raw) To: Huawei Xie; +Cc: dev, mst 2015-04-29 19:11, Huawei Xie: > update of used->idx and read of avail->flags could be reordered. > memory fence should be used to ensure the order, otherwise guest could see a stale used->idx value after it toggles the interrupt suppression flag. > After guest sets the interrupt suppression flag, it will check if there is more buffer to process through used->idx. If it sees a stale value, it will exit the processing while host willn't send interrupt to guest. > > Signed-off-by: Huawei Xie <huawei.xie@intel.com> Applied with following title, thanks vhost: fix virtio freeze due to missed interrupt ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: flush used->idx update before reading avail->flags 2015-05-13 10:46 ` Thomas Monjalon @ 2015-05-15 13:43 ` Nikita Kalyazin 2015-05-15 15:23 ` Michael S. Tsirkin 0 siblings, 1 reply; 17+ messages in thread From: Nikita Kalyazin @ 2015-05-15 13:43 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, mst Hi, Maybe I missed a part of the discussion, but is there any special purpose for using rte_mb (both read and write fence) here rather than rte_wmb (write fence only)? -- Best regards, Nikita Kalyazin, n.kalyazin@samsung.com Software Engineer CE OS Group Samsung R&D Institute Russia Tel: +7 (495) 797-25-00 #3816 Tel: +7 (495) 797-25-03 Office #1501, 12-1, Dvintsev str., Moscow, 127018, Russia On Wed, May 13, 2015 at 12:46:30PM +0200, Thomas Monjalon wrote: > 2015-04-29 19:11, Huawei Xie: > > update of used->idx and read of avail->flags could be reordered. > > memory fence should be used to ensure the order, otherwise guest could see a stale used->idx value after it toggles the interrupt suppression flag. > > After guest sets the interrupt suppression flag, it will check if there is more buffer to process through used->idx. If it sees a stale value, it will exit the processing while host willn't send interrupt to guest. > > > > Signed-off-by: Huawei Xie <huawei.xie@intel.com> > > Applied with following title, thanks > vhost: fix virtio freeze due to missed interrupt > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: flush used->idx update before reading avail->flags 2015-05-15 13:43 ` Nikita Kalyazin @ 2015-05-15 15:23 ` Michael S. Tsirkin 2015-05-18 6:18 ` Nikita Kalyazin 0 siblings, 1 reply; 17+ messages in thread From: Michael S. Tsirkin @ 2015-05-15 15:23 UTC (permalink / raw) To: Nikita Kalyazin; +Cc: dev On Fri, May 15, 2015 at 04:43:33PM +0300, Nikita Kalyazin wrote: > Hi, > > > Maybe I missed a part of the discussion, but is there any special purpose for using rte_mb (both read and write fence) here rather than rte_wmb (write fence only)? The fence is between write of used->idx and read of avail->flags, so rte_wmb won't do anything useful. > -- > > Best regards, > > Nikita Kalyazin, > n.kalyazin@samsung.com > > Software Engineer > CE OS Group > Samsung R&D Institute Russia > Tel: +7 (495) 797-25-00 #3816 > Tel: +7 (495) 797-25-03 > Office #1501, 12-1, Dvintsev str., > Moscow, 127018, Russia > > On Wed, May 13, 2015 at 12:46:30PM +0200, Thomas Monjalon wrote: > > 2015-04-29 19:11, Huawei Xie: > > > update of used->idx and read of avail->flags could be reordered. > > > memory fence should be used to ensure the order, otherwise guest could see a stale used->idx value after it toggles the interrupt suppression flag. > > > After guest sets the interrupt suppression flag, it will check if there is more buffer to process through used->idx. If it sees a stale value, it will exit the processing while host willn't send interrupt to guest. > > > > > > Signed-off-by: Huawei Xie <huawei.xie@intel.com> > > > > Applied with following title, thanks > > vhost: fix virtio freeze due to missed interrupt > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: flush used->idx update before reading avail->flags 2015-05-15 15:23 ` Michael S. Tsirkin @ 2015-05-18 6:18 ` Nikita Kalyazin 0 siblings, 0 replies; 17+ messages in thread From: Nikita Kalyazin @ 2015-05-18 6:18 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: dev Ah, sorry. I looked at it without the context. Thanks. -- Best regards, Nikita Kalyazin, n.kalyazin@samsung.com Software Engineer CE OS Group Samsung R&D Institute Russia Tel: +7 (495) 797-25-00 #3816 Tel: +7 (495) 797-25-03 Office #1501, 12-1, Dvintsev str., Moscow, 127018, Russia On Fri, May 15, 2015 at 05:23:35PM +0200, Michael S. Tsirkin wrote: > On Fri, May 15, 2015 at 04:43:33PM +0300, Nikita Kalyazin wrote: > > Hi, > > > > > > Maybe I missed a part of the discussion, but is there any special purpose for using rte_mb (both read and write fence) here rather than rte_wmb (write fence only)? > > The fence is between write of used->idx and read of avail->flags, so > rte_wmb won't do anything useful. > > > -- > > > > Best regards, > > > > Nikita Kalyazin, > > n.kalyazin@samsung.com > > > > Software Engineer > > CE OS Group > > Samsung R&D Institute Russia > > Tel: +7 (495) 797-25-00 #3816 > > Tel: +7 (495) 797-25-03 > > Office #1501, 12-1, Dvintsev str., > > Moscow, 127018, Russia > > > > On Wed, May 13, 2015 at 12:46:30PM +0200, Thomas Monjalon wrote: > > > 2015-04-29 19:11, Huawei Xie: > > > > update of used->idx and read of avail->flags could be reordered. > > > > memory fence should be used to ensure the order, otherwise guest could see a stale used->idx value after it toggles the interrupt suppression flag. > > > > After guest sets the interrupt suppression flag, it will check if there is more buffer to process through used->idx. If it sees a stale value, it will exit the processing while host willn't send interrupt to guest. > > > > > > > > Signed-off-by: Huawei Xie <huawei.xie@intel.com> > > > > > > Applied with following title, thanks > > > vhost: fix virtio freeze due to missed interrupt > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-06-11 0:51 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-22 16:33 [dpdk-dev] [PATCH] vhost: flush used->idx update before reading avail->flags Huawei Xie 2015-04-23 13:57 ` Luke Gorrie 2015-04-24 1:01 ` Linhaifeng 2015-04-24 2:40 ` Xie, Huawei 2015-04-24 7:27 ` Luke Gorrie 2015-06-09 7:04 ` Linhaifeng 2015-06-09 7:45 ` Luke Gorrie 2015-06-09 8:46 ` Michael S. Tsirkin 2015-06-09 13:34 ` Xie, Huawei 2015-06-10 2:36 ` Linhaifeng 2015-06-10 8:30 ` Luke Gorrie 2015-06-11 0:50 ` Linhaifeng 2015-04-29 11:11 ` [dpdk-dev] [PATCH v2] " Huawei Xie 2015-05-13 10:46 ` Thomas Monjalon 2015-05-15 13:43 ` Nikita Kalyazin 2015-05-15 15:23 ` Michael S. Tsirkin 2015-05-18 6:18 ` Nikita Kalyazin
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).