Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects

[Gu Zheng <guz.fnst@cn.fujitsu.com>]

Hi Igor,

On 09/09/2014 10:40 PM, Igor Mammedov wrote:

> On Thu, 28 Aug 2014 11:36:42 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> 
>> After ACPI get a signal to eject a vCPU, the vCPU must be
>> removed from CPU list，before the vCPU really removed,  then
>> release the all related vCPU objects.
>> But we do not close KVM vcpu fd, just record it into a list, in
>> order to reuse it.
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>>  cpus.c               |   37 ++++++++++++++++++++++++++++++++
>>  include/sysemu/kvm.h |    1 +
>>  kvm-all.c            |   57 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 94 insertions(+), 1 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index eee693b..0608b41 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -851,6 +851,24 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
>>      qemu_cpu_kick(cpu);
>>  }
>>  
>> +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
>> +{
>> +    CPU_REMOVE(cpu);
>> +
>> +    if (kvm_destroy_vcpu(cpu) < 0) {
>> +        fprintf(stderr, "kvm_destroy_vcpu failed.\n");
>> +        exit(1);
>> +    }
>> +
>> +    object_unparent(OBJECT(cpu));
>> +}
>> +
>> +static void qemu_tcg_destroy_vcpu(CPUState *cpu)
>> +{
>> +    CPU_REMOVE(cpu);
>> +    object_unparent(OBJECT(cpu));
>> +}
>> +
>>  static void flush_queued_work(CPUState *cpu)
>>  {
>>      struct qemu_work_item *wi;
>> @@ -942,6 +960,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>>              }
>>          }
>>          qemu_kvm_wait_io_event(cpu);
>> +        if (cpu->exit && !cpu_can_run(cpu)) {
>> +            qemu_kvm_destroy_vcpu(cpu);
>> +            qemu_mutex_unlock(&qemu_global_mutex);
>> +            return NULL;
>> +        }
>>      }
>>  
>>      return NULL;
>> @@ -994,6 +1017,7 @@ static void tcg_exec_all(void);
>>  static void *qemu_tcg_cpu_thread_fn(void *arg)
>>  {
>>      CPUState *cpu = arg;
>> +    CPUState *remove_cpu = NULL;
>>  
>>      qemu_tcg_init_cpu_signals();
>>      qemu_thread_get_self(cpu->thread);
>> @@ -1026,6 +1050,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>>              }
>>          }
>>          qemu_tcg_wait_io_event();
>> +        CPU_FOREACH(cpu) {
>> +            if (cpu->exit && !cpu_can_run(cpu)) {
>> +                remove_cpu = cpu;
>> +                break;
>> +            }
>> +        }
>> +        if (remove_cpu) {
>> +            qemu_tcg_destroy_vcpu(remove_cpu);
>> +            remove_cpu = NULL;
>> +        }
>>      }
>>  
>>      return NULL;
>> @@ -1383,6 +1417,9 @@ static void tcg_exec_all(void)
>>                  break;
>>              }
>>          } else if (cpu->stop || cpu->stopped) {
>> +            if (cpu->exit) {
>> +                next_cpu = CPU_NEXT(cpu);
>> +            }
>>              break;
>>          }
>>      }
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index 174ea36..88e2403 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -178,6 +178,7 @@ int kvm_has_intx_set_mask(void);
>>  
>>  int kvm_init_vcpu(CPUState *cpu);
>>  int kvm_cpu_exec(CPUState *cpu);
>> +int kvm_destroy_vcpu(CPUState *cpu);
>>  
>>  #ifdef NEED_CPU_H
>>  
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 1402f4f..d0caeff 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -74,6 +74,12 @@ typedef struct KVMSlot
>>  
>>  typedef struct kvm_dirty_log KVMDirtyLog;
>>  
>> +struct KVMParkedVcpu {
>> +    unsigned long vcpu_id;
>> +    int kvm_fd;
>> +    QLIST_ENTRY(KVMParkedVcpu) node;
>> +};
>> +
>>  struct KVMState
>>  {
>>      KVMSlot *slots;
>> @@ -108,6 +114,7 @@ struct KVMState
>>      QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
>>      bool direct_msi;
>>  #endif
>> +    QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
>>  };
>>  
>>  KVMState *kvm_state;
>> @@ -226,6 +233,53 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>>      return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
>>  }
>>  
>> +int kvm_destroy_vcpu(CPUState *cpu)
>> +{
>> +    KVMState *s = kvm_state;
>> +    long mmap_size;
>> +    struct KVMParkedVcpu *vcpu = NULL;
>> +    int ret = 0;
>> +
>> +    DPRINTF("kvm_destroy_vcpu\n");
> maybe it would be good to put cpu in RESET state before parking it?

It would be better.

> 
> Do we need a kernel patch to park VCPU so it wouldn't be possible
> to wake it up by sending INIT/SIPI to it?

We sent a kernel patch before, but things is harder than we think, and
it will lead to performance regression. So the suggestion is just parking
vcpu in the QEMU side.

Thanks,
Gu

> 
>> +
>> +    mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
>> +    if (mmap_size < 0) {
>> +        ret = mmap_size;
>> +        DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
>> +        goto err;
>> +    }
>> +
>> +    ret = munmap(cpu->kvm_run, mmap_size);
>> +    if (ret < 0) {
>> +        goto err;
>> +    }
>> +
>> +    vcpu = g_malloc0(sizeof(*vcpu));
>> +    vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
>> +    vcpu->kvm_fd = cpu->kvm_fd;
>> +    QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
>> +err:
>> +    return ret;
>> +}
>> +
>> +static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
>> +{
>> +    struct KVMParkedVcpu *cpu;
>> +
>> +    QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) {
>> +        if (cpu->vcpu_id == vcpu_id) {
>> +            int kvm_fd;
>> +
>> +            QLIST_REMOVE(cpu, node);
>> +            kvm_fd = cpu->kvm_fd;
>> +            g_free(cpu);
>> +            return kvm_fd;
>> +        }
>> +    }
>> +
>> +    return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
>> +}
>> +
>>  int kvm_init_vcpu(CPUState *cpu)
>>  {
>>      KVMState *s = kvm_state;
>> @@ -234,7 +288,7 @@ int kvm_init_vcpu(CPUState *cpu)
>>  
>>      DPRINTF("kvm_init_vcpu\n");
>>  
>> -    ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cpu));
>> +    ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
>>      if (ret < 0) {
>>          DPRINTF("kvm_create_vcpu failed\n");
>>          goto err;
>> @@ -1404,6 +1458,7 @@ int kvm_init(MachineClass *mc)
>>  #ifdef KVM_CAP_SET_GUEST_DEBUG
>>      QTAILQ_INIT(&s->kvm_sw_breakpoints);
>>  #endif
>> +    QLIST_INIT(&s->kvm_parked_vcpus);
>>      s->vmfd = -1;
>>      s->fd = qemu_open("/dev/kvm", O_RDWR);
>>      if (s->fd == -1) {
> 
> .
>