[dpdk-dev] vhost: call write barrier before used index update
Commit Message
Descriptors that have been put into the used vring must be observable by
guest earlier than the new used index value.
Although compiler barrier serves well for Intel architectue here, the
proper cross-platform solution is to use write barrier before the used
index is updated.
Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>
---
lib/librte_vhost/vhost_rxtx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Comments
On Tue, Oct 20, 2015 at 05:07:46PM +0300, Nikita Kalyazin wrote:
> Descriptors that have been put into the used vring must be observable by
> guest earlier than the new used index value.
> Although compiler barrier serves well for Intel architectue here, the
> proper cross-platform solution is to use write barrier before the used
> index is updated.
>
> Signed-off-by: Nikita Kalyazin <n.kalyazin@samsung.com>
> ---
Yes, but no! :-)
This has been discussed a number of times before on list, and the consensus
seems to be that the correct way to fix this is to introduce a set of specific
barrier operations that insert the correct barrier type on each architecture,
i.e. compiler barriers on IA, and full wmbs on architectures that require that.
See discussion here: http://dpdk.org/dev/patchwork/patch/4293/
and in the thread here: http://dpdk.org/ml/archives/dev/2015-March/015202.html
So correct problem statment, but unfortunately NAK for the implementation.
Patches for general memory barrier implementation as described above welcome :-)
Regards,
/Bruce
Hi,
> This has been discussed a number of times before on list, and the consensus
> seems to be that the correct way to fix this is to introduce a set of specific
> barrier operations that insert the correct barrier type on each architecture,
> i.e. compiler barriers on IA, and full wmbs on architectures that require that.
Linux kernel contains two sets of macros: *mb() and smp_*mb(). As far as
I understand, the former are meant to order memory accesses when interacting
with peripherals (physical NICs in our case), and the latter - to provide
synchronization between CPU cores (applicable for virtual NICs in our case).
smp_*mb() for Intel architecture would be a simple compiler barrier, whereas
for processors with weaker memory model they may use real barrier
instructions.
Maybe implementing barriers similar way would work in DPDK as well?
On 21/10/2015 16:42, Nikita Kalyazin wrote:
> Hi,
>
>> This has been discussed a number of times before on list, and the consensus
>> seems to be that the correct way to fix this is to introduce a set of specific
>> barrier operations that insert the correct barrier type on each architecture,
>> i.e. compiler barriers on IA, and full wmbs on architectures that require that.
> Linux kernel contains two sets of macros: *mb() and smp_*mb(). As far as
> I understand, the former are meant to order memory accesses when interacting
> with peripherals (physical NICs in our case), and the latter - to provide
> synchronization between CPU cores (applicable for virtual NICs in our case).
> smp_*mb() for Intel architecture would be a simple compiler barrier, whereas
> for processors with weaker memory model they may use real barrier
> instructions.
> Maybe implementing barriers similar way would work in DPDK as well?
>
It should work indeed.
Not sure if we need two separate sets of barrier functions, the
smb_*mb() might work on their own (certainly for IA, they should do -
not sure for other architectures), but I think the general consensus,
based on previous discussions, is that this is the direction we want to
go in for these barrier issues.
/Bruce
@@ -216,7 +216,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
}
}
- rte_compiler_barrier();
+ rte_wmb();
/* Wait until it's our turn to add our buffer to the used ring. */
while (unlikely(vq->last_used_idx != res_base_idx))
@@ -512,7 +512,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
entry_success = copy_from_mbuf_to_vring(dev, res_base_idx,
res_cur_idx, pkts[pkt_idx]);
- rte_compiler_barrier();
+ rte_wmb();
/*
* Wait until it's our turn to add our buffer
@@ -751,7 +751,7 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
entry_success++;
}
- rte_compiler_barrier();
+ rte_wmb();
vq->used->idx += entry_success;
/* Kick guest if required. */
if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))