Re: [PATCH v3] KVM: SEV: Mark nested locking of vcpu->lock

[Paolo Bonzini]

On 4/29/22 17:51, Peter Gonda wrote:
> > No, you don't need any of this.  You can rely on there being only one
> > depmap, otherwise you wouldn't need the mock releases and acquires at
> > all.  Also the unlocking order does not matter for deadlocks, only the
> > locking order does.  You're overdoing it. :)
>
> Hmm I'm slightly confused here then. If I take your original suggestion of:
>
>          bool acquired = false;
>          kvm_for_each_vcpu(...) {
>                  if (acquired)
>                          mutex_release(&vcpu->mutex.dep_map,
> _THIS_IP_);  <-- Warning here
>                  if (mutex_lock_killable_nested(&vcpu->mutex, role)
>                          goto out_unlock;
>                  acquired = true;
>
> """
> [ 2810.088982] =====================================
> [ 2810.093687] WARNING: bad unlock balance detected!
> [ 2810.098388] 5.17.0-dbg-DEV #5 Tainted: G           O
> [ 2810.103788] -------------------------------------

Ah even if the contents of the dep_map are the same for all locks, it 
also uses the *pointer* to the dep_map to track (class, subclass) -> 
pointer and checks for a match.

So yeah, prev_cpu is needed.  The unlock ordering OTOH is irrelevant so 
you don't need to visit the xarray backwards.

Paolo