On 13/11/15 02:57, David Gibson wrote:
> On Thu, Nov 12, 2015 at 10:40:11AM +0100, Thomas Huth wrote:
>> On 12/11/15 09:09, Thomas Huth wrote:
>>> On 11/11/15 18:16, Aravinda Prasad wrote:
[...]
>>>> + qemu_mutex_lock(&spapr->mc_in_progress);
>>>
>>> Using a mutex here is definitely wrong. The kvm_arch_handle_exit() code
>>> is run under the Big QEMU Lock™ (see qemu_mutex_lock_iothread() in
>>> kvm_cpu_exec()),
[...]
>> Ok, I now had a look into the LoPAPR spec, and if I've got that right,
>> you really have to serialize the NMIs in case they happen at multiple
>> CPUs at the same time. So I guess the best thing you can do here is
>> something like:
>>
>> while (spapr->mc_in_progress) {
>> /*
>> * There is already another NMI in progress, thus we need
>> * to yield here to wait until it has been finsihed
>> */
>> qemu_mutex_unlock_iothread();
>> usleep(10);
>> qemu_mutex_lock_iothread();
>> }
>> spapr->mc_in_progress = true;
[...]
> You should be able to avoid the nasty usleep by using a pthread
> condition variable. So here you'd have
>
> while (spapr->mc_in_progress) {
> pthread_cond_wait(&mc_delivery_cond, &qemu_iothread_lock);
> }
> spapr->mc_in_progress = true;
>
> Or.. there may be qemu wrappers around the pthread functions you
> should be using. Once delivery of a single MC is complete, you'd use
> pthread_cond_signal() to wake up any additional ones.
>
> pthread_cond_wait automatically drops the specified mutex internally,
> so access to mc_in_progress itself is still protected by the iothread
> mutex.That's a nice one, didn't know that function yet! And actually, there is already a QEMU wrapper function: qemu_cond_wait() - so this should be used instead since threads on Windows are working differently in QEMU as far as I know. Thomas
signature.asc
Description: OpenPGP digital signature
