Marcelo Tosatti wrote:
> Hi Xiantao,
>
> On Wed, Oct 15, 2008 at 09:47:24PM +0800, Zhang, Xiantao wrote:
>> + expires = div64_u64(itc_diff, cyc_per_usec);
>> + kt = ktime_set(0, 1000 * expires);
>> +
>> + down_read(&vcpu->kvm->slots_lock);
>> + vcpu->arch.ht_active = 1;
>> + hrtimer_start(p_ht, kt, HRTIMER_MODE_ABS);
>>
>> - if (irqchip_in_kernel(vcpu->kvm)) {
>> vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
>> kvm_vcpu_block(vcpu);
>> hrtimer_cancel(p_ht);
>> vcpu->arch.ht_active = 0;
>>
>> + if (test_and_clear_bit(KVM_REQ_UNHALT, &vcpu->requests))
>> + if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED)
>> + vcpu->arch.mp_state = + KVM_MP_STATE_RUNNABLE;
>> + up_read(&vcpu->kvm->slots_lock);
>
> You should release slots_lock when blocking via kvm_vcpu_block().
> Otherwise paths that grab it for write will depend on these vcpus to
> unhalt.
>
> BTW, none of the data structures in this section of code should be
> protected by slots_lock?
Hi, Marcelo
Agree, I have made a patch to remove it last week, but seems Avi hasn't commited it yet.
Avi,
Could you help to commit the attached patch, even though it is just a potential issue(write lock is very rare in current code).
Thanks
Xiantao --- Begin Message ---
- Subject: [03/03][PATCH] KVM: ia64: kvm halt logic doesn't need lock to protect.
- From: "Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>
- Date: Wed, 29 Oct 2008 12:39:31 +0800
- Accept-language: en-US
- Acceptlanguage: en-US
- Cc: "kvm@xxxxxxxxxxxxxxx" <kvm@xxxxxxxxxxxxxxx>
- Thread-index: Ack5gFWErPHqAs57TJ+nCfJ+qHOTSw==
- Thread-topic: [03/03][PATCH] KVM: ia64: kvm halt logic doesn't need lock to protect.
>From 4858a5c47c5dce88a62a6edf427d8709f3ebda15 Mon Sep 17 00:00:00 2001 From: Xiantao Zhang <xiantao.zhang@xxxxxxxxx> Date: Thu, 23 Oct 2008 15:03:38 +0800 Subject: [PATCH] KVM: ia64: kvm halt logic doesn't need lock to protect. Remove the lock protection for kvm halt logic, otherwise, once other vcpus want to acquire the lock, and they have to wait all vcpus are waken up from halt. Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx> --- arch/ia64/kvm/kvm-ia64.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index 6b1e31b..93c7f18 100644 --- a/arch/ia64/kvm/kvm-ia64.c +++ b/arch/ia64/kvm/kvm-ia64.c @@ -439,7 +439,6 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu) expires = div64_u64(itc_diff, cyc_per_usec); kt = ktime_set(0, 1000 * expires); - down_read(&vcpu->kvm->slots_lock); vcpu->arch.ht_active = 1; hrtimer_start(p_ht, kt, HRTIMER_MODE_ABS); @@ -452,7 +451,6 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu) if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; - up_read(&vcpu->kvm->slots_lock); if (vcpu->arch.mp_state != KVM_MP_STATE_RUNNABLE) return -EINTR; -- 1.6.0Attachment: 0003-KVM-ia64-kvm-halt-logic-doesn-t-need-lock-to-prote.patch
Description: 0003-KVM-ia64-kvm-halt-logic-doesn-t-need-lock-to-prote.patch
--- End Message ---
