From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS Date: Tue, 19 Sep 2017 11:23:01 +0200 Message-ID: <1505812981.3483.8.camel@citrix.com> References: <1504281532-3766-1-git-send-email-mengxu@cis.upenn.edu> <1504281532-3766-3-git-send-email-mengxu@cis.upenn.edu> <1505348195.13935.4.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5128609298237944115==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Meng Xu Cc: George Dunlap , Ian Jackson , Wei Liu , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --===============5128609298237944115== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-rU9cvrXPbLFAhMnc0NEt" --=-rU9cvrXPbLFAhMnc0NEt Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2017-09-15 at 12:01 -0400, Meng Xu wrote: > On Wed, Sep 13, 2017 at 8:16 PM, Dario Faggioli > wrote: > >=20 > > > I'm ok with what it is in this patch, although I feel that we can > > > kill the > > > =C2=A0if (scinfo->extratime !=3D > > > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) > > > because LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT is -1. > > >=20 > >=20 > > No, sorry, I don't understand what you mean here... >=20 > I was thinking about the following code: >=20 > =C2=A0=C2=A0=C2=A0=C2=A0if (scinfo->extratime !=3D > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (scinfo->extratime) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0s= dom.flags |=3D XEN_DOMCTL_SCHEDRT_extra; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0else > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0s= dom.flags &=3D ~XEN_DOMCTL_SCHEDRT_extra; > =C2=A0=C2=A0=C2=A0=C2=A0} >=20 > This code can be changed to > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (scinfo->extratime) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0s= dom.flags |=3D XEN_DOMCTL_SCHEDRT_extra; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0else > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0s= dom.flags &=3D ~XEN_DOMCTL_SCHEDRT_extra; >=20 > If the extratime uses default value (-1), we still set the extratime > flag. >=20 > That's why I feel we may kill the > =C2=A0if (scinfo->extratime !=3D LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAU= LT) >=20 Mmm... Ok, I see it now. Well, this is of course all up to the tools' maintainers. What I think it would be valauble to ask ourself here is, can, at this point, scinfo->extratime be equal to XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT? And if it is, what does it mean, and what do we want to do? I mean, if extratime is -1, it means that we've been called, without it being touched by xl (although, remember that, as a library, libxl can be linked to and called by other programs too, e.g., libvirt). If you think that this is a serious programming bug, you can use XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT to check that, and raise an assert. If you think it's an API misuse, you can use it to check for that, and return an error. If you think it's just fine, you can do whatever you want to do as default (which, AFAIUI, it's set the flag). In this case, it's probably fine to ignore XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT in actual code. Although, I'd still put a reference to it in a comment, to explain what's going on, and why we're doing things differently from budget and period (since _their_ *_DEFAULT are checked). Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-rU9cvrXPbLFAhMnc0NEt 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 iQIcBAABCAAGBQJZwOH1AAoJEBZCeImluHPuTooQAOyPY8+7E9OcxXcYg5pXpvPS CFxUHbbJI1gVcNvCyfsfrNLpSNcx4BL7RdS74YpW1F4Qj9WuHHBXFlt9BkX8Rnz6 nz6ZSJZWG8w7Vq/SHssept4NO553YYd/xI1be7StStpFpenjkghvPysd27O22NwO n+HbsCzFOu+mEWPcwSarTdGnGNin7EbZQCmhSZITHHbgomc74NhwXnOTaAMqv1bB muiNOCFQAfEnPWSbHeLF7bml2KQci5Eiylys7CZt4i2p0a8Mt7FdSVIX1SMzn1Je BI6NWoA2frYoy51J/oC6VvbxIMB923o0vRBtoKQnBwZ1brU6kug/EukAGfK+AZtV LnbuZ4wXPfEoBpP+JAtmvL33IX4+6ea9PCHdWR4fNiCqcGX4frlCsH5w/pQvv8V1 Ol1srlWRZL1vHsZHL1cDyEAs3feXqbjAzQuLguupLMbvYpw33/npOTRs9Jv9cea1 NSG7PvdB5WIRMvL1v3KlL8yKFCvX0jg7t3XSclwVcpxjyaNMzqoKWUk79qIqIQQT MIf3CvqWX7JHIVoTWxR+i0ddIGQKwa9weY6syJ6Lm5kGJFS+r7pfFhLcVesbtiS6 JxDGgnt8A8+btA8EEdEi3N85rqVF4m++yO/+LV8FuwBe8/I5hGq5zsQys6HcfvmK HCEOYkykiJpVdICFi0yk =9srR -----END PGP SIGNATURE----- --=-rU9cvrXPbLFAhMnc0NEt-- --===============5128609298237944115== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============5128609298237944115==--