linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: eventfd: Fix false positive RCU usage warning
@ 2022-01-04  2:45 Hou Wenlong
  2022-01-04 18:01 ` Sean Christopherson
  2022-01-27  6:54 ` [PATCH v2] " Hou Wenlong
  0 siblings, 2 replies; 4+ messages in thread
From: Hou Wenlong @ 2022-01-04  2:45 UTC (permalink / raw)
  To: kvm; +Cc: Paolo Bonzini, linux-kernel

Fix the following false positive warning:
[   20.995979] =============================
[   20.996878] WARNING: suspicious RCU usage
[   20.997792] 5.16.0-rc4+ #57 Not tainted
[   20.998651] -----------------------------
[   20.999544] arch/x86/kvm/../../../virt/kvm/eventfd.c:484 RCU-list traversed in non-reader section!!
[   21.001490]
[   21.001490] other info that might help us debug this:
[   21.001490]
[   21.003240]
[   21.003240] rcu_scheduler_active = 2, debug_locks = 1
[   21.004662] 3 locks held by fc_vcpu 0/330:
[   21.005573]  #0: ffff8884835fc0b0 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x88/0x6f0 [kvm]
[   21.007617]  #1: ffffc90004c0bb68 (&kvm->srcu){....}-{0:0}, at: vcpu_enter_guest+0x600/0x1860 [kvm]
[   21.009627]  #2: ffffc90004c0c1d0 (&kvm->irq_srcu){....}-{0:0}, at: kvm_notify_acked_irq+0x36/0x180 [kvm]
[   21.011732]
[   21.011732] stack backtrace:
[   21.012733] CPU: 26 PID: 330 Comm: fc_vcpu 0 Not tainted 5.16.0-rc4+
[   21.014189] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[   21.016565] Call Trace:
[   21.017105]  <TASK>
[   21.017591]  dump_stack_lvl+0x44/0x57
[   21.018467]  kvm_notify_acked_gsi+0x6b/0x70 [kvm]
[   21.019533]  kvm_notify_acked_irq+0x8d/0x180 [kvm]
[   21.020616]  kvm_ioapic_update_eoi+0x92/0x240 [kvm]
[   21.021736]  kvm_apic_set_eoi_accelerated+0x2a/0xe0 [kvm]
[   21.022968]  handle_apic_eoi_induced+0x3d/0x60 [kvm_intel]
[   21.024168]  vmx_handle_exit+0x19c/0x6a0 [kvm_intel]
[   21.025255]  vcpu_enter_guest+0x66e/0x1860 [kvm]
[   21.026303]  ? lock_acquire+0x27f/0x300
[   21.027166]  ? lock_is_held_type+0xdf/0x130
[   21.028090]  ? kvm_arch_vcpu_ioctl_run+0x438/0x7f0 [kvm]
[   21.029286]  ? kvm_arch_vcpu_ioctl_run+0x11a/0x7f0 [kvm]
[   21.030485]  kvm_arch_vcpu_ioctl_run+0x438/0x7f0 [kvm]
[   21.031647]  kvm_vcpu_ioctl+0x38a/0x6f0 [kvm]
[   21.032647]  ? __fget_files+0x156/0x220
[   21.033522]  __x64_sys_ioctl+0x89/0xc0
[   21.034360]  ? syscall_trace_enter.isra.18+0xea/0x260
[   21.035478]  do_syscall_64+0x3a/0x90
[   21.036259]  entry_SYSCALL_64_after_hwframe+0x44/0xae

Since srcu read lock is held, it's a false positive warning.
Use hlist_for_each_entry_srcu() instead of hlist_for_each_entry_rcu()
as it also checkes if the right lock is held.

Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com>
---
 virt/kvm/eventfd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 2ad013b8bde9..59b1dd4a549e 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -463,8 +463,8 @@ bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin)
 	idx = srcu_read_lock(&kvm->irq_srcu);
 	gsi = kvm_irq_map_chip_pin(kvm, irqchip, pin);
 	if (gsi != -1)
