From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH] Removal of redundant check Date: Thu, 15 Dec 2016 01:18:40 +0100 Message-ID: <1481761120.3445.313.camel@citrix.com> References: <1481739598.6878.10.camel@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8083657429225516748==" Return-path: In-Reply-To: <1481739598.6878.10.camel@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Praveen Kumar , xen-devel@lists.xen.org Cc: george.dunlap@eu.citrix.com List-Id: xen-devel@lists.xenproject.org --===============8083657429225516748== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-6dRtn9Pbqk8O9oOkKL7C" --=-6dRtn9Pbqk8O9oOkKL7C Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hello! Glad to see he patch here. One thing, about the subject line: it should contains "tags", i.e., an indication of the component that the patch affects. So, for example, in this case, the patch touches Credit1, which means scheduling inside Xen. So, a valid subject line could be: =C2=A0[PATCH] xen: sched: removal of redundant check in Credit or: =C2=A0[PATCH] xen: credit: removal of redundant check On Wed, 2016-12-14 at 23:49 +0530, Praveen Kumar wrote: > The patch gets rid of a redundant check in csched_vcpu_acct which > adds > more code clarity and performance.=20 > I'd remove "which adds more code clarity and performance" and put here something like: "In fact, the function is only called from csched_tick, which already checks that current is not the idle vcpu." > This patch also adds an ASSERT to > the same effect, in order to make assumption ( i.e., no calling this > on > idle vcpus) even more clear and as a guard for future mis-use. >=20 > Signed-off-by: Praveen Kumar >=20 Apart from what just said above, the patch looks good to me. Can you send version 2 with the changelog updated? Oh, and I see that you are both inlining in the email and attaching the patch. I personally don't particularly mind, but that may make the life of a committer (the person which, when the patch will have all the Acks, will put it inside the Xen git repository) a bit more difficult. So, this is mostly George's call, I think, but FWIW, I'd suggest you avoid doing that. :-) Thanks and Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-6dRtn9Pbqk8O9oOkKL7C 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 iQIcBAABCAAGBQJYUeFgAAoJEBZCeImluHPuAusQAO0Js0xUpzr9Wu/NvRXCvLBc FEzgesnq8RgtbpYSe9+r1GaPoKQtNmaqWpanfFKG29dgL1If8QAWcthZzyMrr0Tm 7E+W7oeuE6er1zogj3Yq+v+g0buiL0/okFHiw+RtcY0ICOT2EUlJxXI5ipTz1G1B 9C+YIegqs82GNV5iBASYIIlgwljlV82nj1Bd6j5FwIWS2Qe/JrpgLFsyPTlww32x LzCn3+nXyodKXpP4HrjaZ8B0FZoQQ4sHiZQhoiCDUodudlFxGlN47Tgc/pFhLiG8 yB2T67eSngC1wxJBq1r57LeBMGoMJfZOEuYXAyi+DKZew6t+YUgyjlvWculwgzoh a2xggD46pVyoszEcmVhCNlx3ELjugvFZ6vHRP/58IDKPnAi/0rX/TiUHYzm62Jkt zSE/iSj7kaI4x6kxHo1wunBaWhr3M8cWS5IBwzx2mycu554/pH5zfQ6HxUZ1ynDQ sFyqMCSChFZVMNzJilj12diwslAl+yGpjJ6TyfEZ2FCEzLD+JgMC61m38/XbRhul RRMWb+1wibC83F7fY3PmhtCUwkYvSHGZ2gjZYKxGL1/B+hiorXb/VVq7VNiM8xVR IP+a0R54p87SzDzh+AJhFUmgKvRDmRwQd5UrEY4l2H88ettCJoe8p0smSQvSLOI/ 8f24zCMqjvdCo5stZaQs =Dt+K -----END PGP SIGNATURE----- --=-6dRtn9Pbqk8O9oOkKL7C-- --===============8083657429225516748== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============8083657429225516748==--