Hi Vitaly, On 2019/10/22 19:36, Vitaly Kuznetsov wrote:
Zhenzhong Duan<zhenzhong.d...@oracle.com> writes:
...snip
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c index 249f14a..3945aa5 100644 --- a/arch/x86/kernel/kvm.c +++ b/arch/x86/kernel/kvm.c @@ -825,18 +825,36 @@ __visible bool __kvm_vcpu_is_preempted(long cpu) */ void __init kvm_spinlock_init(void) { - /* Does host kernel support KVM_FEATURE_PV_UNHALT? */ - if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)) + /* + * In case host doesn't support KVM_FEATURE_PV_UNHALT there is still an + * advantage of keeping virt_spin_lock_key enabled: virt_spin_lock() is + * preferred over native qspinlock when vCPU is preempted. + */ + if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)) { + pr_info("PV spinlocks disabled, no host support.\n"); return; + }+ /*+ * Disable PV qspinlock and use native qspinlock when dedicated pCPUs + * are available. + */ if (kvm_para_has_hint(KVM_HINTS_REALTIME)) { - static_branch_disable(&virt_spin_lock_key); - return; + pr_info("PV spinlocks disabled with KVM_HINTS_REALTIME hints.\n"); + goto out; }- /* Don't use the pvqspinlock code if there is only 1 vCPU. */- if (num_possible_cpus() == 1) - return; + if (num_possible_cpus() == 1) { + pr_info("PV spinlocks disabled, single CPU.\n"); + goto out; + } + + if (nopvspin) { + pr_info("PV spinlocks disabled, forced by \"nopvspin\" parameter.\n"); + goto out; + } + + pr_info("PV spinlocks enabled\n");__pv_init_lock_hash();pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath; @@ -849,6 +867,8 @@ void __init kvm_spinlock_init(void) pv_ops.lock.vcpu_is_preempted = PV_CALLEE_SAVE(__kvm_vcpu_is_preempted); } +out: + static_branch_disable(&virt_spin_lock_key);You probably need to add 'return' before 'out:' as it seems you're disabling virt_spin_lock_key in all cases now).
virt_spin_lock_key is kept enabled in !kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) case which is the only case virt_spin_lock() optimization is used. When PV qspinlock is enabled, virt_spin_lock() isn't called in __pv_queued_spin_lock_slowpath() in which case we don't care virt_spin_lock_key's value. So adding 'return' or not are both ok, I chosed to save a line, let me know if you prefer to add a 'return' and I'll change it. btw: __pv_queued_spin_lock_slowpath() is alias of queued_spin_lock_slowpath() Thanks Zhenzhong