-		hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
-					 link)
+		hlist_for_each_entry_srcu(kian, &kvm->irq_ack_notifier_list,
+					  link, srcu_read_lock_held(&kvm->irq_srcu))
 			if (kian->gsi == gsi) {
 				srcu_read_unlock(&kvm->irq_srcu, idx);
 				return true;
@@ -480,8 +480,8 @@ void kvm_notify_acked_gsi(struct kvm *kvm, int gsi)
 {
 	struct kvm_irq_ack_notifier *kian;
 
-	hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
-				 link)
+	hlist_for_each_entry_srcu(kian, &kvm->irq_ack_notifier_list,
+				  link, srcu_read_lock_held(&kvm->irq_srcu))
 		if (kian->gsi == gsi)
 			kian->irq_acked(kian);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: eventfd: Fix false positive RCU usage warning
  2022-01-04  2:45 [PATCH] KVM: eventfd: Fix false positive RCU usage warning Hou Wenlong
@ 2022-01-04 18:01 ` Sean Christopherson
  2022-01-27  6:54 ` [PATCH v2] " Hou Wenlong
  1 sibling, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2022-01-04 18:01 UTC (permalink / raw)
  To: Hou Wenlong; +Cc: kvm, Paolo Bonzini, linux-kernel

On Tue, Jan 04, 2022, Hou Wenlong wrote:
> Fix the following false positive warning:
> [   20.995979] =============================
> [   20.996878] WARNING: suspicious RCU usage
> [   20.997792] 5.16.0-rc4+ #57 Not tainted
> [   20.998651] -----------------------------
> [   20.999544] arch/x86/kvm/../../../virt/kvm/eventfd.c:484 RCU-list traversed in non-reader section!!
> [   21.001490]
> [   21.001490] other info that might help us debug this:
> [   21.001490]
> [   21.003240]
> [   21.003240] rcu_scheduler_active = 2, debug_locks = 1
> [   21.004662] 3 locks held by fc_vcpu 0/330:
> [   21.005573]  #0: ffff8884835fc0b0 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x88/0x6f0 [kvm]
> [   21.007617]  #1: ffffc90004c0bb68 (&kvm->srcu){....}-{0:0}, at: vcpu_enter_guest+0x600/0x1860 [kvm]
> [   21.009627]  #2: ffffc90004c0c1d0 (&kvm->irq_srcu){....}-{0:0}, at: kvm_notify_acked_irq+0x36/0x180 [kvm]
> [   21.011732]
> [   21.011732] stack backtrace:
> [   21.012733] CPU: 26 PID: 330 Comm: fc_vcpu 0 Not tainted 5.16.0-rc4+
> [   21.014189] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> [   21.016565] Call Trace:
> [   21.017105]  <TASK>
> [   21.017591]  dump_stack_lvl+0x44/0x57
> [   21.018467]  kvm_notify_acked_gsi+0x6b/0x70 [kvm]
> [   21.019533]  kvm_notify_acked_irq+0x8d/0x180 [kvm]
> [   21.020616]  kvm_ioapic_update_eoi+0x92/0x240 [kvm]
> [   21.021736]  kvm_apic_set_eoi_accelerated+0x2a/0xe0 [kvm]
> [   21.022968]  handle_apic_eoi_induced+0x3d/0x60 [kvm_intel]
> [   21.024168]  vmx_handle_exit+0x19c/0x6a0 [kvm_intel]
> [   21.025255]  vcpu_enter_guest+0x66e/0x1860 [kvm]
> [   21.026303]  ? lock_acquire+0x27f/0x300
> [   21.027166]  ? lock_is_held_type+0xdf/0x130
> [   21.028090]  ? kvm_arch_vcpu_ioctl_run+0x438/0x7f0 [kvm]
> [   21.029286]  ? kvm_arch_vcpu_ioctl_run+0x11a/0x7f0 [kvm]
> [   21.030485]  kvm_arch_vcpu_ioctl_run+0x438/0x7f0 [kvm]
> [   21.031647]  kvm_vcpu_ioctl+0x38a/0x6f0 [kvm]
> [   21.032647]  ? __fget_files+0x156/0x220
> [   21.033522]  __x64_sys_ioctl+0x89/0xc0
> [   21.034360]  ? syscall_trace_enter.isra.18+0xea/0x260
> [   21.035478]  do_syscall_64+0x3a/0x90
> [   21.036259]  entry_SYSCALL_64_after_hwframe+0x44/0xae

For future submissions, please strip the timestamps and the "? " entries, they're
unnecessary noise.

> Since srcu read lock is held, it's a false positive warning.

Stronger phrashing should be used, this makes it sound like SRCU is held by
coincidence, but kvm_unregister_irq_ack_notifier() does synchronize_srcu(&kvm->irq_srcu),
i.e. irq_srcu is indeed intended protection mechansim.

> Use hlist_for_each_entry_srcu() instead of hlist_for_each_entry_rcu()
> as it also checkes if the right lock is held.
> 
> Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com>

Changelog nits aside,

Reviewed-by: Sean Christopherson <seanjc@google.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] KVM: eventfd: Fix false positive RCU usage warning
  2022-01-04  2:45 [PATCH] KVM: eventfd: Fix false positive RCU usage warning Hou Wenlong
  2022-01-04 18:01 ` Sean Christopherson
@ 2022-01-27  6:54 ` Hou Wenlong
  2022-01-27 12:08   ` Paolo Bonzini
  1 sibling, 1 reply; 4+ messages in thread
From: Hou Wenlong @ 2022-01-27  6:54 UTC (permalink / raw)
  To: kvm; +Cc: Hou Wenlong, Sean Christopherson, Paolo Bonzini, linux-kernel

From: Hou Wenlong <houwenlong93@linux.alibaba.com>

Fix the following false positive warning:
 =============================
 WARNING: suspicious RCU usage
 5.16.0-rc4+ #57 Not tainted
 -----------------------------
 arch/x86/kvm/../../../virt/kvm/eventfd.c:484 RCU-list traversed in non-reader section!!

 other info that might help us debug this:

 rcu_scheduler_active = 2, debug_locks = 1
 3 locks held by fc_vcpu 0/330:
  #0: ffff8884835fc0b0 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x88/0x6f0 [kvm]
  #1: ffffc90004c0bb68 (&kvm->srcu){....}-{0:0}, at: vcpu_enter_guest+0x600/0x1860 [kvm]
  #2: ffffc90004c0c1d0 (&kvm->irq_srcu){....}-{0:0}, at: kvm_notify_acked_irq+0x36/0x180 [kvm]

 stack backtrace:
 CPU: 26 PID: 330 Comm: fc_vcpu 0 Not tainted 5.16.0-rc4+
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
 Call Trace:
  <TASK>
  dump_stack_lvl+0x44/0x57
  kvm_notify_acked_gsi+0x6b/0x70 [kvm]
  kvm_notify_acked_irq+0x8d/0x180 [kvm]
  kvm_ioapic_update_eoi+0x92/0x240 [kvm]
  kvm_apic_set_eoi_accelerated+0x2a/0xe0 [kvm]
  handle_apic_eoi_induced+0x3d/0x60 [kvm_intel]
  vmx_handle_exit+0x19c/0x6a0 [kvm_intel]
  vcpu_enter_guest+0x66e/0x1860 [kvm]
  kvm_arch_vcpu_ioctl_run+0x438/0x7f0 [kvm]
  kvm_vcpu_ioctl+0x38a/0x6f0 [kvm]
  __x64_sys_ioctl+0x89/0xc0
  do_syscall_64+0x3a/0x90
  entry_SYSCALL_64_after_hwframe+0x44/0xae

Since kvm_unregister_irq_ack_notifier() does synchronize_srcu(&kvm->irq_srcu),
i.e. kvm->irq_ack_notifier_list is protected by kvm->irq_srcu. And
kvm->irq_srcu SRCU read lock is held in kvm_notify_acked_irq(), it's
a false positive warning. Use hlist_for_each_entry_srcu() instead of
hlist_for_each_entry_rcu() as it also checkes if the right lock is held.

Reviewed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com>
---
 virt/kvm/eventfd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 2ad013b8bde9..59b1dd4a549e 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -463,8 +463,8 @@ bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin)
 	idx = srcu_read_lock(&kvm->irq_srcu);
 	gsi = kvm_irq_map_chip_pin(kvm, irqchip, pin);
 	if (gsi != -1)
-		hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
-					 link)
+		hlist_for_each_entry_srcu(kian, &kvm->irq_ack_notifier_list,
+					  link, srcu_read_lock_held(&kvm->irq_srcu))
 			if (kian->gsi == gsi) {
 				srcu_read_unlock(&kvm->irq_srcu, idx);
 				return true;
@@ -480,8 +480,8 @@ void kvm_notify_acked_gsi(struct kvm *kvm, int gsi)
 {
 	struct kvm_irq_ack_notifier *kian;

-	hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
-				 link)
+	hlist_for_each_entry_srcu(kian, &kvm->irq_ack_notifier_list,
+				  link, srcu_read_lock_held(&kvm->irq_srcu))
 		if (kian->gsi == gsi)
 			kian->irq_acked(kian);
 }
--
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] KVM: eventfd: Fix false positive RCU usage warning
  2022-01-27  6:54 ` [PATCH v2] " Hou Wenlong
