Re: [Qemu-devel] v6: [PATCH 0/3]: Threadlets: A generic task offloading framework

[Stefan Hajnoczi]

On Wed, Oct 27, 2010 at 8:57 AM, Amit Shah <address@hidden> wrote:
> On (Sat) Oct 23 2010 [13:05:48], Stefan Hajnoczi wrote:
>> >> >You cannot access guest memory using QEMU RAM functions (or use the
>> >> >virtqueue_pop() function which uses them) from a thread without taking
>> >> >the QEMU global mutex.
>> >> >
>> >> >The abort stack trace is a result of accessing guest RAM from two
>> >> >threads simultaneously.
>> >> >
>> >> >In general it is not safe to use QEMU functions from a thread unless
>> >> >they are explicitly written to work outside the QEMU global mutex.
>> >> >Most functions assume the global mutex, which serializes I/O thread
>> >> >and vcpu changes to global state, is held.
>> >>
>> >> Yes, threadlets are only meant to be used to make synchronous system
>> >> calls asynchronous.  They are not meant to add parallelism to QEMU
>> >> (yet).
>> >
>> > Yes -- I realised that.  (But I don't see why the virtio rings get
>> > modified elsewhere other than the only function I push/pop from above.)
>>
>> The particular race condition triggered by virtqueue_pop() is because
>> guest RAM access functions try to optimize frequently accessed memory
>> to keep lookup time to a minimum.  The current RAM block is put onto
>> the beginning of the lookup list so the next access to it will be
>> faster.  When two threads are looking up guest memory simultaneously
>> one may be modifying the list while the other traverses it...
>
> Right, thanks.
>
>> > Anyway, just one question as I've still not read the code: does a
>> > running work item in a threadlet block migration?  Do the remaining work
>> > items in a threadlet get migrated fine?
>>
>> There's no migration support in the threadlets infrastructure itself.
>>
>> Threadlets users need to think about how to safely migrate, either
>> through cancellation or waiting for all pending work to complete.  For
>> example, migration will quiesce aio requests using qemu_aio_flush() so
>> there is no outstanding work to be migrated.
>
> The problem then is that the API doesn't have functions to either stall
> all pending work in a threadlet or cancel all work or even block the
> caller till all work is finished.  I think the API relies on the work
> signalling some sort of completion, which means callers (or threadlet
> users) have to keep track of all the work scheduled, which might be
> uneecessary.

I agree that placing responsibility on the caller is not ideal in
simple cases like waiting for all work to complete.  On the other hand
it allows custom behavior to be implemented since the caller has full
control.

I'd wait to see how callers actually use threadlets and deal with
migration.  Patterns may emerge and can be put into common code.

Stefan