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

[Meng Xu <mengxu@xxxxxxxxxxxxx>]

On Wed, Mar 16, 2016 at 10:44 AM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
> 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.

Right. :-)

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

Thank you very much! I saw it! I see the point. I didn't think too
much about the better choice of the comments.
Those suggestions make the comment more useful... Thank you very much! :-)

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

Right! But those are easy to solve. :-)

>
>> 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. :-)

Great!

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

Right! I understood...

>
> 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

OK... Then I will send the tag when I feel comfortable with the patch then.

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

I see. Good to know that. :-)

>
> 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

Sure! OK... No problem. The worst case is that I have to "kill the
bug" that passed me. :-D

Thank you very much!

Best Regards,

Meng

-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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