Re: [PATCH v9]xen: sched: convert RTDS from time to event driven model

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.
>
Sure!

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

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

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel