From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37438) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHRmX-0002Ol-89 for qemu-devel@nongnu.org; Sat, 31 Jan 2015 01:43:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YHRmV-0005BH-0N for qemu-devel@nongnu.org; Sat, 31 Jan 2015 01:43:49 -0500 Received: from ozlabs.org ([2401:3900:2:1::2]:59784) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHRmU-0005Ak-L8 for qemu-devel@nongnu.org; Sat, 31 Jan 2015 01:43:46 -0500 Date: Sat, 31 Jan 2015 11:21:29 +1100 From: David Gibson Message-ID: <20150131002129.GD28703@voom.fritz.box> References: <1420697420-16053-1-git-send-email-bharata@linux.vnet.ibm.com> <1420697420-16053-11-git-send-email-bharata@linux.vnet.ibm.com> <20150129014839.GT14681@voom> <20150130082346.GF24041@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zbGR4y+acU1DwHSi" Content-Disposition: inline In-Reply-To: <20150130082346.GF24041@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v1 10/13] cpus, spapr: reclaim allocated vCPU objects List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: Gu Zheng , imammedo@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com --zbGR4y+acU1DwHSi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 30, 2015 at 01:53:47PM +0530, Bharata B Rao wrote: > On Thu, Jan 29, 2015 at 12:48:39PM +1100, David Gibson wrote: > > On Thu, Jan 08, 2015 at 11:40:17AM +0530, Bharata B Rao wrote: > > > From: Gu Zheng > >=20 > > This needs a commit message, it's not at all clear from the 1-line desc= ription. >=20 > Borrowed patch, but I should have put in a description. >=20 > > > diff --git a/kvm-all.c b/kvm-all.c > > > index 18cc6b4..6f543ce 100644 > > > --- a/kvm-all.c > > > +++ b/kvm-all.c > > > @@ -71,6 +71,12 @@ typedef struct KVMSlot > > > =20 > > > typedef struct kvm_dirty_log KVMDirtyLog; > > > =20 > > > +struct KVMParkedVcpu { > > > + unsigned long vcpu_id; > > > + int kvm_fd; > > > + QLIST_ENTRY(KVMParkedVcpu) node; > > > +}; > > > + > > > struct KVMState > > > { > > > AccelState parent_obj; > > > @@ -107,6 +113,7 @@ struct KVMState > > > QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTA= B_SIZE]; > > > bool direct_msi; > > > #endif > > > + QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus; > > > }; > > > =20 > > > #define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm") > > > @@ -247,6 +254,53 @@ static int kvm_set_user_memory_region(KVMState *= s, 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_destroy_vcpu 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); > >=20 > > What's the reason for parking vcpus rather than removing / recreating > > them at the kvm level? >=20 > Since KVM isn't equipped to handle closure of vcpu fd from userspace(QEMU) > correctly, certain work arounds have to be employed to allow reuse of > vcpu array slot in KVM during cpu hot plug/unplug from guest. One such > proposed workaround is to park the vcpu fd in userspace during cpu unplug > and reuse it later during next hotplug. >=20 > Some details can be found here: > KVM: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html > QEMU: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00859.html Ok, that makes sense but it definitely needs comment, both in the code and in the commit message. --=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 --zbGR4y+acU1DwHSi Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUzCAJAAoJEGw4ysog2bOSpkEP/3d0oopK2JufcT17pR5m55s7 H8asPYTucgWEpBptLVnd5nh7icyawoFitFUU9OpcQLXhrxue+9+Bc/oMXNSHJsjT Gm8yeA1BTACk+QW2F0l5crDmc9z9seF+9eBy5iNiqoIGyy2/4RaC56KgnSLWqL+H NFg0v4tjL+9PklrvAHGh7zZLQBkT9SCAvhRSQATzg9hh3fwzVflFixljdteWEYDb DGlXxyqVrhPhMe1StQmsyDqDXchuiKUMG7osMGBPDUiM5glnY9mQhY0eNVHOcgBJ Bo63NTNmdD4n6DbirkOM7iQpgiOAhQeLzYFJi7d5lGvSgLTdRwvYH0ktauW0swok Wu1Qb1vpFRrLXU0XJC0TZkN/yN4FuqvrJX40xpwyyokoh97DF9YHiKUxsYdzfUuq eLcz680ZlbI3V4/szJjSEwp4K55Lnj+iW+tEKx+Zz4qNMg2IKLdoUITLQmjtkCCz IawskwmbPy7RZOLxE/9hx7S0ZxO8YJwESDuaYRg8+thWSZ02wGvzY5RmVQUrBoKX Gzym30Xm76ewhcEcVAEgBDD2DNgHswDXP37sG6Ejb50cvyaZcNFoo7TdSLBEYeKv /UIwPPWrrsnLz+YGmgdWuMPPNZgOm2oDjZRB5nGhKrUZTUnIukno9Y/2KEROzWIW uo+PbXpduPGLq1gP/bfW =Xkda -----END PGP SIGNATURE----- --zbGR4y+acU1DwHSi--