Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ryan Harper wrote:
>   
>> @@ -388,9 +395,10 @@ static void kvm_add_signal(struct qemu_kvm_signal_table 
>> *sigtab, int signum)
>>  
>>  void kvm_init_new_ap(int cpu, CPUState *env)
>>  {
>> +    pthread_mutex_lock(&vcpu_mutex);
>>      pthread_create(&vcpu_info[cpu].thread, NULL, ap_main_loop, env);
>> -    /* FIXME: wait for thread to spin up */
>> -    usleep(200);
>> +    pthread_cond_wait(&qemu_vcpuup_cond, &vcpu_mutex);
>> +    pthread_mutex_unlock(&vcpu_mutex);
>>     
>
> And something is very wrong here.  The pattern for using a condvar is
>
> 1 take mutex
>
> 2 check condition
>
> 3 if condition is not fulfilled
> 3a   call cond_wait
> 3b   when returning, go back to step 2
>
> 4 unlock mutex
>
>
> Anything else is buggy.
>
> So, either your condvar use is wrong or you don't really want a condvar
> in the first place.  I haven't checked the code.
>   

A flag is needed in the vcpu structure that indicates whether the vcpu 
spun up or not.  This is what the while () condition should be.  This is 
needed as the thread may spin up before it gets to the 
pthread_cond_wait() in which case the signal happens when noone is 
waiting on it.  The other reason a while () is usually needed is that 
cond_signal may not wake up the right thread so it's necessary to check 
whether you really have something to do.  Not really a problem here but 
the former race is.

A condvar is definitely the right thing to use here.

Regards,

Anthony Liguori

> - --
> ➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.7 (GNU/Linux)
>
> iD8DBQFIE2CD2ijCOnn/RHQRAs4kAJ40kbWjNJAzj2gGdbo/sSxZTx5b0ACglbis
> kw7ST4eJK9CXhNbjKphNsUo=
> =ISaC
> -----END PGP SIGNATURE-----
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
> Don't miss this year's exciting event. There's still time to save $100. 
> Use priority code J8TL2D2. 
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> _______________________________________________
> kvm-devel mailing list
> kvm-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Reply via email to