Skip to content

Commit 8228c77

Browse files
dwmw2bonzini
authored andcommitted
KVM: x86: switch pvclock_gtod_sync_lock to a raw spinlock
On the preemption path when updating a Xen guest's runstate times, this lock is taken inside the scheduler rq->lock, which is a raw spinlock. This was shown in a lockdep warning: [ 89.138354] ============================= [ 89.138356] [ BUG: Invalid wait context ] [ 89.138358] 5.15.0-rc5+ #834 Tainted: G S I E [ 89.138360] ----------------------------- [ 89.138361] xen_shinfo_test/2575 is trying to lock: [ 89.138363] ffffa34a0364efd8 (&kvm->arch.pvclock_gtod_sync_lock){....}-{3:3}, at: get_kvmclock_ns+0x1f/0x130 [kvm] [ 89.138442] other info that might help us debug this: [ 89.138444] context-{5:5} [ 89.138445] 4 locks held by xen_shinfo_test/2575: [ 89.138447] #0: ffff972bdc3b8108 (&vcpu->mutex){+.+.}-{4:4}, at: kvm_vcpu_ioctl+0x77/0x6f0 [kvm] [ 89.138483] #1: ffffa34a03662e90 (&kvm->srcu){....}-{0:0}, at: kvm_arch_vcpu_ioctl_run+0xdc/0x8b0 [kvm] [ 89.138526] #2: ffff97331fdbac98 (&rq->__lock){-.-.}-{2:2}, at: __schedule+0xff/0xbd0 [ 89.138534] #3: ffffa34a03662e90 (&kvm->srcu){....}-{0:0}, at: kvm_arch_vcpu_put+0x26/0x170 [kvm] ... [ 89.138695] get_kvmclock_ns+0x1f/0x130 [kvm] [ 89.138734] kvm_xen_update_runstate+0x14/0x90 [kvm] [ 89.138783] kvm_xen_update_runstate_guest+0x15/0xd0 [kvm] [ 89.138830] kvm_arch_vcpu_put+0xe6/0x170 [kvm] [ 89.138870] kvm_sched_out+0x2f/0x40 [kvm] [ 89.138900] __schedule+0x5de/0xbd0 Cc: [email protected] Reported-by: [email protected] Fixes: 30b5c85 ("KVM: x86/xen: Add support for vCPU runstate information") Signed-off-by: David Woodhouse <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent fa13843 commit 8228c77

File tree

2 files changed

+15
-15
lines changed

2 files changed

+15
-15
lines changed

arch/x86/include/asm/kvm_host.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1097,7 +1097,7 @@ struct kvm_arch {
10971097
u64 cur_tsc_generation;
10981098
int nr_vcpus_matched_tsc;
10991099

1100-
spinlock_t pvclock_gtod_sync_lock;
1100+
raw_spinlock_t pvclock_gtod_sync_lock;
11011101
bool use_master_clock;
11021102
u64 master_kernel_ns;
11031103
u64 master_cycle_now;

arch/x86/kvm/x86.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2542,15 +2542,15 @@ static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 data)
25422542
kvm_vcpu_write_tsc_offset(vcpu, offset);
25432543
raw_spin_unlock_irqrestore(&kvm->arch.tsc_write_lock, flags);
25442544

2545-
spin_lock_irqsave(&kvm->arch.pvclock_gtod_sync_lock, flags);
2545+
raw_spin_lock_irqsave(&kvm->arch.pvclock_gtod_sync_lock, flags);
25462546
if (!matched) {
25472547
kvm->arch.nr_vcpus_matched_tsc = 0;
25482548
} else if (!already_matched) {
25492549
kvm->arch.nr_vcpus_matched_tsc++;
25502550
}
25512551

25522552
kvm_track_tsc_matching(vcpu);
2553-
spin_unlock_irqrestore(&kvm->arch.pvclock_gtod_sync_lock, flags);
2553+
raw_spin_unlock_irqrestore(&kvm->arch.pvclock_gtod_sync_lock, flags);
25542554
}
25552555

