Re: [Qemu-devel] [PATCH 0/9] introduce virtio net dataplane

[Paolo Bonzini]

Il 21/02/2013 17:33, Stefan Hajnoczi ha scritto:
> Interesting patch series.  I want to share my thoughts on the status of
> dataplane and what I'm currently working on.  There will probably be
> some overlap that we can coordinate.
> 
> First, I want to eventually delete hw/dataplane/ :).  That is because
> dataplane duplicates an event loop, thread-friendly guest RAM access,
> and virtio.  QEMU already has all this functionality except it's not
> thread-friendly.
> 
> Because hw/dataplane/ will eventually go away we should avoid adding new
> code where possible.

100% agree.  In particular hw/dataplane/event-poll.c should be the first
to go away, but AioContext provides the functionality that Ping Fan
needs.  But hw/dataplane/vring.c will probably be here for a longer
time, so I don't see a problem in touching that.

> This especially means things like adding live
> migration support or solving other limitations.  The limitations are
> there to motivate core QEMU refactoring, so that core QEMU code can be
> used directly instead of reimplementing it.
> 
> Here's my TODO list:
>  * Thread pool <-> AioContext
> 
>    The thread pool uses an EventNotifier to invoke callbacks.  In a
>    multiple event loop world we need to invoke callbacks in the event
>    loop that they are associated with.
> 
>    Worker threads are also started using a BH so that the thread
>    inherits the iothread CPU affinity and other characteristics.  I
>    think this can stay.

It can and should stay. :)  BHs are tied to an AioContext.  Running
thread-creation BHs from the AioContext's main thread ensures that the
worker threads inherit the right attributes for e.g. real-time.

>  * BlockDriverState <-> AioContext
> 
>    It probably makes sense to bind a BlockDriverState to an AioContext
>    in which its file descriptors and aio activity happens.

... and have the full chain of BlockDriverStates (bs->file,
bs->backing_file and any others as in the vmdk driver) bound to a single
AioContext.  We will use two BlockDriverStates if the same backing file
happens to be shared by two disks (BTW, anybody ever checked what kind
of havoc happens if you commit to such a BDS? :)).

>  * bdrv_drain_all()/bdrv_flush_all() safety against concurrent requests
> 
>    The monitor, live migration, and other components use
>    bdrv_drain_all() to complete all requests.  It also prevents new
>    requests from starting since the global mutex is held and no device
>    emulation can run.
> 
>    We need to support equivalent semantics in a world where
>    BlockDriverState may be accessed from another thread and have
>    in-flight aio in that event loop.

I think this has two parts.  The first is to make bdrv_drain_all() more
fine-grained.  Many calls can be changed to bdrv_drain() that operates
on a chain of BlockDriverStates (defined as above).  Initially I was
thinking of adding a locking API to AioContext, like:

    aio_lock(bdrv_get_aio_context(bs));
    bdrv_drain(bs);
    ...
    ... do work
    ...
    aio_unlock(bdrv_get_aio_context(bs));

However, this is not enough, because there are some cases in which
bdrv_drain_all() is genuinely needed; for example atomic snapshots.  And
in general we need to stop ioeventfd processing if monitor code runs
while the VCPU is not paused.

So I think we need to register handlers from the AioContext that disable
and enable the ioeventfd:

    aio_grab(bdrv_get_aio_context(bs));
    bdrv_drain(bs);
    ...
    ... do work
    ...
    aio_release(bdrv_get_aio_context(bs));

The actual functions called by aio_grab/aio_release would be defined by
the dataplane thread.

I'm not sure this would be enough to make locking entirely private to
the AioContext, but I hope so.

Anyhow, at this point hw/dataplane/ioq.c goes.  This is IMO doable for 1.5.

>  * Thread-friendly guest RAM access API

... and hw/dataplane/hostmem.c goes...

>  * Convert hw/virtio.c (if necessary) to use thread-friendly RAM access

... and hw/dataplane/vring.c goes.

>  * Assigning ioeventfd to a dedicated thread with its own AioContext
>    event loop

I think here it starts not to be a problem.  I think you can already
have live migration and hotplug at this point, but I may be missing
something.

>  * Thread-friendly interrupt API

What is this?

What's still missing here is TCG support.  Perhaps you can emulate
ioeventfd at the hw/virtio.c level and always going through a host
notifier.  In retrospect, perhaps it was a mistake to make ioeventfd a
non-experimental option.

Paolo