Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1695451 > unrolled thread
| Started by | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| First post | 2017-07-25 09:50 +0200 |
| Last post | 2017-07-26 16:10 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v4] KVM: LAPIC: Fix reentrancy issues with preempt notifiers Wanpeng Li <kernellwp@gmail.com> - 2017-07-25 09:50 +0200
Re: [PATCH v4] KVM: LAPIC: Fix reentrancy issues with preempt notifiers Takashi Iwai <tiwai@suse.de> - 2017-07-26 16:10 +0200
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-07-25 09:50 +0200 |
| Subject | [PATCH v4] KVM: LAPIC: Fix reentrancy issues with preempt notifiers |
| Message-ID | <u72Lo-8oY-43@gated-at.bofh.it> |
From: Wanpeng Li <wanpeng.li@hotmail.com>
Preempt can occur in the preemption timer expiration handler:
CPU0 CPU1
preemption timer vmexit
handle_preemption_timer(vCPU0)
kvm_lapic_expired_hv_timer
hv_timer_is_use == true
sched_out
sched_in
kvm_arch_vcpu_load
kvm_lapic_restart_hv_timer
restart_apic_timer
start_hv_timer
already-expired timer or sw timer triggerd in the window
start_sw_timer
cancel_hv_timer
/* back in kvm_lapic_expired_hv_timer */
cancel_hv_timer
WARN_ON(!apic->lapic_timer.hv_timer_in_use); ==> Oops
This can be reproduced if CONFIG_PREEMPT is enabled.
------------[ cut here ]------------
WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1563 kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm]
CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G OE 4.13.0-rc2+ #16
RIP: 0010:kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm]
Call Trace:
handle_preemption_timer+0xe/0x20 [kvm_intel]
vmx_handle_exit+0xb8/0xd70 [kvm_intel]
kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm]
? kvm_arch_vcpu_load+0x47/0x230 [kvm]
? kvm_arch_vcpu_load+0x62/0x230 [kvm]
kvm_vcpu_ioctl+0x340/0x700 [kvm]
? kvm_vcpu_ioctl+0x340/0x700 [kvm]
? __fget+0xfc/0x210
do_vfs_ioctl+0xa4/0x6a0
? __fget+0x11d/0x210
SyS_ioctl+0x79/0x90
do_syscall_64+0x81/0x220
entry_SYSCALL64_slow_path+0x25/0x25
------------[ cut here ]------------
WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1498 cancel_hv_timer.isra.40+0x4f/0x60 [kvm]
CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G W OE 4.13.0-rc2+ #16
RIP: 0010:cancel_hv_timer.isra.40+0x4f/0x60 [kvm]
Call Trace:
kvm_lapic_expired_hv_timer+0x3e/0xb0 [kvm]
handle_preemption_timer+0xe/0x20 [kvm_intel]
vmx_handle_exit+0xb8/0xd70 [kvm_intel]
kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm]
? kvm_arch_vcpu_load+0x47/0x230 [kvm]
? kvm_arch_vcpu_load+0x62/0x230 [kvm]
kvm_vcpu_ioctl+0x340/0x700 [kvm]
? kvm_vcpu_ioctl+0x340/0x700 [kvm]
? __fget+0xfc/0x210
do_vfs_ioctl+0xa4/0x6a0
? __fget+0x11d/0x210
SyS_ioctl+0x79/0x90
do_syscall_64+0x81/0x220
entry_SYSCALL64_slow_path+0x25/0x25
This patch fixes it by making the caller of start_hv_timer and start_sw_timer
be in preemption-disabled regions, which trivially avoid any reentrancy
issue with preempt notifier.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
v3 -> v4:
* move the out label at the end of the function
v2 -> v3:
* the caller of start_hv_timer and start_sw_timer in a preemption-disabled region
v1 -> v2:
* cleanup patch description
arch/x86/kvm/lapic.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 2819d4c..0221e6e 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1495,11 +1495,10 @@ EXPORT_SYMBOL_GPL(kvm_lapic_hv_timer_in_use);
static void cancel_hv_timer(struct kvm_lapic *apic)
{
+ WARN_ON(preemptible());
WARN_ON(!apic->lapic_timer.hv_timer_in_use);
- preempt_disable();
kvm_x86_ops->cancel_hv_timer(apic->vcpu);
apic->lapic_timer.hv_timer_in_use = false;
- preempt_enable();
}
static bool start_hv_timer(struct kvm_lapic *apic)
@@ -1552,15 +1551,20 @@ static void start_sw_timer(struct kvm_lapic *apic)
static void restart_apic_timer(struct kvm_lapic *apic)
{
+ preempt_disable();
if (!start_hv_timer(apic))
start_sw_timer(apic);
+ preempt_enable();
}
void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
{
struct kvm_lapic *apic = vcpu->arch.apic;
- WARN_ON(!apic->lapic_timer.hv_timer_in_use);
+ preempt_disable();
+ /* The preempt notifier has called apic_timer_expired already */
+ if (!apic->lapic_timer.hv_timer_in_use)
+ goto out;
WARN_ON(swait_active(&vcpu->wq));
cancel_hv_timer(apic);
apic_timer_expired(apic);
@@ -1569,6 +1573,8 @@ void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
advance_periodic_target_expiration(apic);
restart_apic_timer(apic);
}
+out:
+ preempt_enable();
}
EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
@@ -1582,9 +1588,11 @@ void kvm_lapic_switch_to_sw_timer(struct kvm_vcpu *vcpu)
{
struct kvm_lapic *apic = vcpu->arch.apic;
+ preempt_disable();
/* Possibly the TSC deadline timer is not enabled yet */
if (apic->lapic_timer.hv_timer_in_use)
start_sw_timer(apic);
+ preempt_enable();
}
EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
--
2.7.4
[toc] | [next] | [standalone]
| From | Takashi Iwai <tiwai@suse.de> |
|---|---|
| Date | 2017-07-26 16:10 +0200 |
| Message-ID | <u7vaG-1ke-29@gated-at.bofh.it> |
| In reply to | #1695451 |
On Tue, 25 Jul 2017 09:43:15 +0200, Wanpeng Li wrote: > > From: Wanpeng Li <wanpeng.li@hotmail.com> > > Preempt can occur in the preemption timer expiration handler: > > CPU0 CPU1 > > preemption timer vmexit > handle_preemption_timer(vCPU0) > kvm_lapic_expired_hv_timer > hv_timer_is_use == true > sched_out > sched_in > kvm_arch_vcpu_load > kvm_lapic_restart_hv_timer > restart_apic_timer > start_hv_timer > already-expired timer or sw timer triggerd in the window > start_sw_timer > cancel_hv_timer > /* back in kvm_lapic_expired_hv_timer */ > cancel_hv_timer > WARN_ON(!apic->lapic_timer.hv_timer_in_use); ==> Oops > > This can be reproduced if CONFIG_PREEMPT is enabled. > > ------------[ cut here ]------------ > WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1563 kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm] > CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G OE 4.13.0-rc2+ #16 > RIP: 0010:kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm] > Call Trace: > handle_preemption_timer+0xe/0x20 [kvm_intel] > vmx_handle_exit+0xb8/0xd70 [kvm_intel] > kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm] > ? kvm_arch_vcpu_load+0x47/0x230 [kvm] > ? kvm_arch_vcpu_load+0x62/0x230 [kvm] > kvm_vcpu_ioctl+0x340/0x700 [kvm] > ? kvm_vcpu_ioctl+0x340/0x700 [kvm] > ? __fget+0xfc/0x210 > do_vfs_ioctl+0xa4/0x6a0 > ? __fget+0x11d/0x210 > SyS_ioctl+0x79/0x90 > do_syscall_64+0x81/0x220 > entry_SYSCALL64_slow_path+0x25/0x25 > ------------[ cut here ]------------ > WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1498 cancel_hv_timer.isra.40+0x4f/0x60 [kvm] > CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G W OE 4.13.0-rc2+ #16 > RIP: 0010:cancel_hv_timer.isra.40+0x4f/0x60 [kvm] > Call Trace: > kvm_lapic_expired_hv_timer+0x3e/0xb0 [kvm] > handle_preemption_timer+0xe/0x20 [kvm_intel] > vmx_handle_exit+0xb8/0xd70 [kvm_intel] > kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm] > ? kvm_arch_vcpu_load+0x47/0x230 [kvm] > ? kvm_arch_vcpu_load+0x62/0x230 [kvm] > kvm_vcpu_ioctl+0x340/0x700 [kvm] > ? kvm_vcpu_ioctl+0x340/0x700 [kvm] > ? __fget+0xfc/0x210 > do_vfs_ioctl+0xa4/0x6a0 > ? __fget+0x11d/0x210 > SyS_ioctl+0x79/0x90 > do_syscall_64+0x81/0x220 > entry_SYSCALL64_slow_path+0x25/0x25 > > This patch fixes it by making the caller of start_hv_timer and start_sw_timer > be in preemption-disabled regions, which trivially avoid any reentrancy > issue with preempt notifier. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> I've hit this warning today, and your patch seems fixing it. Tested-by: Takashi Iwai <tiwai@suse.de> Thanks! Takashi
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web