From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46124) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afiTy-0006mA-GN for qemu-devel@nongnu.org; Tue, 15 Mar 2016 02:29:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afiTw-0006OY-Es for qemu-devel@nongnu.org; Tue, 15 Mar 2016 02:29:30 -0400 Date: Tue, 15 Mar 2016 17:20:39 +1100 From: David Gibson Message-ID: <20160315062039.GN15272@voom.fritz.box> References: <1457672078-17307-1-git-send-email-bharata@linux.vnet.ibm.com> <1457672078-17307-4-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mI5TpPl/qDRoXxBg" Content-Disposition: inline In-Reply-To: <1457672078-17307-4-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v2 3/9] cpu: Reclaim vCPU objects List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mjrosato@linux.vnet.ibm.com, agraf@suse.de, thuth@redhat.com, pkrempa@redhat.com, Zhu Guihua , ehabkost@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org, armbru@redhat.com, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, Chen Fan , pbonzini@redhat.com, Gu Zheng , imammedo@redhat.com, afaerber@suse.de, mdroth@linux.vnet.ibm.com --mI5TpPl/qDRoXxBg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 11, 2016 at 10:24:32AM +0530, Bharata B Rao wrote: > From: Gu Zheng >=20 > In order to deal well with the kvm vcpus (which can not be removed withou= t any > protection), we do not close KVM vcpu fd, just record and mark it as stop= ped > into a list, so that we can reuse it for the appending cpu hot-add reques= t if > possible. It is also the approach that kvm guys suggested: > https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html >=20 > Signed-off-by: Chen Fan > Signed-off-by: Gu Zheng > Signed-off-by: Zhu Guihua > Signed-off-by: Bharata B Rao > [- Explicit CPU_REMOVE() from qemu_kvm/tcg_destroy_vcpu() > isn't needed as it is done from cpu_exec_exit() > - Use iothread mutex instead of global mutex during > destroy > - Don't cleanup vCPU object from vCPU thread context > but leave it to the callers (device_add/device_del)] Reviewed-by: David Gibson > --- > cpus.c | 39 +++++++++++++++++++++++++++++++++-- > include/qom/cpu.h | 10 +++++++++ > include/sysemu/kvm.h | 1 + > kvm-all.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++= +++++- > kvm-stub.c | 5 +++++ > 5 files changed, 109 insertions(+), 3 deletions(-) >=20 > diff --git a/cpus.c b/cpus.c > index bc774e2..be0ac6a 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -953,6 +953,18 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(vo= id *data), void *data) > qemu_cpu_kick(cpu); > } > =20 > +static void qemu_kvm_destroy_vcpu(CPUState *cpu) > +{ > + if (kvm_destroy_vcpu(cpu) < 0) { > + error_report("kvm_destroy_vcpu failed"); > + exit(EXIT_FAILURE); > + } > +} > + > +static void qemu_tcg_destroy_vcpu(CPUState *cpu) > +{ > +} > + > static void flush_queued_work(CPUState *cpu) > { > struct qemu_work_item *wi; > @@ -1045,7 +1057,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) > cpu->created =3D true; > qemu_cond_signal(&qemu_cpu_cond); > =20 > - while (1) { > + do { > if (cpu_can_run(cpu)) { > r =3D kvm_cpu_exec(cpu); > if (r =3D=3D EXCP_DEBUG) { > @@ -1053,8 +1065,10 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) > } > } > qemu_kvm_wait_io_event(cpu); > - } > + } while (!cpu->unplug || cpu_can_run(cpu)); > =20 > + qemu_kvm_destroy_vcpu(cpu); > + qemu_mutex_unlock_iothread(); > return NULL; > } > =20 > @@ -1108,6 +1122,7 @@ static void tcg_exec_all(void); > static void *qemu_tcg_cpu_thread_fn(void *arg) > { > CPUState *cpu =3D arg; > + CPUState *remove_cpu =3D NULL; > =20 > rcu_register_thread(); > =20 > @@ -1145,6 +1160,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) > } > } > qemu_tcg_wait_io_event(QTAILQ_FIRST(&cpus)); > + CPU_FOREACH(cpu) { > + if (cpu->unplug && !cpu_can_run(cpu)) { > + remove_cpu =3D cpu; > + break; > + } > + } > + if (remove_cpu) { > + qemu_tcg_destroy_vcpu(remove_cpu); > + remove_cpu =3D NULL; > + } > } > =20 > return NULL; > @@ -1301,6 +1326,13 @@ void resume_all_vcpus(void) > } > } > =20 > +void cpu_remove(CPUState *cpu) > +{ > + cpu->stop =3D true; > + cpu->unplug =3D true; > + qemu_cpu_kick(cpu); > +} > + > /* For temporary buffers for forming a name */ > #define VCPU_THREAD_NAME_SIZE 16 > =20 > @@ -1517,6 +1549,9 @@ static void tcg_exec_all(void) > break; > } > } else if (cpu->stop || cpu->stopped) { > + if (cpu->unplug) { > + next_cpu =3D CPU_NEXT(cpu); > + } > break; > } > } > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 7052eee..0720dd7 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -237,6 +237,7 @@ struct kvm_run; > * @halted: Nonzero if the CPU is in suspended state. > * @stop: Indicates a pending stop request. > * @stopped: Indicates the CPU has been artificially stopped. > + * @unplug: Indicates a pending CPU unplug request. > * @crash_occurred: Indicates the OS reported a crash (panic) for this C= PU > * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this > * CPU and return to its top level loop. > @@ -289,6 +290,7 @@ struct CPUState { > bool created; > bool stop; > bool stopped; > + bool unplug; > bool crash_occurred; > bool exit_request; > uint32_t interrupt_request; > @@ -756,6 +758,14 @@ void cpu_exit(CPUState *cpu); > void cpu_resume(CPUState *cpu); > =20 > /** > + * cpu_remove: > + * @cpu: The CPU to remove. > + * > + * Requests the CPU to be removed. > + */ > +void cpu_remove(CPUState *cpu); > + > +/** > * qemu_init_vcpu: > * @cpu: The vCPU to initialize. > * > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 6695fa7..5d5b602 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -216,6 +216,7 @@ int kvm_has_intx_set_mask(void); > =20 > int kvm_init_vcpu(CPUState *cpu); > int kvm_cpu_exec(CPUState *cpu); > +int kvm_destroy_vcpu(CPUState *cpu); > =20 > #ifdef NEED_CPU_H > =20 > diff --git a/kvm-all.c b/kvm-all.c > index 44c0464..35c0621 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -61,6 +61,12 @@ > =20 > #define KVM_MSI_HASHTAB_SIZE 256 > =20 > +struct KVMParkedVcpu { > + unsigned long vcpu_id; > + int kvm_fd; > + QLIST_ENTRY(KVMParkedVcpu) node; > +}; > + > struct KVMState > { > AccelState parent_obj; > @@ -94,6 +100,7 @@ struct KVMState > QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SI= ZE]; > #endif > KVMMemoryListener memory_listener; > + QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus; > }; > =20 > KVMState *kvm_state; > @@ -237,6 +244,53 @@ static int kvm_set_user_memory_region(KVMMemoryListe= ner *kml, KVMSlot *slot) > return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem); > } > =20 > +int kvm_destroy_vcpu(CPUState *cpu) > +{ > + KVMState *s =3D kvm_state; > + long mmap_size; > + struct KVMParkedVcpu *vcpu =3D NULL; > + int ret =3D 0; > + > + DPRINTF("kvm_destroy_vcpu\n"); > + > + mmap_size =3D kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0); > + if (mmap_size < 0) { > + ret =3D mmap_size; > + DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n"); > + goto err; > + } > + > + ret =3D munmap(cpu->kvm_run, mmap_size); > + if (ret < 0) { > + goto err; > + } > + > + vcpu =3D g_malloc0(sizeof(*vcpu)); > + vcpu->vcpu_id =3D kvm_arch_vcpu_id(cpu); > + vcpu->kvm_fd =3D 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 =3D=3D vcpu_id) { > + int kvm_fd; > + > + QLIST_REMOVE(cpu, node); > + kvm_fd =3D 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 =3D kvm_state; > @@ -245,7 +299,7 @@ int kvm_init_vcpu(CPUState *cpu) > =20 > DPRINTF("kvm_init_vcpu\n"); > =20 > - ret =3D kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cp= u)); > + ret =3D kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu)); > if (ret < 0) { > DPRINTF("kvm_create_vcpu failed\n"); > goto err; > @@ -1495,6 +1549,7 @@ static int kvm_init(MachineState *ms) > #ifdef KVM_CAP_SET_GUEST_DEBUG > QTAILQ_INIT(&s->kvm_sw_breakpoints); > #endif > + QLIST_INIT(&s->kvm_parked_vcpus); > s->vmfd =3D -1; > s->fd =3D qemu_open("/dev/kvm", O_RDWR); > if (s->fd =3D=3D -1) { > diff --git a/kvm-stub.c b/kvm-stub.c > index b962b24..61f9d5c 100644 > --- a/kvm-stub.c > +++ b/kvm-stub.c > @@ -33,6 +33,11 @@ bool kvm_allowed; > bool kvm_readonly_mem_allowed; > bool kvm_ioeventfd_any_length_allowed; > =20 > +int kvm_destroy_vcpu(CPUState *cpu) > +{ > + return -ENOSYS; > +} > + > int kvm_init_vcpu(CPUState *cpu) > { > return -ENOSYS; --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --mI5TpPl/qDRoXxBg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW56m3AAoJEGw4ysog2bOSJqgQALJqGMie9AF/lYmwxlPZws8L 9Aif7xtZK9hPVr6Gv+RInbKQVPJravD5nhniQ/BZcsVtSAOWPBjn7MJ4wDWSJGe7 ntXka1UtMg4ZlHg8iDRaZe1Ln6S0g3dwCjMErhsQ0womQx7k9YM6px7w03O9iDh/ QwdAiPoVD15WjbZ+PsWUtU02I15wYX63NWcuvifWfExcnpfSeNi5Jgo/vFpTlGUl e77pCgtRjJ6YJwoJwfIbNyzymhqMMrF4EP0YMKEES22oop2oGa8aZm9M4vsLpqeT ZnoHJLpUpB3oTr0E83I3a6GOUx9MU04Hv2RMsVoiod71uDbkUS738DcibBYcxqln r4vkhLLSxmNQIE/wM+TJFh1gWgtURDZtCra+UB56Hj56PSTYc+eKaOYLjLrdpgVK n4OgRYL2PQUGbg5p2UgH7qkvliJdHqKyEouJXvb/s18x7NOY+XwsuKARudZNYyby VNvVLa+k0cpqgXtc6jpb1zkPckf3S9PkCxWYfNqj0coPoFnj9LZbHCmIveB9sy/+ HI2soW2FYzhuTdrZGq7s06YISNO9jArpwBfCOIcQz3BPin8lfsnsOedoePm85xmk //fXSdaFe2otERMkJlWPuZRg3QLnL+x3u37vy4L2wEG7YzG9KF8Hyxpwsiou/0rh 4gKh/vy13juVO6/o5aVp =gB6S -----END PGP SIGNATURE----- --mI5TpPl/qDRoXxBg--