@ 2022-01-27 12:08   ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2022-01-27 12:08 UTC (permalink / raw)
  To: Hou Wenlong, kvm; +Cc: Hou Wenlong, Sean Christopherson, linux-kernel

On 1/27/22 07:54, Hou Wenlong wrote:
> From: Hou Wenlong <houwenlong93@linux.alibaba.com>
> 
> Fix the following false positive warning:
>   =============================
>   WARNING: suspicious RCU usage
>   5.16.0-rc4+ #57 Not tainted
>   -----------------------------
>   arch/x86/kvm/../../../virt/kvm/eventfd.c:484 RCU-list traversed in non-reader section!!
> 
>   other info that might help us debug this:
> 
>   rcu_scheduler_active = 2, debug_locks = 1
>   3 locks held by fc_vcpu 0/330:
>    #0: ffff8884835fc0b0 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x88/0x6f0 [kvm]
>    #1: ffffc90004c0bb68 (&kvm->srcu){....}-{0:0}, at: vcpu_enter_guest+0x600/0x1860 [kvm]
>    #2: ffffc90004c0c1d0 (&kvm->irq_srcu){....}-{0:0}, at: kvm_notify_acked_irq+0x36/0x180 [kvm]
> 
>   stack backtrace:
>   CPU: 26 PID: 330 Comm: fc_vcpu 0 Not tainted 5.16.0-rc4+
>   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x44/0x57
>    kvm_notify_acked_gsi+0x6b/0x70 [kvm]
>    kvm_notify_acked_irq+0x8d/0x180 [kvm]
>    kvm_ioapic_update_eoi+0x92/0x240 [kvm]
>    kvm_apic_set_eoi_accelerated+0x2a/0xe0 [kvm]
>    handle_apic_eoi_induced+0x3d/0x60 [kvm_intel]
>    vmx_handle_exit+0x19c/0x6a0 [kvm_intel]
>    vcpu_enter_guest+0x66e/0x1860 [kvm]
>    kvm_arch_vcpu_ioctl_run+0x438/0x7f0 [kvm]
>    kvm_vcpu_ioctl+0x38a/0x6f0 [kvm]
>    __x64_sys_ioctl+0x89/0xc0
>    do_syscall_64+0x3a/0x90
>    entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> Since kvm_unregister_irq_ack_notifier() does synchronize_srcu(&kvm->irq_srcu),
> i.e. kvm->irq_ack_notifier_list is protected by kvm->irq_srcu. And
> kvm->irq_srcu SRCU read lock is held in kvm_notify_acked_irq(), it's
> a false positive warning. Use hlist_for_each_entry_srcu() instead of
> hlist_for_each_entry_rcu() as it also checkes if the right lock is held.
> 
> Reviewed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Hou Wenlong <houwenlong93@linux.alibaba.com>
> ---
>   virt/kvm/eventfd.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 2ad013b8bde9..59b1dd4a549e 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -463,8 +463,8 @@ bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin)
>   	idx = srcu_read_lock(&kvm->irq_srcu);
>   	gsi = kvm_irq_map_chip_pin(kvm, irqchip, pin);
>   	if (gsi != -1)
> -		hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
> -					 link)
> +		hlist_for_each_entry_srcu(kian, &kvm->irq_ack_notifier_list,
> +					  link, srcu_read_lock_held(&kvm->irq_srcu))
>   			if (kian->gsi == gsi) {
>   				srcu_read_unlock(&kvm->irq_srcu, idx);
>   				return true;
> @@ -480,8 +480,8 @@ void kvm_notify_acked_gsi(struct kvm *kvm, int gsi)
>   {
>   	struct kvm_irq_ack_notifier *kian;
> 
> -	hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
> -				 link)
> +	hlist_for_each_entry_srcu(kian, &kvm->irq_ack_notifier_list,
> +				  link, srcu_read_lock_held(&kvm->irq_srcu))
>   		if (kian->gsi == gsi)
>   			kian->irq_acked(kian);
>   }
> --
> 2.31.1
> 

Queued, thanks.

Paolo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-01-27 12:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04  2:45 [PATCH] KVM: eventfd: Fix false positive RCU usage warning Hou Wenlong
2022-01-04 18:01 ` Sean Christopherson
2022-01-27  6:54 ` [PATCH v2] " Hou Wenlong
2022-01-27 12:08   ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).