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
--
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
