lkml.org 
[lkml]   [2022]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: arch/x86/kvm/svm/sev.c:1605:30: warning: parameter 'role' set but not used
On Mon, Jun 13, 2022, Peter Gonda wrote:
> On Mon, Jun 13, 2022 at 5:03 AM kernel test robot <lkp@intel.com> wrote:
> > 1603
> > 1604 static int sev_lock_vcpus_for_migration(struct kvm *kvm,
> > > 1605 enum sev_migration_role role)
> > 1606 {
> > 1607 struct kvm_vcpu *vcpu;
> > 1608 unsigned long i, j;
> > 1609 bool first = true;
> > 1610
> > 1611 kvm_for_each_vcpu(i, vcpu, kvm) {
> > 1612 if (mutex_lock_killable_nested(&vcpu->mutex, role))
> > 1613 goto out_unlock;
> > 1614
> >
>
> I am confused about this warning. |role| is used on this line above.
> Is this because CONFIG_DEBUG_LOCK_ALLOC the subclass argument is
> dropped in the macro?

Yep, at that point the compiler can easily detect that there's no actual usage of
the parameter.

There's no need for the "first" variable, it's the same as "i == 0" and "j == 0".
With that out of the way, the role and mutex_release/mutex_acquire shenanigans can
be wrapped with ifdeffery. Not the prettiest thing, but I actually like the #ifdefs
because they effectively document that KVM is playing games to make lockdep happy.

I'll send this as a formal patch, I think it'll make clang-15 happy.

---
arch/x86/kvm/svm/sev.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 51fd985cf21d..593c61683484 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1606,38 +1606,35 @@ static int sev_lock_vcpus_for_migration(struct kvm *kvm,
{
struct kvm_vcpu *vcpu;
unsigned long i, j;
- bool first = true;

kvm_for_each_vcpu(i, vcpu, kvm) {
if (mutex_lock_killable_nested(&vcpu->mutex, role))
goto out_unlock;

- if (first) {
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ if (!i)
/*
* Reset the role to one that avoids colliding with
* the role used for the first vcpu mutex.
*/
role = SEV_NR_MIGRATION_ROLES;
- first = false;
- } else {
+ else
mutex_release(&vcpu->mutex.dep_map, _THIS_IP_);
- }
+#endif
}

return 0;

out_unlock:

- first = true;
kvm_for_each_vcpu(j, vcpu, kvm) {
if (i == j)
break;

- if (first)
- first = false;
- else
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ if (j)
mutex_acquire(&vcpu->mutex.dep_map, role, 0, _THIS_IP_);
-
+#endif

mutex_unlock(&vcpu->mutex);
}
base-commit: 8a6d3a6ec6a821c5ddb3972fdbad9c4149eabf1e
--


\
 
 \ /
  Last update: 2022-06-13 20:53    [W:0.037 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site