On Mon, Aug 22, 2016 at 4:29 PM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> 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?
I did not. Do you want me to?
> --
> 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
