On Wed, Jun 08, 2016 at 02:58:04PM +0200, Ladi Prosek wrote:
> On Mon, Jun 6, 2016 at 11:31 PM, Ladi Prosek <lprosek@xxxxxxxxxx> wrote:
> > On Mon, Jun 6, 2016 at 4:35 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> >>
> >>
> >> On 06/06/2016 15:55, Michael S. Tsirkin wrote:
> >>> > According to the spec, if the VIRTIO_RING_F_EVENT_IDX feature bit is
> >>> > negotiated the driver MUST set flags to 0. Not dirtying the available
> >>> > ring in virtqueue_disable_cb may also have a positive performance impact.
> >>>
> >>> Question would be, is this a gain or a loss. We have an extra branch,
> >>> and the write might serve to prefetch the cache line.
> >>>
> >>> > Writes to the used event field (vring_used_event) are still unconditional.
> >>> >
> >>> > Signed-off-by: Ladi Prosek <lprosek@xxxxxxxxxx>
> >>>
> >>> Wow you are right wrt the spec. Should we change the spec or the
> >>> code though?
> >>
> >> I would change the spec and note that bit 0 of the flags is ignored if
> >> event indices are in use.
> >
> > Changing the spec sounds good. I'll see if I can get any meaningful
> > perf numbers with vring_bench, just in case. Would there be any
> > interest in having the tool checked in the tree? There are several
> > commits referencing vring_bench but it seems to exist only in a list
> > archive - took me a while to figure it out.
>
> vring_bench with two threads, host thread polls the queue and moves
> indices from available to used, guest thread polls returned indices
> with:
>
> do {
> virtqueue_disable_cb(vq);
> while ((p = virtqueue_get_buf(vq, &len)) != NULL)
> returned++;
> if (unlikely(virtqueue_is_broken(vq)))
> break;
> } while (!virtqueue_enable_cb(vq));
>
> Results:
> - no effect on branch misses
> - L1 dcache load misses ~0.5% down but with ~0.5% variance so not
> super convincing
>
> Ladi
I think it makes sense to change this - the reason it
was written like this is because we did not have
a shadow, it was easier to change and check the flags directly.
Did you open an issue with virtio spec?
--
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
