On 2024-06-25 at 16:42:11 +0300, Nikolay Borisov wrote:
>
>
> On 25.06.24 г. 15:54 ч., Chen Yu wrote:
> > The kernel can change spinlock behavior when running as a guest. But
> > this guest-friendly behavior causes performance problems on bare metal.
> > So there's a 'virt_spin_lock_key' static key to switch between the two
> > modes.
> >
> > The static key is always enabled by default (run in guest mode) and
> > should be disabled for bare metal (and in some guests that want native
> > behavior).
> >
> > Performance drop is reported when running encode/decode workload and
> > BenchSEE cache sub-workload.
> > Bisect points to commit ce0a1b608bfc ("x86/paravirt: Silence unused
> > native_pv_lock_init() function warning"). When CONFIG_PARAVIRT_SPINLOCKS
> > is disabled the virt_spin_lock_key is incorrectly set to true on bare
> > metal. The qspinlock degenerates to test-and-set spinlock, which
> > decrease the performance on bare metal.
> >
> > Set the default value of virt_spin_lock_key to false. If booting in a VM,
> > enable this key. Later during the VM initialization, if other
> > high-efficient spinlock is preferred(paravirt-spinlock eg), the
> > virt_spin_lock_key is disabled accordingly. The relation is described as
> > below:
> >
> > X86_FEATURE_HYPERVISOR Y Y Y N
> > CONFIG_PARAVIRT_SPINLOCKS Y Y N Y/N
> > PV spinlock Y N N Y/N
> >
> > virt_spin_lock_key N N Y N
> >
> > -DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
> > +DEFINE_STATIC_KEY_FALSE(virt_spin_lock_key);
> > void __init native_pv_lock_init(void)
> > {
> > - if (IS_ENABLED(CONFIG_PARAVIRT_SPINLOCKS) &&
>
> Actually now shouldn't the CONFIG_PARAVIRT_SPINLOCKS check be retained?
> Otherwise we'll have the virtspinlock enabled even if we are a guest but
> CONFIG_PARAVIRT_SPINLOCKS is disabled, no ?
>
It seems to be the expected behavior? If CONFIG_PARAVIRT_SPINLOCKS is disabled,
should the virt_spin_lock_key be enabled in the guest?
The previous behavior before commit ce0a1b608bfc ("x86/paravirt: Silence unused
native_pv_lock_init() function warning"): kvm_spinlock_init() is NULL if
CONFIG_PARAVIRT_SPINLOCKS is disabled, and static_branch_disable(&virt_spin_lock_key)
can not be invoked, so the virt_spin_lock_key keeps enabled.
thanks,
Chenyu
