From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tianyang Chen Subject: Re: [PATCH v3 1/1] xen: sched: convert RTDS from time to event driven model Date: Thu, 21 Jan 2016 23:31:25 -0500 Message-ID: <56A1B09D.7030501@gmail.com> References: <1453435594-4407-1-git-send-email-tiche@seas.upenn.edu> <1453435594-4407-2-git-send-email-tiche@seas.upenn.edu> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aMTNn-0005mP-Eg for xen-devel@lists.xenproject.org; Fri, 22 Jan 2016 04:31:35 +0000 Received: by mail-qg0-f67.google.com with SMTP id 6so4292335qgy.3 for ; Thu, 21 Jan 2016 20:31:34 -0800 (PST) In-Reply-To: <1453435594-4407-2-git-send-email-tiche@seas.upenn.edu> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xenproject.org Cc: dario.faggioli@citrix.com, george.dunlap@citrix.com, Dagaen Golomb , Meng Xu List-Id: xen-devel@lists.xenproject.org On 1/21/2016 11:06 PM, Tianyang Chen wrote: > Budget replenishment and enforcement are separated by adding > a replenishment timer, which fires at the next most imminent > release time of all runnable vcpus. > > A new runningq has been added to keep track of all vcpus that > are on pcpus. > > The following functions have major changes to manage the runningq > and replenishment Well, I'm just gonna comment on couple things here. The major difference between this patch and the previous one is the addition of a runningq, which leads to extra code in context_save(). Since the scheduler doesn't run until the first vcpu wakes up, wake() does some of the timer stuff, in additional to the logic in the handler. It works nicely especially when a vcpu wakes up and has the earliest next release time. > @@ -160,6 +169,7 @@ struct rt_private { > */ > struct rt_vcpu { > struct list_head q_elem; /* on the runq/depletedq list */ > + struct list_head runningq_elem; /* on the runningq list */ > struct list_head sdom_elem; /* on the domain VCPU list */ > Is it better to re-use q-elem so extra helper functions can be avoided? Maybe another flag thing that shows which q a vcpu is on? > @@ -848,8 +880,6 @@ rt_schedule(const struct scheduler *ops, s_time_t now, bool_t tasklet_work_sched > /* burn_budget would return for IDLE VCPU */ > burn_budget(ops, scurr, now); > > - __repl_update(ops, now); > - > if ( tasklet_work_scheduled ) > { > snext = rt_vcpu(idle_vcpu[cpu]); > @@ -875,6 +905,9 @@ rt_schedule(const struct scheduler *ops, s_time_t now, bool_t tasklet_work_sched > set_bit(__RTDS_delayed_runq_add, &scurr->flags); > > snext->last_start = now; > + > + ret.time = -1; /* if an idle vcpu is picked */ > + I kinda just stick this in based on previous discussion on RTDS busy-waiting. > > +/* The replenishment timer handler scans > + * all three queues and find the most > + * imminent release time. If there is no > + * vcpus, timer is set in wake() > + */ > +static void repl_handler(void *data){ > + unsigned long flags; > + s_time_t now = NOW(); > + s_time_t min_repl = LONG_MAX; /* max time used in comparisoni */ > + struct scheduler *ops = data; > + struct rt_private *prv = rt_priv(ops); > + struct list_head *runq = rt_runq(ops); > + struct list_head *depletedq = rt_depletedq(ops); > + struct list_head *runningq = rt_runningq(ops); > + struct list_head *iter; > + struct list_head *tmp; > + struct rt_vcpu *svc = NULL; > + > + stop_timer(&repl_timer); > + > + spin_lock_irqsave(&prv->lock, flags); > + Here is a lock for protecting queues. Correct me if I'm wrong, since in alloc_pdata(), the private lock points to the global system lock, I just assume there is no difference here. -- Tianyang Chen