On Fri, Sep 16, 2016 at 12:11:59PM +0200, Paolo Bonzini wrote:
>
>
> On 16/09/2016 11:46, Stefan Hajnoczi wrote:
> >> > The idea is to have a r/w descriptor in a ring structure,
> >> > replacing the used and available ring, index and descriptor
> >> > buffer.
> > This is probably obvious but:
> >
> > Why is the desc.index field needed?
> >
> > The index field would be needed if the driver had to jump over a DESC_HW
> > descriptor when adding descriptors, but that doesn't seem necessary
> > since the device is supposed to overwrite descriptors in ascending ring
> > order when they complete out-of-order.
>
> AIUI, when reading completed descriptors, the driver uses the index in
> order to map the descriptor to the original request.
>
> It's more of an id; it's certainly not a linked list as in virtio 1.0 rings.
>
> I think we can refine the description of the index, saying that only the
> first descriptor in a request needs an index; descriptors reached via
> VRING_DESC_F_NEXT do not need an index. The advantage is that the
> device only needs to remember the descriptor of the first descriptor.
>
> However...
>
> - is there any reason for the device or driver to even look at DESC_HW
> if VRING_DESC_F_NEXT is set? DESC_HW only matters on the first buffer.
>
> - why not drop VRING_DESC_F_NEXT altogether?
>
> Because only two cases are strictly necessary:
>
> - single-buffer requests. These have some very important cases:
> virtio-rng, virtio-serial, virtio-net with merged rxbufs, virtio-scsi
> and virtio-vsock events, virtio-balloon, virtio-input all use
> single-buffer requests exclusively.
>
> - indirect requests, where the length of the indirect descriptor can be
> used to figure out the number of buffers
>
> Let's just drop the added complication of multi-buffer direct requests.
> Yes, this can cause one extra cache miss but 1) as shown above a lot of
> performance-critical cases use single-buffer requests 2) no one has ever
> bothered to use multi-buffer direct requests in Linux drivers, at least
> when indirect descriptors are available.
True. I note that dpdk used direct descriptors exclusively until
very recently.
commit 6dc5de3a6aefba3946fe04368d93994db3f7a5fd
Author: Stephen Hemminger <>
Date: Fri Mar 4 10:19:19 2016 -0800
virtio: use indirect ring elements
I'd like to make sure removing these doesn't cause regressions.
Stephen, could you please confirm that
1. with your patch, each packet takes at most 1 slot
2. your patch was a win for most workloads
(as opposed to e.g. enabling/disabling this depending on workload)
Also, it does not look like vhost in dpdk supports
indirect descriptors.
> This simplifies the device logic like this
>
> read lookahead descriptor from ring
> while (DESC_HW set in descriptor) {
> advance internal used index
> if VRING_DESC_F_INDIRECT
> collect len/16 descriptors from address
> else
> use (len,addr,flags & VRING_DESC_F_WRITE) as a single buffer
> process request
> read lookahead descriptor from ring
> }
>
> and on completion
>
> if (next descriptor has DESC_HW set, i.e. ring full) {
> set broken flag
> } else {
> write addr,index from original descriptor into addr,index
> write number of written bytes into len
> wmb
> write flags from original descriptor & ~DESC_HW into flags
> }
>
> Paolo
>