25562556
static inline void adjust_tsc_offset_guest(struct kvm_vcpu *vcpu,
@@ -2780,9 +2780,9 @@ static void kvm_gen_update_masterclock(struct kvm *kvm)
27802780
kvm_make_mclock_inprogress_request(kvm);
27812781

27822782
/* no guest entries from this point */
2783-
spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
2783+
raw_spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
27842784
pvclock_update_vm_gtod_copy(kvm);
2785-
spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
2785+
raw_spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
27862786

27872787
kvm_for_each_vcpu(i, vcpu, kvm)
27882788
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
@@ -2800,15 +2800,15 @@ u64 get_kvmclock_ns(struct kvm *kvm)
28002800
unsigned long flags;
28012801
u64 ret;
28022802

2803-
spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
2803+
raw_spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
28042804
if (!ka->use_master_clock) {
2805-
spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
2805+
raw_spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
28062806
return get_kvmclock_base_ns() + ka->kvmclock_offset;
28072807
}
28082808

28092809
hv_clock.tsc_timestamp = ka->master_cycle_now;
28102810
hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset;
2811-
spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
2811+
raw_spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
28122812

28132813
/* both __this_cpu_read() and rdtsc() should be on the same cpu */
28142814
get_cpu();
@@ -2902,13 +2902,13 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
29022902
* If the host uses TSC clock, then passthrough TSC as stable
29032903
* to the guest.
29042904
*/
2905-
spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
2905+
raw_spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
29062906
use_master_clock = ka->use_master_clock;
29072907
if (use_master_clock) {
29082908
host_tsc = ka->master_cycle_now;
29092909
kernel_ns = ka->master_kernel_ns;
29102910
}
2911-
spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
2911+
raw_spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
29122912

29132913
/* Keep irq disabled to prevent changes to the clock */
29142914
local_irq_save(flags);
@@ -6100,13 +6100,13 @@ long kvm_arch_vm_ioctl(struct file *filp,
61006100
* is slightly ahead) here we risk going negative on unsigned
61016101
* 'system_time' when 'user_ns.clock' is very small.
61026102
*/
6103-
spin_lock_irq(&ka->pvclock_gtod_sync_lock);
6103+
raw_spin_lock_irq(&ka->pvclock_gtod_sync_lock);
61046104
if (kvm->arch.use_master_clock)
61056105
now_ns = ka->master_kernel_ns;
61066106
else
61076107
now_ns = get_kvmclock_base_ns();
61086108
ka->kvmclock_offset = user_ns.clock - now_ns;
6109-
spin_unlock_irq(&ka->pvclock_gtod_sync_lock);
6109+
raw_spin_unlock_irq(&ka->pvclock_gtod_sync_lock);
61106110

61116111
kvm_make_all_cpus_request(kvm, KVM_REQ_CLOCK_UPDATE);
61126112
break;
@@ -8139,9 +8139,9 @@ static void kvm_hyperv_tsc_notifier(void)
81398139
list_for_each_entry(kvm, &vm_list, vm_list) {
81408140
struct kvm_arch *ka = &kvm->arch;
81418141

8142-
spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
8142+
raw_spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
81438143
pvclock_update_vm_gtod_copy(kvm);
8144-
spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
8144+
raw_spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
81458145

81468146
kvm_for_each_vcpu(cpu, vcpu, kvm)
81478147
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
@@ -11182,7 +11182,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
1118211182

1118311183
raw_spin_lock_init(&kvm->arch.tsc_write_lock);
1118411184
mutex_init(&kvm->arch.apic_map_lock);
11185-
spin_lock_init(&kvm->arch.pvclock_gtod_sync_lock);
11185+
raw_spin_lock_init(&kvm->arch.pvclock_gtod_sync_lock);
1118611186

1118711187
kvm->arch.kvmclock_offset = -get_kvmclock_base_ns();
1118811188
pvclock_update_vm_gtod_copy(kvm);

0 commit comments

Comments
 (0)