Re: [Qemu-devel] [regression] dataplane: throughout -40% by commit 580b6b2aa2

[Kevin Wolf]

Am 02.07.2014 um 11:13 hat Paolo Bonzini geschrieben:
> Il 02/07/2014 10:54, Stefan Hajnoczi ha scritto:
> >>Both can be eliminated by introducing a fast path in bdrv_aio_{read,write}v,
> >>that bypasses coroutines in the common case of no I/O throttling, no
> >>copy-on-write, etc.
> >
> >I tried that in 2012 and couldn't measure an improvement above the noise
> >threshold, although it was without dataplane.
> >
> >BTW, we cannot eliminate the BH because the block layer guarantees that
> >callbacks are not invoked with reentrancy.  They are always invoked
> >directly from the event loop through a BH.  This simplifies callers
> >since they don't need to worry about callbacks happening while they are
> >still in bdrv_aio_readv(), for example.
> >
> >Removing this guarantee (by making callers safe first) is orthogonal to
> >coroutines.  But it's hard to do since it requires auditing a lot of
> >code.
> 
> You could also audit the few implementations of
> bdrv_aio_readv/writev (including bdrv_aio_readv/writev_em) to
> guarantee that they do not directly invoke the callbacks.  The rule
> was there before conversion to coroutine, so the implementations
> should be fine.  In fact, most of them are just forwarding to
> another bdrv_aio_readv/writev, and the others go through an
> EventNotifier or bottom half.
> 
> Drivers that implement bdrv_co_readv/writev would not enjoy the fast
> path, and would keep using the BH.

I don't think starting with that fast path as _the_ solution is a good
idea. It would essentially restrict dataplane to the scenarios that used
to work well in 2.0 - just look at what the block.c read/write functions
do: no image formats, (almost?) no block jobs, no 4k sector support, no
writethrough mode, no zero detection, no throttling, no nothing.
Anything we want to keep while using the fast path we would have to
duplicate there.

I much prefer to approach to make it use the normal path and optimise
that one, so that all cases benefit from it. We can probably get the
same results with coroutines - if the BH isn't necessary with AIO in the
common case, it also isn't necessary with coroutines.

> >Another idea is to skip aio_notify() when we're sure the event loop
> >isn't blocked in g_poll().  Doing this is a thread-safe and lockless way
> >might be tricky though.
> 
> Yes, good idea for 2.2 but not now.

Isn't it a first approximation that it's unnecessary when we're already
running in the thread with the AIO main loop? (Which pretty much means
always with dataplane.) Or can it be required even when we don't switch
to a different thread?

> >3. The block layer requires a BH with aio_notify() for
> >bdrv_aio_readv()/bdrv_aio_writev()/bdrv_aio_flush() callbacks regardless
> >of coroutines or not.  Eliminating the BH or skipping aio_notify() will
> >take some work but could speed up QEMU as a whole.
> 
> I think (3) is not really true, and the BH is the actual reason why
> coroutines are slower.

Can't we provide a mechanism with coroutines that checks whether we've
been reentered from the main loop at least once, so that we can avoid
the BH then?

We would still have to think about which stack we want to run it on, but
there would definitely be no aio_notify() involved then.

Kevin