From: Dario Faggioli <dario.faggioli@citrix.com>
To: Praveen Kumar <kpraveen.lkml@gmail.com>, xen-devel@lists.xen.org
Cc: george.dunlap@eu.citrix.com
Subject: Re: [PATCH] Removal of redundant check
Date: Thu, 15 Dec 2016 01:18:40 +0100 [thread overview]
Message-ID: <1481761120.3445.313.camel@citrix.com> (raw)
In-Reply-To: <1481739598.6878.10.camel@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1835 bytes --]
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:
[PATCH] xen: sched: removal of redundant check in Credit
or:
[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.
>
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.
>
> Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
>
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
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-12-15 0:18 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-14 18:19 [PATCH] Removal of redundant check Praveen Kumar
2016-12-15 0:18 ` Dario Faggioli [this message]
2016-12-15 5:34 ` George Dunlap
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1481761120.3445.313.camel@citrix.com \
--to=dario.faggioli@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=kpraveen.lkml@gmail.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.