Re: [PATCH v3] KVM: VMX: fix lockdep warning on posted intr wakeup

[Yan Zhao]

On Fri, Mar 24, 2023 at 04:13:37PM -0700, Sean Christopherson wrote:
> On Mon, Mar 13, 2023, Yan Zhao wrote:
> > The lock ordering after this patch are:
> > - &p->pi_lock --> &rq->__lock -->
> >   &per_cpu(wakeup_vcpus_on_cpu_lock_out, cpu)
> > - &per_cpu(wakeup_vcpus_on_cpu_lock_in, cpu) -->
> >   &per_cpu(wakeup_vcpus_on_cpu_lock_out, cpu)
> > - &per_cpu(wakeup_vcpus_on_cpu_lock_in, cpu) --> &p->pi_lock
> > 
> > Currently, &rq->__lock is not held in "path sched_in".
> > However, if in future "path sched_in" takes &p->pi_lock or &rq->__lock,
> > lockdep is able to detect and warn in that case.
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> > [sean: path sched_out and path irq does not race, path sched_in does not
> > take &rq->__lock]
> 
> But there's no actual deadlock, right?  I have zero interest in fixing a lockdep
> false positive by making functional changes to KVM.  I am definitely open to making
> changes to somehow let lockdep know what's going on, but complicating KVM's actual
> functionality is too much.
Yes, there's no actual deadlock currently.

But without fixing this issue, debug_locks will be set to false along
with below messages printed. Then lockdep will be turned off and any
other lock detections like lockdep_assert_held()... will not print
warning even when it's obviously violated. 

[  118.873499] ======================================================
[  118.880413] WARNING: possible circular locking dependency detected
[  118.887325] 6.2.0-rc5+ #600 Not tainted
[  118.891613] ------------------------------------------------------
[  118.898519] swapper/5/0 is trying to acquire lock:
[  118.903869] ffff88810f5cac90 (&p->pi_lock){-.-.}-{2:2}, at: try_to_wake_up+0xbb/0x510
[  118.912624]
[  118.912624] but task is already holding lock:
[  118.919138] ffff88885f7fdab8 (&per_cpu(wakeup_vcpus_on_cpu_lock, cpu)){-...}-{2:2}, at: pi_wakeup_handler+0x36/0x80 [kvm_intel]
[  118.931962]
[  118.931962] which lock already depends on the new lock.

Any suggestion?