Re: [PATCH v2 2/6] reuse env stop and stopped states

[Avi Kivity <avi@redhat.com>]

On 07/28/2009 09:17 AM, Avi Kivity wrote:
>> I found out that doing kill -38<your_pid>  makes it run again, so 
>> we're likely
>> hanging somewhere while holding qemu_mutex. The state of the process 
>> is "D",
>> so we're holding qemu_mutex, and then calling something that can block.
>
> Sounds like we call a vcpu ioctl from the iothread (or from a 
> different vcpu thread).

That's indeed the case.  We reload the local apic state from the 
iothread instead of the vcpu thread.  Please write a patch to fix this.

>> It's hard for me to believe that this patch introduced it. At best, 
>> it might have
>> made it more likely. Also, I also verified that it sometimes takes a 
>> while until
>> it happen for the first time. Are you sure this is the first patch 
>> that makes it happen?
>
> I haven't been able to reproduce it before this patch.  Maybe this 
> patch doesn't introduce it, only exposes it.
>

It does.  The root problem is that env->stopped is cleared during reset, 
so pause_all_threads() doesn't work:

     uint32_t stop;   /* Stop request */                                 \
     uint32_t stopped; /* Artificially stopped */                        \
...
     /* from this point: preserved by CPU reset */                       \

This kind of bug is incredibly hard to find - you now owe Gleb a solar 
mass worth of beer.  IMO we shouldn't be coding like this, please patch 
upstream to explicitly clear what needs clearing.

I'm now testing the simple fix (moving the variables after the memset 
point).

-- 
error compiling committee.c: too many arguments to function