[PATCH v3] xen:rtds: Fix bug in budget accounting

* [PATCH v3] xen:rtds: Fix bug in budget accounting
@ 2016-10-26 19:06 Meng Xu
  2016-10-27  8:54 ` Dario Faggioli
  0 siblings, 1 reply; 3+ messages in thread
From: Meng Xu @ 2016-10-26 19:06 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Dagaen Golomb, George Dunlap, Haoran Li, Dario Faggioli,
	Linh Thi Xuan Phan, Meng Xu, Meng Xu, Tianyang Chen

Bug scenario:
repl_timer_handler() may be called before rt_schedule() for a VCPU.
This situation may happen in two scenarios:
(1) The VCPU misses deadline due to the system is oversubscribed. For example,
    the sum of VCPUs utilization on a core is larger than one.
(2) The VCPU has budget = period, which causes the timers for
    rt_schedule() and repl_timer_handler() are fired at the same time.
When the situation happens, it causes the following incorrect behavior:
repl_timer_handler() will update the VCPU period and deadline.
If the VCPU is still the highest priority one, even with the new deadline,
it will continue to run, but with new period and deadline.
Since the budget enforcement timer for the previous period is still armed,
rt_schedule() will still be called in the new period and enforce the budget
for the previous period.
The current burn_budget() will deduct the time spent in previous period from
the budget in current period, which is incorrect.

Fix:
We keeps last_start always within the current period for a VCPU, so that
we only deduct the time spent in the current period from the VCPU budget.
We always update last_start whenever we update cur_deadline for a VCPU.

Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
Reported-by: Dagaen Golomb <dgolomb@cis.upenn.edu>

---
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Linh Thi Xuan Phan <linhphan@cis.upenn.edu>
Cc: Haoran Li <lihaoran@wustl.edu>
Cc: Meng Xu <xumengpanda@gmail.com>
Cc: Dagaen Golomb <dgolomb@cis.upenn.edu>
Cc: Tianyang Chen <tiche@cis.upenn.edu>
---
Changes from v1:
* Change commit message to make the bug scenario easier to understand;
* The two bug scenarios described in v1 can be actually fixed by this patch;
  so we do not need to change the runq_tickle

Changes from v2:
* Change commit message to make the bug scenario clear
* Always update last_start whenever cur_deadline is updated for a VCPU
* Update last_start to now, instead of (cur_deadline - period)
  as suggested by Dario Faggioli
---
 xen/common/sched_rt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index d95f798..4b4f232 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -407,6 +407,12 @@ rt_update_deadline(s_time_t now, struct rt_vcpu *svc)
         svc->cur_deadline += count * svc->period;
     }
 
+    /*
+     * svc may be scheduled to run immediately after it misses deadline
+     * Then rt_update_deadline is called before rt_schedule, which
+     * should only deduct the time spent in current period from the budget
+     */
+    svc->last_start = now;
     svc->cur_budget = svc->budget;
 
     /* TRACE */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 3+ messages in thread