All of lore.kernel.org
 help / color / mirror / Atom feed
From: anshul makkar <anshul.makkar@citrix.com>
To: George Dunlap <george.dunlap@citrix.com>,
	Dario Faggioli <dario.faggioli@citrix.com>,
	xen-devel@lists.xen.org
Cc: george.dunlap@eu.citrix.com
Subject: Re: [PATCH -v3 1/1] ratelimit: Implement rate limit for credit2 scheduler Rate limit assures that a vcpu will execute for a minimum amount of time before being put at the back of a queue or being preempted by higher priority thread.
Date: Wed, 3 Aug 2016 12:43:44 +0100	[thread overview]
Message-ID: <57A1D8F0.5020307@citrix.com> (raw)
In-Reply-To: <1877c9ff-ac9d-6ef9-9b83-488502f99bb3@citrix.com>

On 03/08/16 11:16, George Dunlap wrote:
> On 26/07/16 17:21, Dario Faggioli wrote:
>> On Mon, 2016-07-25 at 17:12 +0100, Anshul Makkar wrote:
>>> It introduces context-switch rate-limiting. The patch enables the VM
>> The subject, which will become the "title" of the commit, is way too
>> long. That must be a very concise headline of what the patch is about,
>> and should also have tags, specifying what areas of the codebase are
>> affected. So what do you think of this:
>>
>>    xen: credit2: implement context switch rate-limiting.
>
> It looks like it's just missing a carrage return -- I could fix that up
> on check-in.
>
>>
>> On a more technical side, I think that...
>>> +    if ( prv->ratelimit_us )
>>> +    {
>>> +        s_time_t ratelimit_min = prv->ratelimit_us;
>>>
>> ... this should be:
>>
>>   s_time_t ratelimit_min = MICROSECS(prv->ratelimit_us);
>>
>> I realized that by looking at traces and seeing entries for which
>> CSCHED2_MIN_TIMER was being returned, even if I had set
>> sched_ratelimit_us to a value greater than that.
>
> Yes, Dario is correct here.
>
> There's also a small typo in one of the comments ("onw" instead of "own").
>
> With all those changed:
>
> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
>
> If Anshul and Dario are happy, I can fix all those thing up on commit.
>
Fine with me.
> -George
>
Anshul


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

  reply	other threads:[~2016-08-03 11:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-25 16:12 [PATCH -v3 1/1] ratelimit: Implement rate limit for credit2 scheduler Rate limit assures that a vcpu will execute for a minimum amount of time before being put at the back of a queue or being preempted by higher priority thread Anshul Makkar
2016-07-26 16:21 ` Dario Faggioli
2016-08-03 10:16   ` George Dunlap
2016-08-03 11:43     ` anshul makkar [this message]
2016-08-03 12:22       ` Dario Faggioli

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=57A1D8F0.5020307@citrix.com \
    --to=anshul.makkar@citrix.com \
    --cc=dario.faggioli@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=george.dunlap@eu.citrix.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.