xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: Meng Xu <mengxu@cis.upenn.edu>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Chen, Tianyang" <tiche@seas.upenn.edu>,
	Meng Xu <xumengpanda@gmail.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Dagaen Golomb <dgolomb@seas.upenn.edu>
Subject: Re: [PATCH v9]xen: sched: convert RTDS from time to event driven model
Date: Wed, 16 Mar 2016 15:44:40 +0100	[thread overview]
Message-ID: <1458139480.3102.918.camel@citrix.com> (raw)
In-Reply-To: <CAENZ-+kVMfTbii8rh4E4-bZRK+hSTG5dbpJ9GNNHYzON82hb8A@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2958 bytes --]

On Wed, 2016-03-16 at 10:20 -0400, Meng Xu wrote:
> As to the comment, I will suggest:
> /*
>  * RTDS_was_depleted: Is a vcpus budget depleted?
>  * + Set in burn_budget() when a vcpus budget turns to zero
>  * + Checked and cleared in repl_handler() to replenish the budget
>  */
> What do you think?
Wow, we really are on the same page, I just suggested something quite
similar to this.

> BTW, how about other parts of the patch? Is there something that you
> don't like?
I just sent in my comments. I did have a few of them, I'm afraid.

Still, there weren't anything really substantial, from the logical or
algorithmical point of view. They almost all are about how to make both
the patch and the final result as easy to understand as possible.

This patch is doing some important and serious rework, of a piece of
code which is not in the best possible shape, so I consider the above
really really important.

> I think the invalid budget returned in rt_schedule() in this patch is
> a serious logical bug.
It's a bug. And you did a good job catching it. That's it. :-)

> If all of my comments are solved, I think it is in a good state and
> I'm considering to send the reviewed-by tag in the near future.

I don't expect it to take much longer for me to also be able to give
green light. :-)

> However, I won't send the reviewed-by until I get your confirmation,
> since this will be the first reviewed-by I will be giving as
> maintainer. I'd like to take a safe step. :-D
Mmm... That's not how it works, though. I know the feeling, and we've
all been there, I guess. However, you see and are saying yourself
already that such attitude from you can't really fly, in the long run.

Do as you wish, but you must know that you lost the right of "taking
safe steps" when you've become part of the MAINTAINERS file. :-P

So, jokes aside, it's perfectly fine that, as soon as you're happy
about the status of the patch, you send in your tag. In this specific
case, even if you are a maintainer, that would not mean the patch could
go in, because there are outstanding comments from another maintainer. 

And yes, this means that co-maintainers are not entirely agreeing on
the readiness of a particular patch, but it's not at all a big deal...
It happens quite frequently, actually! :-)

So, again, I understand the feeling... But it really reduces to "with
great powers comes great responsibility". And just like in the movie
(which I don't even like that much!! :-/), the sooner you accept that,
the better. For you. For us. For the project. For the World. :-D :-D

<<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: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

Xen-devel mailing list

  reply	other threads:[~2016-03-16 14:45 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-15  0:04 [PATCH v9]xen: sched: convert RTDS from time to event driven model Tianyang Chen
2016-03-16  3:11 ` Meng Xu
2016-03-16  3:32   ` Chen, Tianyang
2016-03-16  3:40     ` Meng Xu
2016-03-16 10:23       ` Dario Faggioli
2016-03-16 14:20         ` Meng Xu
2016-03-16 14:44           ` Dario Faggioli [this message]
2016-03-16 20:51             ` Meng Xu
2016-03-16 14:25 ` Dario Faggioli
2016-03-16 15:43   ` Chen, Tianyang
2016-03-16 16:11     ` Dario Faggioli
2016-03-16 20:54     ` Meng Xu
2016-03-16 20:45   ` Meng Xu
2016-03-17  2:24   ` Chen, Tianyang

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1458139480.3102.918.camel@citrix.com \
    --to=dario.faggioli@citrix.com \
    --cc=dgolomb@seas.upenn.edu \
    --cc=george.dunlap@citrix.com \
    --cc=mengxu@cis.upenn.edu \
    --cc=tiche@seas.upenn.edu \
    --cc=xen-devel@lists.xenproject.org \
    --cc=xumengpanda@gmail.com \


* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).