On 26/06/12 05:35, Christoffer Dall wrote:
> On Mon, Jun 25, 2012 at 6:13 PM, Christoffer Dall
> <c.dall at virtualopensystems.com> wrote:
>> On Tue, May 15, 2012 at 12:00 PM, Marc Zyngier <marc.zyngier at arm.com> wrote:
>>> Move kvm_timer_vcpu_terminate() call to kvm_arch_vcpu_uninit(),
>>> as kvm_arch_vcpu_destroy() happens too late and we end up with
>>> the timers firing when the VCPUs are already dead.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
>>> ---
>>> arch/arm/kvm/arm.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>>> index 49ed283..1cf2ca8 100644
>>> --- a/arch/arm/kvm/arm.c
>>> +++ b/arch/arm/kvm/arm.c
>>> @@ -231,7 +231,6 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
>>>
>>> void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>>> {
>>> - kvm_timer_vcpu_terminate(vcpu);
>>> kvm_arch_vcpu_free(vcpu);
>>> }
>>
>> this is only called if kvm_vm_ioctl_create_vcpu() fails, where we
>> would never have scheduled any timers - in fact it's dangerous because
>> if that code is reordered so the INIT_WORK has not been done already
>> I'm not sure the cancel_work_sync will go well.
>>
>>>
>>> @@ -272,6 +271,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>>>
>>> void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
>>> {
>>> + kvm_timer_vcpu_terminate(vcpu);
>>> }
>>>
>>
>> eh, we never call this on ARM since kvm_arch_vcpu_init() never fails.
>
> strike the bit about kvm_arch_vcpu_init, it should be called if
> kvm_arch_vcpu_create() fails (patch in the workings). I still think
> you wish to look at kvm_arch_destroy_vm() though.
>
Have a look at my current tree (commit 4b10009). It puts the call to
kvm_timer_vcpu_terminate() in kvm_arch_vcpu_free, which is called from
both kvm_arch_vcpu_destroy() and kvm_arch_destroy_vm().
M.
--
Jazz is not dead. It just smells funny...
