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?
