Hi Peter,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
commit: 0c2c7c069285374fc8feacddc0498f8ab7627117 KVM: SEV: Mark nested locking of vcpu->lock
date: 5 weeks ago
config: x86_64-buildonly-randconfig-r005-20220613 (https://download.01.org/0day-ci/archive/20220613/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d378268ead93c85803c270277f0243737b536ae7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c2c7c069285374fc8feacddc0498f8ab7627117
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 0c2c7c069285374fc8feacddc0498f8ab7627117
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/kvm/ drivers/media/platform/qcom/camss/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> arch/x86/kvm/svm/sev.c:1605:30: warning: parameter 'role' set but not used [-Wunused-but-set-parameter]
enum sev_migration_role role)
^
1 warning generated.
vim +/role +1605 arch/x86/kvm/svm/sev.c
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
1615 if (first) {
1616 /*
1617 * Reset the role to one that avoids colliding with
1618 * the role used for the first vcpu mutex.
1619 */
1620 role = SEV_NR_MIGRATION_ROLES;
1621 first = false;
1622 } else {
1623 mutex_release(&vcpu->mutex.dep_map, _THIS_IP_);
1624 }
1625 }
1626
1627 return 0;
1628
1629 out_unlock:
1630
1631 first = true;
1632 kvm_for_each_vcpu(j, vcpu, kvm) {
1633 if (i == j)
1634 break;
1635
1636 if (first)
1637 first = false;
1638 else
1639 mutex_acquire(&vcpu->mutex.dep_map, role, 0, _THIS_IP_);
1640
1641
1642 mutex_unlock(&vcpu->mutex);
1643 }
1644 return -EINTR;
1645 }
1646
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Mon, Jun 13, 2022 at 5:03 AM kernel test robot <[email protected]> wrote:
>
> Hi Peter,
>
> FYI, the error/warning still remains.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
> commit: 0c2c7c069285374fc8feacddc0498f8ab7627117 KVM: SEV: Mark nested locking of vcpu->lock
> date: 5 weeks ago
> config: x86_64-buildonly-randconfig-r005-20220613 (https://download.01.org/0day-ci/archive/20220613/[email protected]/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d378268ead93c85803c270277f0243737b536ae7)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c2c7c069285374fc8feacddc0498f8ab7627117
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout 0c2c7c069285374fc8feacddc0498f8ab7627117
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/kvm/ drivers/media/platform/qcom/camss/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> arch/x86/kvm/svm/sev.c:1605:30: warning: parameter 'role' set but not used [-Wunused-but-set-parameter]
> enum sev_migration_role role)
> ^
> 1 warning generated.
>
>
> vim +/role +1605 arch/x86/kvm/svm/sev.c
>
> 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?
On Mon, Jun 13, 2022, Peter Gonda wrote:
> On Mon, Jun 13, 2022 at 5:03 AM kernel test robot <[email protected]> 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
--