lkml.org 
[lkml]   [2022]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2 06/19] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock
    Date
    From: Isaku Yamahata <isaku.yamahata@intel.com>

    Because kvm_count_lock unnecessarily complicates the KVM locking convention
    Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock for
    simplicity.

    Opportunistically add some comments on locking.

    Suggested-by: Sean Christopherson <seanjc@google.com>
    Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
    ---
    Documentation/virt/kvm/locking.rst | 14 +++++-------
    virt/kvm/kvm_main.c | 36 +++++++++++++++++++++---------
    2 files changed, 30 insertions(+), 20 deletions(-)

    diff --git a/Documentation/virt/kvm/locking.rst b/Documentation/virt/kvm/locking.rst
    index 845a561629f1..8957e32aa724 100644
    --- a/Documentation/virt/kvm/locking.rst
    +++ b/Documentation/virt/kvm/locking.rst
    @@ -216,15 +216,11 @@ time it will be set using the Dirty tracking mechanism described above.
    :Type: mutex
    :Arch: any
    :Protects: - vm_list
    -
    -``kvm_count_lock``
    -^^^^^^^^^^^^^^^^^^
    -
    -:Type: raw_spinlock_t
    -:Arch: any
    -:Protects: - hardware virtualization enable/disable
    -:Comment: 'raw' because hardware enabling/disabling must be atomic /wrt
    - migration.
    + - kvm_usage_count
    + - hardware virtualization enable/disable
    +:Comment: Use cpus_read_lock() for hardware virtualization enable/disable
    + because hardware enabling/disabling must be atomic /wrt
    + migration. The lock order is cpus lock => kvm_lock.

    ``kvm->mn_invalidate_lock``
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
    index 6ce6f27f2934..606ac6bb67d0 100644
    --- a/virt/kvm/kvm_main.c
    +++ b/virt/kvm/kvm_main.c
    @@ -100,7 +100,6 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink);
    */

    DEFINE_MUTEX(kvm_lock);
    -static DEFINE_RAW_SPINLOCK(kvm_count_lock);
    LIST_HEAD(vm_list);

    static cpumask_var_t cpus_hardware_enabled;
    @@ -4996,6 +4995,8 @@ static void hardware_enable_nolock(void *junk)
    int cpu = raw_smp_processor_id();
    int r;

    + WARN_ON_ONCE(preemptible());
    +
    if (cpumask_test_cpu(cpu, cpus_hardware_enabled))
    return;

    @@ -5014,7 +5015,7 @@ static int kvm_online_cpu(unsigned int cpu)
    {
    int ret = 0;

    - raw_spin_lock(&kvm_count_lock);
    + mutex_lock(&kvm_lock);
    /*
    * Abort the CPU online process if hardware virtualization cannot
    * be enabled. Otherwise running VMs would encounter unrecoverable
    @@ -5029,7 +5030,7 @@ static int kvm_online_cpu(unsigned int cpu)
    ret = -EIO;
    }
    }
    - raw_spin_unlock(&kvm_count_lock);
    + mutex_unlock(&kvm_lock);
    return ret;
    }

    @@ -5037,6 +5038,8 @@ static void hardware_disable_nolock(void *junk)
    {
    int cpu = raw_smp_processor_id();

    + WARN_ON_ONCE(preemptible());
    +
    if (!cpumask_test_cpu(cpu, cpus_hardware_enabled))
    return;
    cpumask_clear_cpu(cpu, cpus_hardware_enabled);
    @@ -5045,10 +5048,10 @@ static void hardware_disable_nolock(void *junk)

    static int kvm_offline_cpu(unsigned int cpu)
    {
    - raw_spin_lock(&kvm_count_lock);
    + mutex_lock(&kvm_lock);
    if (kvm_usage_count)
    hardware_disable_nolock(NULL);
    - raw_spin_unlock(&kvm_count_lock);
    + mutex_unlock(&kvm_lock);
    return 0;
    }

    @@ -5063,16 +5066,19 @@ static void hardware_disable_all_nolock(void)

    static void hardware_disable_all(void)
    {
    - raw_spin_lock(&kvm_count_lock);
    + cpus_read_lock();
    + mutex_lock(&kvm_lock);
    hardware_disable_all_nolock();
    - raw_spin_unlock(&kvm_count_lock);
    + mutex_unlock(&kvm_lock);
    + cpus_read_unlock();
    }

    static int hardware_enable_all(void)
    {
    int r = 0;

    - raw_spin_lock(&kvm_count_lock);
    + cpus_read_lock();
    + mutex_lock(&kvm_lock);

    kvm_usage_count++;
    if (kvm_usage_count == 1) {
    @@ -5085,7 +5091,8 @@ static int hardware_enable_all(void)
    }
    }

    - raw_spin_unlock(&kvm_count_lock);
    + mutex_unlock(&kvm_lock);
    + cpus_read_unlock();

    return r;
    }
    @@ -5691,15 +5698,22 @@ static void kvm_init_debug(void)

    static int kvm_suspend(void)
    {
    - if (kvm_usage_count)
    + /*
    + * The caller ensures that CPU hotlug is disabled by
    + * cpu_hotplug_disable() and other CPUs are offlined. No need for
    + * locking.
    + */
    + if (kvm_usage_count) {
    + lockdep_assert_not_held(&kvm_lock);
    hardware_disable_nolock(NULL);
    + }
    return 0;
    }

    static void kvm_resume(void)
    {
    if (kvm_usage_count) {
    - lockdep_assert_not_held(&kvm_count_lock);
    + lockdep_assert_not_held(&kvm_lock);
    hardware_enable_nolock(NULL);
    }
    }
    --
    2.25.1
    \
     
     \ /
      Last update: 2022-08-30 14:03    [W:2.626 / U:0.232 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site