xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Chen, Tianyang" <tiche@seas.upenn.edu>
To: Dario Faggioli <dario.faggioli@citrix.com>,
Cc: george.dunlap@citrix.com, Dagaen Golomb <dgolomb@seas.upenn.edu>,
	Meng Xu <mengxu@cis.upenn.edu>
Subject: Re: [PATCH v11]xen: sched: convert RTDS from time to event driven model
Date: Thu, 17 Mar 2016 16:12:06 -0400	[thread overview]
Message-ID: <56EB0F96.8090905@seas.upenn.edu> (raw)
In-Reply-To: <1458239259.15374.67.camel@citrix.com>

On 03/17/2016 02:27 PM, Dario Faggioli wrote:
>> Finally, when waking up a vcpu, it is now enough to tickle the
>> various
>> CPUs appropriately, like all other schedulers also do.
>> Signed-off-by: Tianyang Chen <tiche@seas.upenn.edu>
>> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
>> Signed-off-by: Dagaen Golomb <dgolomb@seas.upenn.edu>
> You seem to have taken care of all my remaining comments to v10, so the
> patch is:
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> (which you could have put there yourself, as I said. :-))
> Thanks for all the good work.


> If I can already point you to something else that can be improved
> further in this scheduler, _completely_ independently from this, and
> hence in a fully new patch (series), can you have a look at whether we
> really need set_bit(), test_and_clear(), etc, instead of __set_bit(),
> __test_and_clear(), etc.?
> I'm asking because I'm under the impression that the latter are enough,
> and if that is the case, we should go for them, as they're more
> efficient.

Sure. __set_bit() and __test_and_clear() are not atomic and can be 
reordered. They need to be protected by locks. So, it looks like where 
RTDS is using them already have spin_locks in place and re-ordering 
inside the critical region doesn't really mess anything up.

rt_schedule() sets: __RTDS_scheduled, __RTDS_delay_runq_add
burn_budget() sets: __RTDS_depleted
rt_wake() sets: __RTDS_delay_runq_add

rt_vcpu_sleep() clears: __RTDS_delayed_runq_add
rt_context_saved() clears: __RTDS_scheduled, __RTDS_delayed_runq_add
repl_timer_handler() clears: __RTDS_depleted

burn_budget() is inside of rt_schedule() but they are using different 
flags of different vcpus. So it's enough to just use __set_bit(), 
__clear_bit() and __test_and_clear().

I'm assuming the new patch should be based on the last patch I sent? 
Because replenishment handler is not presented in the old RTDS code?

Tianyang Chen

Xen-devel mailing list

  reply	other threads:[~2016-03-17 20:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-17 16:06 [PATCH v11]xen: sched: convert RTDS from time to event driven model Tianyang Chen
2016-03-17 18:27 ` Dario Faggioli
2016-03-17 20:12   ` Chen, Tianyang [this message]
2016-03-18  1:45     ` Meng Xu
2016-03-18  4:09 ` Meng Xu
2016-03-18  7:45   ` Jan Beulich
2016-03-18  9:23     ` Dario Faggioli
2016-03-18 14:23       ` Meng Xu
2016-03-18 14:19     ` Meng Xu
2016-03-18  9:24   ` Dario Faggioli
2016-03-18 14:25 ` Meng Xu

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=56EB0F96.8090905@seas.upenn.edu \
    --to=tiche@seas.upenn.edu \
    --cc=dario.faggioli@citrix.com \
    --cc=dgolomb@seas.upenn.edu \
    --cc=george.dunlap@citrix.com \
    --cc=mengxu@cis.upenn.edu \
    --cc=xen-devel@lists.xenproject.org \


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