xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Meng Xu <mengxu@cis.upenn.edu>
To: Dario Faggioli <dario.faggioli@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Chen, Tianyang" <tiche@seas.upenn.edu>,
	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 16:51:56 -0400	[thread overview]
Message-ID: <CAENZ-+nY6ns0VRCwP42BC24O9TMb9UzqsFOmKvZTYhPTe-VQmw@mail.gmail.com> (raw)
In-Reply-To: <1458139480.3102.918.camel@citrix.com>

On Wed, Mar 16, 2016 at 10:44 AM, Dario Faggioli
<dario.faggioli@citrix.com> 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@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-03-16 20:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-15  0:04 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
2016-03-16 20:51             ` Meng Xu [this message]
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:
  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=CAENZ-+nY6ns0VRCwP42BC24O9TMb9UzqsFOmKvZTYhPTe-VQmw@mail.gmail.com \
    --to=mengxu@cis.upenn.edu \
    --cc=dario.faggioli@citrix.com \
    --cc=dgolomb@seas.upenn.edu \
    --cc=george.dunlap@citrix.com \
    --cc=tiche@seas.upenn.edu \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH v9]xen: sched: convert RTDS from time to event driven model' \
    /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

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