Re: [Qemu-devel] [RFC] queue_work proposal

[Glauber Costa <glommer@redhat.com>]

On Thu, Sep 03, 2009 at 11:45:36AM +0300, Avi Kivity wrote:
> On 09/03/2009 03:52 AM, Glauber Costa wrote:
>> Hi guys
>>
>> In this patch, I am attaching an early version of a new "on_vcpu" mechanism (after
>> making it generic, I saw no reason to keep its name). It allows us to guarantee
>> that a piece of code will be executed in a certain vcpu, indicated by a CPUState.
>>
>> I am sorry for the big patch, I just dumped what I had so we can have early directions.
>> When it comes to submission state, I'll split it accordingly.
>>
>> As we discussed these days at qemu-devel, I am using pthread_set/get_specific for
>> dealing with thread-local variables. Note that they are not used from signal handlers.
>> A first optimization would be to use TLS variables where available.
>>
>> In vl.c, I am providing a version of queue_work for the IO-thread, and other for normal
>> operation. The "normal" one should fix the problems Jan is having, since it does nothing
>> more than just issuing the function we want to execute.
>>
>> The io-thread version is tested with both tcg and kvm, and works (to the extent they were
>> working before, which in kvm case, is not much)
>>    
>
> on_vcpu() and queue_work() are fundamentally different (yes, I see the  
> wait parameter, and I think there should be two separate functions for  
> such different behaviours).
Therefore, the name change. The exact on_vcpu behaviour, however, can be
implemented ontop of queue_work(). Instead of doing that, I opted for using it
implicitly inside kvm_vcpu_ioctl, to guarantee that vcpu ioctls will always run
on the right thread context. Looking at qemu-kvm, it seems that there are a couple
of other functions that are not ioctls, and need on_vcpu semantics. Using them becomes
a simple matter of doing:

   queue_work(env, func, data, 1);

I really don't see the big difference you point. They are both there to force a specific
function to be executed in the right thread context.

>
> Why do we need queue_work() in the first place?
To force a function to be executed in the correct thread context.
Why do we need on_vcpu in the first place?

>
> Is there a way to limit the queue size to prevent overflow?
It can be, but it gets awkward. What do you do when you want a function needs to execute
on another thread, but you can't? Block it? Refuse?

We could pick one, but I see no need. The vast majority of work will never get queued,
since we'll be in the right context already.