From: "Chen, Tianyang" <email@example.com> To: Dario Faggioli <firstname.lastname@example.org>, email@example.com Cc: firstname.lastname@example.org, Dagaen Golomb <email@example.com>, Meng Xu <firstname.lastname@example.org> 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.email@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> 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 <email@example.com> >> Signed-off-by: Meng Xu <firstname.lastname@example.org> >> Signed-off-by: Dagaen Golomb <email@example.com> >> > You seem to have taken care of all my remaining comments to v10, so the > patch is: > > Reviewed-by: Dario Faggioli <firstname.lastname@example.org> > > (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? Thanks, Tianyang Chen _______________________________________________ Xen-devel mailing list Xenemail@example.com http://lists.xen.org/xen-devel
next prev parent 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 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: 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=56EB0F96.firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH v11]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).