From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1/3] libxc: Revert "do some retries in xc_cpupool_removecpu() for EBUSY case" Date: Fri, 15 Apr 2016 09:46:42 +0200 Message-ID: <1460706402.13871.234.camel@citrix.com> References: <1460653660-6654-1-git-send-email-ian.jackson@eu.citrix.com> <1460653660-6654-2-git-send-email-ian.jackson@eu.citrix.com> <571078F0.4010003@suse.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5856601921823737072==" Return-path: In-Reply-To: <571078F0.4010003@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Juergen Gross , Ian Jackson , xen-devel@lists.xensource.com Cc: George Dunlap , Tim Deegan , Wei Liu , Jan Beulich List-Id: xen-devel@lists.xenproject.org --===============5856601921823737072== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-QyD8CsRLixzYy5bQxMH+" --=-QyD8CsRLixzYy5bQxMH+ Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2016-04-15 at 07:15 +0200, Juergen Gross wrote: > On 14/04/16 19:07, Ian Jackson wrote: > >=20 > > libxc may be called from within long-running daemons such as > > libvirt. > >=20 > > In such a system this sleep would enable an uncooperative or buggy > > guest to block all toolstack operations for an extended period. > >=20 > > Sadly, therefore, such a retry loop is not feasible without a lot > > of > > engineering which is probably not appropriate. > I understand your concerns. OTOH you should consider that libvirt has > no support for cpupool operations today, so it won't run this code. >=20 True, and it does not even have the concept of pooling (not in a compatible way with what we provide with cpupools), so it probably won't be start supporting them anytime soon. HOWEVER, Ian's concern applies to any daemon based toolstack (i.e., not necessarily libvirt) which may want to support cpupools, and would then fall into this. > > diff --git a/tools/libxc/xc_cpupool.c b/tools/libxc/xc_cpupool.c > > index 261b9c9..c42273e 100644 > > --- a/tools/libxc/xc_cpupool.c > > +++ b/tools/libxc/xc_cpupool.c > > @@ -20,7 +20,6 @@ > >=C2=A0 > > =C2=A0int xc_cpupool_removecpu(xc_interface *xch, > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0uint32_t poolid, > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0int cpu) > > =C2=A0{ > > -=C2=A0=C2=A0=C2=A0=C2=A0unsigned retries; > > -=C2=A0=C2=A0=C2=A0=C2=A0int err; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0DECLARE_SYSCTL; > > =C2=A0 > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sysctl.cmd =3D XEN_SYSCTL_cpupool_op; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sysctl.u.cpupool_op.op =3D XEN_SYSCTL_CPU= POOL_OP_RMCPU; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sysctl.u.cpupool_op.cpupool_id =3D poolid= ; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sysctl.u.cpupool_op.cpu =3D (cpu < 0) ? > > XEN_SYSCTL_CPUPOOL_PAR_ANY : cpu; > > -=C2=A0=C2=A0=C2=A0=C2=A0for ( retries =3D 0; retries < NUM_RMCPU_BUSY_= RETRIES; retries++ > > ) { > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0err =3D do_sysctl_save= (xch, &sysctl); > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if ( err < 0 && errno = =3D=3D EBUSY ) > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0sleep(1); > I'd rather just remove this sleep() call than the whole retry logic. > EBUSY cases should be very very very very rare and last for some > msecs or usecs only. >=20 +1 Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-QyD8CsRLixzYy5bQxMH+ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEABECAAYFAlcQnGIACgkQk4XaBE3IOsTBOACgrh6KFgqkQmGN2otj4/RuofBv QUIAnAwzlpmY0EFCZilk1qmlSy3RPW2/ =Bouu -----END PGP SIGNATURE----- --=-QyD8CsRLixzYy5bQxMH+-- --===============5856601921823737072== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============5856601921823737072==--