From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1/1] Improved RTDS scheduler Date: Mon, 4 Jan 2016 15:36:58 +0100 Message-ID: <1451918218.27063.27.camel@citrix.com> References: <1451555121-8085-1-git-send-email-tiche@seas.upenn.edu> <1451555121-8085-2-git-send-email-tiche@seas.upenn.edu> <1451907723.13361.39.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8345895122587978820==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aG6HA-0007HH-SN for xen-devel@lists.xenproject.org; Mon, 04 Jan 2016 14:38:25 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Meng Xu , Ian Campbell Cc: Tianyang Chen , Ian Jackson , George Dunlap , Meng Xu , Jan Beulich , "xen-devel@lists.xenproject.org" , Dagaen Golomb List-Id: xen-devel@lists.xenproject.org --===============8345895122587978820== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-fIsLW1y6hALMGbOb1hTV" --=-fIsLW1y6hALMGbOb1hTV Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2016-01-04 at 21:48 +0800, Meng Xu wrote: > On Mon, Jan 4, 2016 at 7:42 PM, Ian Campbell > wrote: > > On Thu, 2015-12-31 at 04:45 -0500, Tianyang Chen wrote: > > > Budget replenishment is now handled by a dedicated timer which is > > > triggered at the most imminent release time of all runnable > > > vcpus. > > >=20 > > > Signed-off-by: Tianyang Chen > > > Signed-off-by: Meng Xu > > > Signed-off-by: Dagaen Golomb > > > --- > > > =C2=A00000-cover-letter.patch=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=A016 +++ > > > =C2=A00001-Improved-RTDS-scheduler.patch |=C2=A0=C2=A0280 > > > ++++++++++++++++++++++++++++++++++++ > > > =C2=A0bak=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=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A0=C2= =A062 ++++++++ > >=20 > > I assume you didn't actually intend to commit those ;-) >=20 > No. This version was sent by mistake. Please ignore this version. :-( >=20 > Tianyang sent another version (v2) with the subject "[PATCH V2 1/1] > Improved RTDS scheduler". >=20 Indeed. BTW, I'm back today from Winter Holidays. I'll give a look at this series as soon as practical. I'll comment directly on V2, with the only exception of this very email. So, first of all, Cc list (which is the same here and in V2): how have the names of the people that are in there been picked? The patch is only about xen/common/sched_rt.c. As per MAINTAINERS, the maintainer of the code hosted in that file is just me. Surely it's a good thing to also include George, as he's the other maintainer of scheduling in general. All the other people, though, should not be bothered by being copied directly... Especially IanC and IanJ, I'd say. > > (In general single patches do not require a separate cover letter > > unless > > the required context contains a large amount of information which > > is not > > appropriate for the commit message of the actual change, I'll leave > > it to > > you and the scheduler maintainers to decide how much of your cover > > letter > > it would be appropriate to move to the commit message). >=20 > The cover letter is supposed to explain the design idea of the > improved RTDS scheduler so that reviewers could (potentially) save > time in reviewing the patch, we think. :-) > Probably, the commit message should be refined and self-contained? >=20 That's true. In particular, the "context", defined as summary of what happened and have been discussed on the mailing list before, during previous submissions, etc., definitely belongs to a cover letter. High level design ideas and concepts, also do, e.g., in order to avoid ending up with commit changelogs that are all several pages long! :-) All that being said, we certainly want patches' subject lines and changelogs to be descriptive of at least what is being done, and why. If we commit this as it is right now, here it is what we'll find in `git log': =C2=A0 Improved RTDS scheduler =C2=A0 Budget replenishment is now handled by a dedicated timer which is =C2=A0 triggered at the most imminent release time of all runnable vcpus. Which won't, I'm quite sure about it, be that much useful when looking at it, say, in 3 years! :-D I know, finding the proper balance of what's better put where is not always super easy, especially at the beginning... but that's part of the process of cooking a good patch (series) :-) A few suggestions: =C2=A0- we want tags in the subject. E.g., in this case, something like =C2=A0 =C2=A0"xen: sched: Improved RTDS scheduler" =C2=A0- avoid (in both subject and changelog) generic terms like "improve" =C2=A0 =C2=A0"fix", etc., unless you can explain more specifically to what = they=C2=A0 =C2=A0 =C2=A0refer... I mean, pretty much all commits that touch sched_rt.c= can=C2=A0 =C2=A0 =C2=A0have "Improve RTDS" as a subject, can't they? (Unless we decid= e to=C2=A0 =C2=A0 =C2=A0commit something making things deliberately worse!! :-)) =C2=A0 =C2=A0In this case, e.g. (although, perhaps a bit long): =C2=A0 =C2=A0 =C2=A0"xen: sched: convert RTDS from time to event driven mod= el" =C2=A0- as Ian said, there are information in the cover letter that can =C2=A0 =C2=A0well be moved in the changelog, making it a lot more useful, a= nd =C2=A0 =C2=A0not less refined or self contained. 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) --=-fIsLW1y6hALMGbOb1hTV 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 v1 iEYEABECAAYFAlaKg4sACgkQk4XaBE3IOsQeJgCgoOb9H1+LvDzNM+N8ydXD7pMA GwYAoKIW3SkbjIXhS/HLy4ynXPKAMgKu =GQq8 -----END PGP SIGNATURE----- --=-fIsLW1y6hALMGbOb1hTV-- --===============8345895122587978820== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============8345895122587978820==--