[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 7/7] cpus: reclaim allocated vCPU objects
From: |
Anshul Makkar |
Subject: |
Re: [Qemu-devel] [RFC PATCH 7/7] cpus: reclaim allocated vCPU objects |
Date: |
Fri, 1 Aug 2014 17:34:54 +0200 |
Hi Gu,
Thanks for clarifying.
Ah I missed that bit of the patch. Sorry about that and for making noise.
Yes, now cpu-hotplug and unplug works fine. Next week I plan to run a
series of automated and stress test. Will keep the group posted about
the results.
Thanks
Anshul Makkar
On Fri, Aug 1, 2014 at 6:42 AM, Gu Zheng <address@hidden> wrote:
> Hi Anshul,
> Thanks for your test.
> On 07/30/2014 10:31 PM, Anshul Makkar wrote:
>
>> Hi,
>>
>> I am testing the cpu-hotunplug patches. I observed that after the
>> deletion of the cpu with id = x, if I cpu-add the same cpu again id =
>> x, then qemu exits with the error that file descriptor already exists.
>
> Could you please offer the whole reproduce routine? In my test box, we
> can add a removed cpu with the id.
>
>>
>> On debugging I found that if I give cpu-add <apic-id = x>, then
>> qemu_kvm_cpu_thread_fn->kvm_init_vcpu is called which sends an IOCTL
>> (KVM_CREATE_VCPU) to kvm to create a new fd. As the fd already exists
>> in KVM as we never delete the fd from the kernel and just park it in
>> separate list, it returns false and QEMU exits. In the above code
>> flow, no where its being checked if we have the cpu with cpuid = x
>> available in the parked list and we can reuse it.
>>
>> Am I missing something or this bit is yet to be implmented.
>
> Yes, it is implemented, in the same way as you mention above, please refer
> to function kvm_get_vcpu().
>
> Thanks,
> Gu
>
>>
>> Thanks
>> Anshul Makkar
>>
>> On Fri, Jul 18, 2014 at 4:09 AM, Gu Zheng <address@hidden> wrote:
>>> Hi Anshul,
>>> On 07/18/2014 12:24 AM, Anshul Makkar wrote:
>>>
>>>> Are we not going to introduce new command cpu_del for deleting the cpu ?
>>>>
>>>> I couldn't find any patch for addition of cpu_del command. Is this
>>>> intentional and we intend to use device_del (and similarly device_add)
>>>> for cpu hot(un)plug or just skipped to be added later. I have the
>>>> patch for the same which I can release, if the intent is to add this
>>>> command.
>>>
>>> The "device_add/device_del" interface is the approved way to support
>>> add/del cpu,
>>> which is also more common and elegant than "cpu_add/del".
>>> <http://wiki.qemu.org/Features/CPUHotplug>
>>> so we intend to use device_del rather than the cpu_del.
>>> And IMO, the cpu_add will be replaced by "device_add" sooner or later.
>>>
>>> Thanks,
>>> Gu
>>>
>>>>
>>>> Thanks
>>>> Anshul Makkar
>>>>
>>>> On Fri, Jul 11, 2014 at 11:59 AM, Gu Zheng <address@hidden> 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 <address@hidden>
>>>>> Signed-off-by: Gu Zheng <address@hidden>
>>>>> ---
>>>>> 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 4dfb889..9a73407 100644
>>>>> --- a/cpus.c
>>>>> +++ b/cpus.c
>>>>> @@ -786,6 +786,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;
>>>>> @@ -877,6 +895,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;
>>>>> @@ -929,6 +952,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);
>>>>> @@ -961,6 +985,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;
>>>>> @@ -1316,6 +1350,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 3ae30ee..25e2a43 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");
>>>>> +
>>>>> + 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) {
>>>>> --
>>>>> 1.7.7
>>>>>
>>>> .
>>>>
>>>
>>>
>> .
>>
>
>