Date:   Tue, 4 Jan 2022 18:01:16 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Hou Wenlong <houwenlong93@linux.alibaba.com>
Cc:     kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: eventfd: Fix false positive RCU usage warning
Message-ID: <YdSLbGM8QQXJ7WWG@google.com>
References: <ab1358b84c60e6c942c270e3fe1a32bfa3177f3c.1641264282.git.houwenlong93@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ab1358b84c60e6c942c270e3fe1a32bfa3177f3c.1641264282.git.houwenlong93@linux.alibaba.com>
Precedence: bulk

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>