[PATCH v2 0/5] Towards work-conserving RTDS

[PATCH v2 0/5] Towards work-conserving RTDS

[Meng Xu]

This series of patches make RTDS scheduler work-conserving
without breaking real-time guarantees.
VCPUs with extratime flag set can get extra time
from the unreserved system resource.
System administrators can decide which VCPUs have extratime flag set.

Example:
Set the extratime bit of all VCPUs of domain 1:
# xl sched-rtds -d 1 -v all -p 10000 -b 2000 -e 1
Each VCPU of domain 1 will be guaranteed to have 2000ms every 10000ms
(if the system is schedulable).
If there is a CPU having no work to do,
domain 1's VCPUs will be scheduled onto the CPU,
even though the VCPUs have got 2000ms in 10000ms.

Clear the extra bit of all VCPUs of domain 1:
# xl sched-rtds -d 1 -v all -p 10000 -b 2000 -e 0

Set/Clear the extratime bit of one specific VCPU of domain 1:
# xl sched-rtds -d 1 -v 1 -p 10000 -b 2000 -e 1
# xl sched-rtds -d 1 -v 1 -p 10000 -b 2000 -e 0


The original design of the work-conserving RTDS was discussed at
https://www.mail-archive.com/xen-devel@lists.xen.org/msg77150.html

The first version was discussed at
https://www.mail-archive.com/xen-devel@lists.xen.org/msg117361.html

The series of patch can be found at github:
https://github.com/PennPanda/RT-Xen
under the branch:
xenbits/rtds/work-conserving-v2

Changes from v1
Change XEN_DOMCTL_SCHED_RTDS_extratime to XEN_DOMCTL_SCHEDRT_extra
Revise xentrace, xenalyze, and docs
Add LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA symbol in libxl.h

Changes from RFC v1
Merge changes in sched_rt.c into one patch;
Minor change in variable name and comments.

Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>

[PATCH v2 1/5] xen:rtds: towards work conserving RTDS
[PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS
[PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS
[PATCH v2 4/5] xentrace: enable per-VCPU extratime flag for RTDS
[PATCH v2 5/5] docs: enable per-VCPU extratime flag for RTDS


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

[PATCH v2 1/5] xen:rtds: towards work conserving RTDS

[Meng Xu]

Make RTDS scheduler work conserving without breaking the real-time guarantees.

VCPU model:
Each real-time VCPU is extended to have an extratime flag
and a priority_level field.
When a VCPU's budget is depleted in the current period,
if it has extratime flag set,
its priority_level will increase by 1 and its budget will be refilled;
othewrise, the VCPU will be moved to the depletedq.

Scheduling policy is modified global EDF:
A VCPU v1 has higher priority than another VCPU v2 if
(i) v1 has smaller priority_leve; or
(ii) v1 has the same priority_level but has a smaller deadline

Queue management:
Run queue holds VCPUs with extratime flag set and VCPUs with
remaining budget. Run queue is sorted in increasing order of VCPUs priorities.
Depleted queue holds VCPUs which have extratime flag cleared and depleted budget.
Replenished queue is not modified.

Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>

---
Changes from v1
Change XEN_DOMCTL_SCHED_RTDS_extratime to XEN_DOMCTL_SCHEDRT_extra as
suggested by Dario

Changes from RFC v1
Rewording comments and commit message
Remove is_work_conserving field from rt_vcpu structure
Use one bit in VCPU's flag to indicate if a VCPU will have extra time
Correct comments style
---
 xen/common/sched_rt.c       | 90 ++++++++++++++++++++++++++++++++++++++-------
 xen/include/public/domctl.h |  4 ++
 2 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 0ac5816..fab6f49 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -49,13 +49,15 @@
  * A PCPU is feasible if the VCPU can run on this PCPU and (the PCPU is idle or
  * has a lower-priority VCPU running on it.)
  *
- * Each VCPU has a dedicated period and budget.
+ * Each VCPU has a dedicated period, budget and a extratime flag
  * The deadline of a VCPU is at the end of each period;
  * A VCPU has its budget replenished at the beginning of each period;
  * While scheduled, a VCPU burns its budget.
  * The VCPU needs to finish its budget before its deadline in each period;
  * The VCPU discards its unused budget at the end of each period.
- * If a VCPU runs out of budget in a period, it has to wait until next period.
+ * When a VCPU runs out of budget in a period, if its extratime flag is set,
+ * the VCPU increases its priority_level by 1 and refills its budget; otherwise,
+ * it has to wait until next period.
  *
  * Each VCPU is implemented as a deferable server.
  * When a VCPU has a task running on it, its budget is continuously burned;
@@ -63,7 +65,8 @@
  *
  * Queue scheme:
  * A global runqueue and a global depletedqueue for each CPU pool.
- * The runqueue holds all runnable VCPUs with budget, sorted by deadline;
+ * The runqueue holds all runnable VCPUs with budget,
+ * sorted by priority_level and deadline;
  * The depletedqueue holds all VCPUs without budget, unsorted;
  *
  * Note: cpumask and cpupool is supported.
@@ -151,6 +154,14 @@
 #define RTDS_depleted (1<<__RTDS_depleted)
 
 /*
+ * RTDS_extratime: Can the vcpu run in the time that is
+ * not part of any real-time reservation, and would therefore
+ * be otherwise left idle?
+ */
+#define __RTDS_extratime    4
+#define RTDS_extratime (1<<__RTDS_extratime)
+
+/*
  * rt tracing events ("only" 512 available!). Check
  * include/public/trace.h for more details.
  */
@@ -201,6 +212,8 @@ struct rt_vcpu {
     struct rt_dom *sdom;
     struct vcpu *vcpu;
 
+    unsigned priority_level;
+
     unsigned flags;              /* mark __RTDS_scheduled, etc.. */
 };
 
@@ -245,6 +258,11 @@ static inline struct list_head *rt_replq(const struct scheduler *ops)
     return &rt_priv(ops)->replq;
 }
 
+static inline bool has_extratime(const struct rt_vcpu *svc)
+{
+    return (svc->flags & RTDS_extratime) ? 1 : 0;
+}
+
 /*
  * Helper functions for manipulating the runqueue, the depleted queue,
  * and the replenishment events queue.
@@ -274,6 +292,21 @@ vcpu_on_replq(const struct rt_vcpu *svc)
 }
 
 /*
+ * If v1 priority >= v2 priority, return value > 0
+ * Otherwise, return value < 0
+ */
+static s_time_t
+compare_vcpu_priority(const struct rt_vcpu *v1, const struct rt_vcpu *v2)
+{
+    int prio = v2->priority_level - v1->priority_level;
+
+    if ( prio == 0 )
+        return v2->cur_deadline - v1->cur_deadline;
+
+    return prio;
+}
+
+/*
  * Debug related code, dump vcpu/cpu information
  */
 static void
@@ -303,6 +336,7 @@ rt_dump_vcpu(const struct scheduler *ops, const struct rt_vcpu *svc)
     cpulist_scnprintf(keyhandler_scratch, sizeof(keyhandler_scratch), mask);
     printk("[%5d.%-2u] cpu %u, (%"PRI_stime", %"PRI_stime"),"
            " cur_b=%"PRI_stime" cur_d=%"PRI_stime" last_start=%"PRI_stime"\n"
+           " \t\t priority_level=%d has_extratime=%d\n"
            " \t\t onQ=%d runnable=%d flags=%x effective hard_affinity=%s\n",
             svc->vcpu->domain->domain_id,
             svc->vcpu->vcpu_id,
@@ -312,6 +346,8 @@ rt_dump_vcpu(const struct scheduler *ops, const struct rt_vcpu *svc)
             svc->cur_budget,
             svc->cur_deadline,
             svc->last_start,
+            svc->priority_level,
+            has_extratime(svc),
             vcpu_on_q(svc),
             vcpu_runnable(svc->vcpu),
             svc->flags,
@@ -423,15 +459,18 @@ rt_update_deadline(s_time_t now, struct rt_vcpu *svc)
      */
     svc->last_start = now;
     svc->cur_budget = svc->budget;
+    svc->priority_level = 0;
 
     /* TRACE */
     {
         struct __packed {
             unsigned vcpu:16, dom:16;
+            unsigned priority_level;
             uint64_t cur_deadline, cur_budget;
         } d;
         d.dom = svc->vcpu->domain->domain_id;
         d.vcpu = svc->vcpu->vcpu_id;
+        d.priority_level = svc->priority_level;
         d.cur_deadline = (uint64_t) svc->cur_deadline;
         d.cur_budget = (uint64_t) svc->cur_budget;
         trace_var(TRC_RTDS_BUDGET_REPLENISH, 1,
@@ -454,7 +493,7 @@ rt_update_deadline(s_time_t now, struct rt_vcpu *svc)
  * cases, if the vcpu with the earliest deadline is what we
  * are dealing with).
  */
-static inline bool_t
+static inline bool
 deadline_queue_remove(struct list_head *queue, struct list_head *elem)
 {
     int pos = 0;
@@ -466,7 +505,7 @@ deadline_queue_remove(struct list_head *queue, struct list_head *elem)
     return !pos;
 }
 
-static inline bool_t
+static inline bool
 deadline_queue_insert(struct rt_vcpu * (*qelem)(struct list_head *),
                       struct rt_vcpu *svc, struct list_head *elem,
                       struct list_head *queue)
@@ -477,7 +516,7 @@ deadline_queue_insert(struct rt_vcpu * (*qelem)(struct list_head *),
     list_for_each ( iter, queue )
     {
         struct rt_vcpu * iter_svc = (*qelem)(iter);
-        if ( svc->cur_deadline <= iter_svc->cur_deadline )
+        if ( compare_vcpu_priority(svc, iter_svc) > 0 )
             break;
         pos++;
     }
@@ -537,8 +576,9 @@ runq_insert(const struct scheduler *ops, struct rt_vcpu *svc)
     ASSERT( !vcpu_on_q(svc) );
     ASSERT( vcpu_on_replq(svc) );
 
-    /* add svc to runq if svc still has budget */
-    if ( svc->cur_budget > 0 )
+    /* add svc to runq if svc still has budget or its extratime is set */
+    if ( svc->cur_budget > 0 ||
+         has_extratime(svc) )
         deadline_runq_insert(svc, &svc->q_elem, runq);
     else
         list_add(&svc->q_elem, &prv->depletedq);
@@ -857,6 +897,8 @@ rt_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd)
     svc->vcpu = vc;
     svc->last_start = 0;
 
+    __set_bit(__RTDS_extratime, &svc->flags);
+    svc->priority_level = 0;
     svc->period = RTDS_DEFAULT_PERIOD;
     if ( !is_idle_vcpu(vc) )
         svc->budget = RTDS_DEFAULT_BUDGET;
@@ -966,8 +1008,16 @@ burn_budget(const struct scheduler *ops, struct rt_vcpu *svc, s_time_t now)
 
     if ( svc->cur_budget <= 0 )
     {
-        svc->cur_budget = 0;
-        __set_bit(__RTDS_depleted, &svc->flags);
+        if ( has_extratime(svc) )
+        {
+            svc->priority_level++;
+            svc->cur_budget = svc->budget;
+        }
+        else
+        {
+            svc->cur_budget = 0;
+            __set_bit(__RTDS_depleted, &svc->flags);
+        }
     }
 
     /* TRACE */
@@ -976,11 +1026,15 @@ burn_budget(const struct scheduler *ops, struct rt_vcpu *svc, s_time_t now)
             unsigned vcpu:16, dom:16;
             uint64_t cur_budget;
             int delta;
+            unsigned priority_level;
+            bool has_extratime;
         } d;
         d.dom = svc->vcpu->domain->domain_id;
         d.vcpu = svc->vcpu->vcpu_id;
         d.cur_budget = (uint64_t) svc->cur_budget;
         d.delta = delta;
+        d.priority_level = svc->priority_level;
+        d.has_extratime = svc->flags & RTDS_extratime;
         trace_var(TRC_RTDS_BUDGET_BURN, 1,
                   sizeof(d),
                   (unsigned char *) &d);
@@ -1088,7 +1142,7 @@ rt_schedule(const struct scheduler *ops, s_time_t now, bool_t tasklet_work_sched
              vcpu_runnable(current) &&
              scurr->cur_budget > 0 &&
              ( is_idle_vcpu(snext->vcpu) ||
-               scurr->cur_deadline <= snext->cur_deadline ) )
+               compare_vcpu_priority(scurr, snext) > 0 ) )
             snext = scurr;
     }
 
@@ -1194,7 +1248,7 @@ runq_tickle(const struct scheduler *ops, struct rt_vcpu *new)
         }
         iter_svc = rt_vcpu(iter_vc);
         if ( latest_deadline_vcpu == NULL ||
-             iter_svc->cur_deadline > latest_deadline_vcpu->cur_deadline )
+             compare_vcpu_priority(iter_svc, latest_deadline_vcpu) < 0 )
             latest_deadline_vcpu = iter_svc;
 
         cpumask_clear_cpu(cpu, &not_tickled);
@@ -1203,7 +1257,7 @@ runq_tickle(const struct scheduler *ops, struct rt_vcpu *new)
 
     /* 2) candicate has higher priority, kick out lowest priority vcpu */
     if ( latest_deadline_vcpu != NULL &&
-         new->cur_deadline < latest_deadline_vcpu->cur_deadline )
+         compare_vcpu_priority(latest_deadline_vcpu, new) < 0 )
     {
         SCHED_STAT_CRANK(tickled_busy_cpu);
         cpu_to_tickle = latest_deadline_vcpu->vcpu->processor;
@@ -1394,6 +1448,10 @@ rt_dom_cntl(
                 svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
                 local_sched.u.rtds.budget = svc->budget / MICROSECS(1);
                 local_sched.u.rtds.period = svc->period / MICROSECS(1);
+                if ( has_extratime(svc) )
+                    local_sched.u.rtds.flags |= XEN_DOMCTL_SCHEDRT_extra;
+                else
+                    local_sched.u.rtds.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
                 spin_unlock_irqrestore(&prv->lock, flags);
 
                 if ( copy_to_guest_offset(op->u.v.vcpus, index,
@@ -1418,6 +1476,10 @@ rt_dom_cntl(
                 svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
                 svc->period = period;
                 svc->budget = budget;
+                if ( local_sched.u.rtds.flags & XEN_DOMCTL_SCHEDRT_extra )
+                    __set_bit(__RTDS_extratime, &svc->flags);
+                else
+                    __clear_bit(__RTDS_extratime, &svc->flags);
                 spin_unlock_irqrestore(&prv->lock, flags);
             }
             /* Process a most 64 vCPUs without checking for preemptions. */
@@ -1492,7 +1554,7 @@ static void repl_timer_handler(void *data){
         {
             struct rt_vcpu *next_on_runq = q_elem(runq->next);
 
-            if ( svc->cur_deadline > next_on_runq->cur_deadline )
+            if ( compare_vcpu_priority(svc, next_on_runq) < 0 )
                 runq_tickle(ops, next_on_runq);
         }
         else if ( __test_and_clear_bit(__RTDS_depleted, &svc->flags) &&
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 83c7c75..c747af0 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -360,6 +360,10 @@ typedef struct xen_domctl_sched_credit2 {
 typedef struct xen_domctl_sched_rtds {
     uint32_t period;
     uint32_t budget;
+/* Can this vCPU execute beyond its reserved amount of time? */
+#define _XEN_DOMCTL_SCHEDRT_extra   0
+#define XEN_DOMCTL_SCHEDRT_extra    (1U<<_XEN_DOMCTL_SCHEDRT_extra)
+    uint32_t flags;
 } xen_domctl_sched_rtds_t;
 
 typedef struct xen_domctl_schedparam_vcpu {
-- 
1.9.1


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

[PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Meng Xu]

Modify libxl_vcpu_sched_params_get/set and sched_rtds_vcpu_get/set
functions to support per-VCPU extratime flag

Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>

---
Changes from v1
1) Add LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA to indicate if extratime flag is
supported
2) Change flag name in domctl.h from XEN_DOMCTL_SCHED_RTDS_extratime to
XEN_DOMCTL_SCHEDRT_extra

Changes from RFC v1
Change work_conserving flag to extratime flag
---
 tools/libxl/libxl_sched.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
---
 tools/libxl/libxl.h       |  6 ++++++
 tools/libxl/libxl_sched.c | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 1704525..ead300f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -257,6 +257,12 @@
 #define LIBXL_HAVE_SCHED_RTDS_VCPU_PARAMS 1
 
 /*
+ * LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA indicates RTDS scheduler
+ * now supports per-vcpu extratime settings.
+ */
+#define LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA 1
+
+/*
  * libxl_domain_build_info has the arm.gic_version field.
  */
 #define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 1
diff --git a/tools/libxl/libxl_sched.c b/tools/libxl/libxl_sched.c
index faa604e..b76a29a 100644
--- a/tools/libxl/libxl_sched.c
+++ b/tools/libxl/libxl_sched.c
@@ -558,6 +558,10 @@ static int sched_rtds_vcpu_get_all(libxl__gc *gc, uint32_t domid,
     for (i = 0; i < num_vcpus; i++) {
         scinfo->vcpus[i].period = vcpus[i].u.rtds.period;
         scinfo->vcpus[i].budget = vcpus[i].u.rtds.budget;
+        if (vcpus[i].u.rtds.flags & XEN_DOMCTL_SCHEDRT_extra)
+           scinfo->vcpus[i].extratime = 1;
+        else
+           scinfo->vcpus[i].extratime = 0;
         scinfo->vcpus[i].vcpuid = vcpus[i].vcpuid;
     }
     rc = 0;
@@ -607,6 +611,10 @@ static int sched_rtds_vcpu_set(libxl__gc *gc, uint32_t domid,
         vcpus[i].vcpuid = scinfo->vcpus[i].vcpuid;
         vcpus[i].u.rtds.period = scinfo->vcpus[i].period;
         vcpus[i].u.rtds.budget = scinfo->vcpus[i].budget;
+        if (scinfo->vcpus[i].extratime)
+            vcpus[i].u.rtds.flags |= XEN_DOMCTL_SCHEDRT_extra;
+        else
+            vcpus[i].u.rtds.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
     }
 
     r = xc_sched_rtds_vcpu_set(CTX->xch, domid,
@@ -655,6 +663,10 @@ static int sched_rtds_vcpu_set_all(libxl__gc *gc, uint32_t domid,
         vcpus[i].vcpuid = i;
         vcpus[i].u.rtds.period = scinfo->vcpus[0].period;
         vcpus[i].u.rtds.budget = scinfo->vcpus[0].budget;
+        if (scinfo->vcpus[0].extratime)
+            vcpus[i].u.rtds.flags |= XEN_DOMCTL_SCHEDRT_extra;
+        else
+            vcpus[i].u.rtds.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
     }
 
     r = xc_sched_rtds_vcpu_set(CTX->xch, domid,
@@ -705,6 +717,12 @@ static int sched_rtds_domain_set(libxl__gc *gc, uint32_t domid,
         sdom.period = scinfo->period;
     if (scinfo->budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT)
         sdom.budget = scinfo->budget;
+    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
+        if (scinfo->extratime)
+            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
+        else
+            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
+    }
     if (sched_rtds_validate_params(gc, sdom.period, sdom.budget))
         return ERROR_INVAL;
 
-- 
1.9.1


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

[PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS

[Meng Xu]

Change main_sched_rtds and related output functions to support
per-VCPU extratime flag.

Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>

---
Changes from v1
No change because we agree on using -e 0/1 option to
set if a vcpu will get extra time or not

Changes from RFC v1
Changes work_conserving flag to extratime flag
---
 tools/xl/xl_cmdtable.c |  3 ++-
 tools/xl/xl_sched.c    | 56 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 19 deletions(-)
---
 tools/xl/xl_cmdtable.c |  3 ++-
 tools/xl/xl_sched.c    | 56 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index ba0159d..1b03d44 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -272,12 +272,13 @@ struct cmd_spec cmd_table[] = {
     { "sched-rtds",
       &main_sched_rtds, 0, 1,
       "Get/set rtds scheduler parameters",
-      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]]]",
+      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]] [-e[=EXTRATIME]]]",
       "-d DOMAIN, --domain=DOMAIN     Domain to modify\n"
       "-v VCPUID/all, --vcpuid=VCPUID/all    VCPU to modify or output;\n"
       "               Using '-v all' to modify/output all vcpus\n"
       "-p PERIOD, --period=PERIOD     Period (us)\n"
       "-b BUDGET, --budget=BUDGET     Budget (us)\n"
+      "-e EXTRATIME, --extratime=EXTRATIME EXTRATIME (1=yes, 0=no)\n"
     },
     { "domid",
       &main_domid, 0, 0,
diff --git a/tools/xl/xl_sched.c b/tools/xl/xl_sched.c
index 85722fe..5138012 100644
--- a/tools/xl/xl_sched.c
+++ b/tools/xl/xl_sched.c
@@ -251,7 +251,7 @@ static int sched_rtds_domain_output(
     libxl_domain_sched_params scinfo;
 
     if (domid < 0) {
-        printf("%-33s %4s %9s %9s\n", "Name", "ID", "Period", "Budget");
+        printf("%-33s %4s %9s %9s %10s\n", "Name", "ID", "Period", "Budget", "Extra time");
         return 0;
     }
 
@@ -262,11 +262,12 @@ static int sched_rtds_domain_output(
     }
 
     domname = libxl_domid_to_name(ctx, domid);
-    printf("%-33s %4d %9d %9d\n",
+    printf("%-33s %4d %9d %9d %10s\n",
         domname,
         domid,
         scinfo.period,
-        scinfo.budget);
+        scinfo.budget,
+        scinfo.extratime ? "yes" : "no");
     free(domname);
     libxl_domain_sched_params_dispose(&scinfo);
     return 0;
@@ -279,8 +280,8 @@ static int sched_rtds_vcpu_output(int domid, libxl_vcpu_sched_params *scinfo)
     int i;
 
     if (domid < 0) {
-        printf("%-33s %4s %4s %9s %9s\n", "Name", "ID",
-               "VCPU", "Period", "Budget");
+        printf("%-33s %4s %4s %9s %9s %10s\n", "Name", "ID",
+               "VCPU", "Period", "Budget", "Extra time");
         return 0;
     }
 
@@ -290,12 +291,13 @@ static int sched_rtds_vcpu_output(int domid, libxl_vcpu_sched_params *scinfo)
 
     domname = libxl_domid_to_name(ctx, domid);
     for ( i = 0; i < scinfo->num_vcpus; i++ ) {
-        printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32"\n",
+        printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32" %10s\n",
                domname,
                domid,
                scinfo->vcpus[i].vcpuid,
                scinfo->vcpus[i].period,
-               scinfo->vcpus[i].budget);
+               scinfo->vcpus[i].budget,
+               scinfo->vcpus[i].extratime ? "yes" : "no");
     }
     free(domname);
     return 0;
@@ -309,8 +311,8 @@ static int sched_rtds_vcpu_output_all(int domid,
     int i;
 
     if (domid < 0) {
-        printf("%-33s %4s %4s %9s %9s\n", "Name", "ID",
-               "VCPU", "Period", "Budget");
+        printf("%-33s %4s %4s %9s %9s %10s\n", "Name", "ID",
+               "VCPU", "Period", "Budget", "Extra time");
         return 0;
     }
 
@@ -321,12 +323,13 @@ static int sched_rtds_vcpu_output_all(int domid,
 
     domname = libxl_domid_to_name(ctx, domid);
     for ( i = 0; i < scinfo->num_vcpus; i++ ) {
-        printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32"\n",
+        printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32" %10s\n",
                domname,
                domid,
                scinfo->vcpus[i].vcpuid,
                scinfo->vcpus[i].period,
-               scinfo->vcpus[i].budget);
+               scinfo->vcpus[i].budget,
+               scinfo->vcpus[i].extratime ? "yes" : "no");
     }
     free(domname);
     return 0;
@@ -702,14 +705,18 @@ int main_sched_rtds(int argc, char **argv)
     int *vcpus = (int *)xmalloc(sizeof(int)); /* IDs of VCPUs that change */
     int *periods = (int *)xmalloc(sizeof(int)); /* period is in microsecond */
     int *budgets = (int *)xmalloc(sizeof(int)); /* budget is in microsecond */
+    bool *extratimes = (bool *)xmalloc(sizeof(bool)); /* extratime is bool */
     int v_size = 1; /* size of vcpus array */
     int p_size = 1; /* size of periods array */
     int b_size = 1; /* size of budgets array */
+    int e_size = 1; /* size of extratimes array */
     int v_index = 0; /* index in vcpus array */
     int p_index =0; /* index in periods array */
     int b_index =0; /* index for in budgets array */
+    int e_index = 0; /* index in extratimes array */
     bool opt_p = false;
     bool opt_b = false;
+    bool opt_e = false;
     bool opt_v = false;
     bool opt_all = false; /* output per-dom parameters */
     int opt, i, rc, r;
@@ -717,12 +724,13 @@ int main_sched_rtds(int argc, char **argv)
         {"domain", 1, 0, 'd'},
         {"period", 1, 0, 'p'},
         {"budget", 1, 0, 'b'},
+        {"extratime", 1, 0, 'e'},
         {"vcpuid",1, 0, 'v'},
         {"cpupool", 1, 0, 'c'},
         COMMON_LONG_OPTS
     };
 
-    SWITCH_FOREACH_OPT(opt, "d:p:b:v:c", opts, "sched-rtds", 0) {
+    SWITCH_FOREACH_OPT(opt, "d:p:b:e:v:c", opts, "sched-rtds", 0) {
     case 'd':
         dom = optarg;
         break;
@@ -746,6 +754,14 @@ int main_sched_rtds(int argc, char **argv)
         budgets[b_index++] = strtol(optarg, NULL, 10);
         opt_b = 1;
         break;
+    case 'e':
+        if (e_index >= e_size) { /* extratime array is full */
+            e_size *= 2;
+            extratimes = xrealloc(extratimes, e_size);
+        }
+        extratimes[e_index++] = strtol(optarg, NULL, 10);
+        opt_e = 1;
+        break;
     case 'v':
         if (!strcmp(optarg, "all")) { /* get or set all vcpus of a domain */
             opt_all = 1;
@@ -763,18 +779,18 @@ int main_sched_rtds(int argc, char **argv)
         break;
     }
 
-    if (cpupool && (dom || opt_p || opt_b || opt_v || opt_all)) {
+    if (cpupool && (dom || opt_p || opt_b || opt_e || opt_v || opt_all)) {
         fprintf(stderr, "Specifying a cpupool is not allowed with "
                 "other options.\n");
         r = EXIT_FAILURE;
         goto out;
     }
-    if (!dom && (opt_p || opt_b || opt_v)) {
+    if (!dom && (opt_p || opt_b || opt_e || opt_v)) {
         fprintf(stderr, "Missing parameters.\n");
         r = EXIT_FAILURE;
         goto out;
     }
-    if (dom && !opt_v && !opt_all && (opt_p || opt_b)) {
+    if (dom && !opt_v && !opt_all && (opt_p || opt_b || opt_e)) {
         fprintf(stderr, "Must specify VCPU.\n");
         r = EXIT_FAILURE;
         goto out;
@@ -785,8 +801,9 @@ int main_sched_rtds(int argc, char **argv)
         goto out;
     }
     if (((v_index > b_index) && opt_b) || ((v_index > p_index) && opt_p)
-        || p_index != b_index) {
-        fprintf(stderr, "Incorrect number of period and budget\n");
+         || ((v_index > e_index) && opt_e) || p_index != b_index
+         || p_index != e_index || b_index != e_index ) {
+        fprintf(stderr, "Incorrect number of period, budget and extratime\n");
         r = EXIT_FAILURE;
         goto out;
     }
@@ -820,7 +837,7 @@ int main_sched_rtds(int argc, char **argv)
                 r = EXIT_FAILURE;
                 goto out;
             }
-        } else if (!opt_p && !opt_b) {
+        } else if (!opt_p && !opt_b && !opt_e) {
             /* get per-vcpu rtds scheduling parameters */
             libxl_vcpu_sched_params scinfo;
             libxl_vcpu_sched_params_init(&scinfo);
@@ -852,6 +869,7 @@ int main_sched_rtds(int argc, char **argv)
                     scinfo.vcpus[i].vcpuid = vcpus[i];
                     scinfo.vcpus[i].period = periods[i];
                     scinfo.vcpus[i].budget = budgets[i];
+                    scinfo.vcpus[i].extratime = extratimes[i] ? 1 : 0;
                 }
                 rc = sched_vcpu_set(domid, &scinfo);
             } else { /* set params for all vcpus */
@@ -860,6 +878,7 @@ int main_sched_rtds(int argc, char **argv)
                                xmalloc(sizeof(libxl_sched_params));
                 scinfo.vcpus[0].period = periods[0];
                 scinfo.vcpus[0].budget = budgets[0];
+                scinfo.vcpus[0].extratime = extratimes[0] ? 1 : 0;
                 rc = sched_vcpu_set_all(domid, &scinfo);
             }
 
@@ -876,6 +895,7 @@ out:
     free(vcpus);
     free(periods);
     free(budgets);
+    free(extratimes);
     return r;
 }
 
-- 
1.9.1


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

[PATCH v2 4/5] xentrace: enable per-VCPU extratime flag for RTDS

[Meng Xu]

Change repl_budget event output for xentrace formats and xenalyze

Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>

---
Changes from v1
Add this changes from v1
---
 tools/xentrace/formats    | 2 +-
 tools/xentrace/xenalyze.c | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/xentrace/formats b/tools/xentrace/formats
index f39182a..470ac5c 100644
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -75,7 +75,7 @@
 0x00022801  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:tickle        [ cpu = %(1)d ]
 0x00022802  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:runq_pick     [ dom:vcpu = 0x%(1)08x, cur_deadline = 0x%(3)08x%(2)08x, cur_budget = 0x%(5)08x%(4)08x ]
 0x00022803  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:burn_budget   [ dom:vcpu = 0x%(1)08x, cur_budget = 0x%(3)08x%(2)08x, delta = %(4)d ]
-0x00022804  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:repl_budget   [ dom:vcpu = 0x%(1)08x, cur_deadline = 0x%(3)08x%(2)08x, cur_budget = 0x%(5)08x%(4)08x ]
+0x00022804  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:repl_budget   [ dom:vcpu = 0x%(1)08x, priority_level = 0x%(2)08d cur_deadline = 0x%(4)08x%(3)08x, cur_budget = 0x%(6)08x%(5)08x ]
 0x00022805  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:sched_tasklet
 0x00022806  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:schedule      [ cpu[16]:tasklet[8]:idle[4]:tickled[4] = %(1)08x ]
 
diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 39fc35f..6fb952c 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -7944,12 +7944,14 @@ void sched_process(struct pcpu_info *p)
             if(opt.dump_all) {
                 struct {
                     unsigned int vcpuid:16, domid:16;
+                    unsigned int priority_level;
                     uint64_t cur_dl, cur_bg;
                 } __attribute__((packed)) *r = (typeof(r))ri->d;
 
-                printf(" %s rtds:repl_budget d%uv%u, deadline = %"PRIu64", "
-                       "budget = %"PRIu64"\n", ri->dump_header,
-                       r->domid, r->vcpuid, r->cur_dl, r->cur_bg);
+                printf(" %s rtds:repl_budget d%uv%u, priority_level = %u,"
+                       "deadline = %"PRIu64", budget = %"PRIu64"\n",
+                       ri->dump_header, r->domid, r->vcpuid,
+                       r->priority_level, r->cur_dl, r->cur_bg);
             }
             break;
         case TRC_SCHED_CLASS_EVT(RTDS, 5): /* SCHED_TASKLET    */
-- 
1.9.1


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

[PATCH v2 5/5] docs: enable per-VCPU extratime flag for RTDS

[Meng Xu]

Revise xl tool use case by adding -e option
Remove work-conserving from TODO list

Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>

---
Changes from v1
Revise rtds docs
---
 docs/features/sched_rtds.pandoc | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/docs/features/sched_rtds.pandoc b/docs/features/sched_rtds.pandoc
index 354097b..d51b499 100644
--- a/docs/features/sched_rtds.pandoc
+++ b/docs/features/sched_rtds.pandoc
@@ -40,7 +40,7 @@ as follows:
 
 It is possible, for a multiple vCPUs VM, to change the parameters of
 each vCPU individually:
-    * `xl sched-rtds -d vm-rt -v 0 -p 20000 -b 10000 -v 1 -p 45000 -b 12000`
+    * `xl sched-rtds -d vm-rt -v 0 -p 20000 -b 10000 -e 1 -v 1 -p 45000 -b 12000 -e 0`
 
 # Technical details
 
@@ -53,7 +53,8 @@ the presence of the LIBXL\_HAVE\_SCHED\_RTDS symbol. The ability of
 specifying different scheduling parameters for each vcpu has been
 introduced later, and is available if the following symbols are defined:
     * `LIBXL\_HAVE\_VCPU\_SCHED\_PARAMS`,
-    * `LIBXL\_HAVE\_SCHED\_RTDS\_VCPU\_PARAMS`.
+    * `LIBXL\_HAVE\_SCHED\_RTDS\_VCPU\_PARAMS`,
+    * `LIBXL\_HAVE\_SCHED\_RTDS\_VCPU\_EXTRA`.
 
 # Limitations
 
@@ -95,7 +96,6 @@ at a macroscopic level), the following should be done:
 
 # Areas for improvement
 
-* Work-conserving mode to be added;
 * performance assessment, especially focusing on what level of real-time
   behavior the scheduler enables.
 
@@ -118,4 +118,5 @@ at a macroscopic level), the following should be done:
 Date       Revision Version  Notes
 ---------- -------- -------- -------------------------------------------
 2016-10-14 1        Xen 4.8  Document written
+2017-08-31 2        Xen 4.10 Revise for work conserving feature
 ---------- -------- -------- -------------------------------------------
-- 
1.9.1


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

Re: [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Meng Xu]

Dario,

I didn't include your Reviewed-by tag because I made one small change.


On Fri, Sep 1, 2017 at 11:58 AM, Meng Xu <mengxu@cis.upenn.edu> wrote:
>
> Modify libxl_vcpu_sched_params_get/set and sched_rtds_vcpu_get/set
> functions to support per-VCPU extratime flag
>
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
>
> ---
> Changes from v1
> 1) Add LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA to indicate if extratime flag is
> supported
> 2) Change flag name in domctl.h from XEN_DOMCTL_SCHED_RTDS_extratime to
> XEN_DOMCTL_SCHEDRT_extra
>
> Changes from RFC v1
> Change work_conserving flag to extratime flag
> ---
>  tools/libxl/libxl_sched.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> ---
>  tools/libxl/libxl.h       |  6 ++++++
>  tools/libxl/libxl_sched.c | 18 ++++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 1704525..ead300f 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -257,6 +257,12 @@
>  #define LIBXL_HAVE_SCHED_RTDS_VCPU_PARAMS 1
>
>  /*
> + * LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA indicates RTDS scheduler
> + * now supports per-vcpu extratime settings.
> + */
> +#define LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA 1
> +
> +/*
>   * libxl_domain_build_info has the arm.gic_version field.
>   */
>  #define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 1
> diff --git a/tools/libxl/libxl_sched.c b/tools/libxl/libxl_sched.c
> index faa604e..b76a29a 100644
> --- a/tools/libxl/libxl_sched.c
> +++ b/tools/libxl/libxl_sched.c
> @@ -558,6 +558,10 @@ static int sched_rtds_vcpu_get_all(libxl__gc *gc, uint32_t domid,
>      for (i = 0; i < num_vcpus; i++) {
>          scinfo->vcpus[i].period = vcpus[i].u.rtds.period;
>          scinfo->vcpus[i].budget = vcpus[i].u.rtds.budget;
> +        if (vcpus[i].u.rtds.flags & XEN_DOMCTL_SCHEDRT_extra)
> +           scinfo->vcpus[i].extratime = 1;
> +        else
> +           scinfo->vcpus[i].extratime = 0;
>          scinfo->vcpus[i].vcpuid = vcpus[i].vcpuid;
>      }
>      rc = 0;
> @@ -607,6 +611,10 @@ static int sched_rtds_vcpu_set(libxl__gc *gc, uint32_t domid,
>          vcpus[i].vcpuid = scinfo->vcpus[i].vcpuid;
>          vcpus[i].u.rtds.period = scinfo->vcpus[i].period;
>          vcpus[i].u.rtds.budget = scinfo->vcpus[i].budget;
> +        if (scinfo->vcpus[i].extratime)
> +            vcpus[i].u.rtds.flags |= XEN_DOMCTL_SCHEDRT_extra;
> +        else
> +            vcpus[i].u.rtds.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
>      }
>
>      r = xc_sched_rtds_vcpu_set(CTX->xch, domid,
> @@ -655,6 +663,10 @@ static int sched_rtds_vcpu_set_all(libxl__gc *gc, uint32_t domid,
>          vcpus[i].vcpuid = i;
>          vcpus[i].u.rtds.period = scinfo->vcpus[0].period;
>          vcpus[i].u.rtds.budget = scinfo->vcpus[0].budget;
> +        if (scinfo->vcpus[0].extratime)
> +            vcpus[i].u.rtds.flags |= XEN_DOMCTL_SCHEDRT_extra;
> +        else
> +            vcpus[i].u.rtds.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
>      }
>
>      r = xc_sched_rtds_vcpu_set(CTX->xch, domid,
> @@ -705,6 +717,12 @@ static int sched_rtds_domain_set(libxl__gc *gc, uint32_t domid,
>          sdom.period = scinfo->period;
>      if (scinfo->budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT)
>          sdom.budget = scinfo->budget;
> +    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
> +        if (scinfo->extratime)
> +            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
> +        else
> +            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
> +    }
>      if (sched_rtds_validate_params(gc, sdom.period, sdom.budget))
>          return ERROR_INVAL;


As you mentioned in the comment to the xl patch v1, I used
LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT for extratime flag as what
we did for period and budget. But the way we handle flags is exactly
the same with the way we handle period and budget.

I'm ok with what it is in this patch, although I feel that we can kill the
 if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
because LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT is -1.


What do you think?

Thanks,

Meng


-- 
-----------
Meng Xu
PhD Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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

Re: [PATCH v2 0/5] Towards work-conserving RTDS

[Wei Liu]

On Fri, Sep 01, 2017 at 11:58:47AM -0400, Meng Xu wrote:
> This series of patches make RTDS scheduler work-conserving
> without breaking real-time guarantees.
> VCPUs with extratime flag set can get extra time
> from the unreserved system resource.
> System administrators can decide which VCPUs have extratime flag set.
> 
> Example:
> Set the extratime bit of all VCPUs of domain 1:
> # xl sched-rtds -d 1 -v all -p 10000 -b 2000 -e 1
> Each VCPU of domain 1 will be guaranteed to have 2000ms every 10000ms
> (if the system is schedulable).
> If there is a CPU having no work to do,
> domain 1's VCPUs will be scheduled onto the CPU,
> even though the VCPUs have got 2000ms in 10000ms.
> 
> Clear the extra bit of all VCPUs of domain 1:
> # xl sched-rtds -d 1 -v all -p 10000 -b 2000 -e 0
> 
> Set/Clear the extratime bit of one specific VCPU of domain 1:
> # xl sched-rtds -d 1 -v 1 -p 10000 -b 2000 -e 1
> # xl sched-rtds -d 1 -v 1 -p 10000 -b 2000 -e 0
> 
> 
> The original design of the work-conserving RTDS was discussed at
> https://www.mail-archive.com/xen-devel@lists.xen.org/msg77150.html
> 
> The first version was discussed at
> https://www.mail-archive.com/xen-devel@lists.xen.org/msg117361.html
> 
> The series of patch can be found at github:
> https://github.com/PennPanda/RT-Xen
> under the branch:
> xenbits/rtds/work-conserving-v2
> 
> Changes from v1
> Change XEN_DOMCTL_SCHED_RTDS_extratime to XEN_DOMCTL_SCHEDRT_extra
> Revise xentrace, xenalyze, and docs
> Add LIBXL_HAVE_SCHED_RTDS_VCPU_EXTRA symbol in libxl.h
> 
> Changes from RFC v1
> Merge changes in sched_rt.c into one patch;
> Minor change in variable name and comments.
> 
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> 
> [PATCH v2 1/5] xen:rtds: towards work conserving RTDS
> [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS
> [PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS
> [PATCH v2 4/5] xentrace: enable per-VCPU extratime flag for RTDS
> [PATCH v2 5/5] docs: enable per-VCPU extratime flag for RTDS
> 

FAOD I will leave this series to Dario. Most of the code looks
mechanical.

P.S. You got my email address wrong, otherwise I would have noticed
series sooner.

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

Re: [PATCH v2 0/5] Towards work-conserving RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 919 bytes --]

On Wed, 2017-09-13 at 11:28 +0100, Wei Liu wrote:
> On Fri, Sep 01, 2017 at 11:58:47AM -0400, Meng Xu wrote:
> > Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> > 
> > [PATCH v2 1/5] xen:rtds: towards work conserving RTDS
> > [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS
> > [PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS
> > [PATCH v2 4/5] xentrace: enable per-VCPU extratime flag for RTDS
> > [PATCH v2 5/5] docs: enable per-VCPU extratime flag for RTDS
> > 
> 
> FAOD I will leave this series to Dario. Most of the code looks
> mechanical.
> 
Yes, this is on my radar. I'll be able to look at it shortly.

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1586 bytes --]

On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> Modify libxl_vcpu_sched_params_get/set and sched_rtds_vcpu_get/set
> functions to support per-VCPU extratime flag
> 
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> 
This patch looks ok to me.

Only one thing: in libxl_types.idl is, when its inside
libxl_domain_sched_params, marked as deprecated. I think it should be
moved out of that.

One (very) minor thing too:

> diff --git a/tools/libxl/libxl_sched.c b/tools/libxl/libxl_sched.c
> index faa604e..b76a29a 100644
> --- a/tools/libxl/libxl_sched.c
> +++ b/tools/libxl/libxl_sched.c
> @@ -558,6 +558,10 @@ static int sched_rtds_vcpu_get_all(libxl__gc
> *gc, uint32_t domid,
>      for (i = 0; i < num_vcpus; i++) {
>          scinfo->vcpus[i].period = vcpus[i].u.rtds.period;
>          scinfo->vcpus[i].budget = vcpus[i].u.rtds.budget;
> +        if (vcpus[i].u.rtds.flags & XEN_DOMCTL_SCHEDRT_extra)
> +           scinfo->vcpus[i].extratime = 1;
> +        else
> +           scinfo->vcpus[i].extratime = 0;
>
This can be:

 scinfo->vcpus[i].extratime = vcpus[i].u.rtds.flags & XEN_DOMCTL_SCHEDRT_extra ? 1 : 0

or:

 scinfo->vcpus[i].extratime = !!(vcpus[i].u.rtds.flags & XEN_DOMCTL_SCHEDRT_extra);

Regards,
Dario 
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1714 bytes --]

On Fri, 2017-09-01 at 12:03 -0400, Meng Xu wrote:
> On Fri, Sep 1, 2017 at 11:58 AM, Meng Xu <mengxu@cis.upenn.edu>
> wrote:
> > @@ -705,6 +717,12 @@ static int sched_rtds_domain_set(libxl__gc
> > *gc, uint32_t domid,
> >          sdom.period = scinfo->period;
> >      if (scinfo->budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT)
> >          sdom.budget = scinfo->budget;
> > +    if (scinfo->extratime !=
> > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
> > +        if (scinfo->extratime)
> > +            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
> > +        else
> > +            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
> > +    }
> >      if (sched_rtds_validate_params(gc, sdom.period, sdom.budget))
> >          return ERROR_INVAL;
> 
> 
> As you mentioned in the comment to the xl patch v1, I used
> LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT for extratime flag as what
> we did for period and budget. But the way we handle flags is exactly
> the same with the way we handle period and budget.
> 
Mmm... and (since you say 'But') is that a problem?

> I'm ok with what it is in this patch, although I feel that we can
> kill the
>  if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> because LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT is -1.
> 
No, sorry, I don't understand what you mean here...

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 4330 bytes --]

On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index ba0159d..1b03d44 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -272,12 +272,13 @@ struct cmd_spec cmd_table[] = {
>      { "sched-rtds",
>        &main_sched_rtds, 0, 1,
>        "Get/set rtds scheduler parameters",
> -      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]]]",
> +      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]] [-
> e[=EXTRATIME]]]",
>        "-d DOMAIN, --domain=DOMAIN     Domain to modify\n"
>        "-v VCPUID/all, --vcpuid=VCPUID/all    VCPU to modify or
> output;\n"
>        "               Using '-v all' to modify/output all vcpus\n"
>        "-p PERIOD, --period=PERIOD     Period (us)\n"
>        "-b BUDGET, --budget=BUDGET     Budget (us)\n"
> +      "-e EXTRATIME, --extratime=EXTRATIME EXTRATIME (1=yes, 0=no)\n"
                                              Extratime
?

>      },
>      { "domid",
>        &main_domid, 0, 0,
> diff --git a/tools/xl/xl_sched.c b/tools/xl/xl_sched.c
> index 85722fe..5138012 100644
> --- a/tools/xl/xl_sched.c
> +++ b/tools/xl/xl_sched.c
> @@ -251,7 +251,7 @@ static int sched_rtds_domain_output(
>      libxl_domain_sched_params scinfo;
>  
>      if (domid < 0) {
> -        printf("%-33s %4s %9s %9s\n", "Name", "ID", "Period",
> "Budget");
> +        printf("%-33s %4s %9s %9s %10s\n", "Name", "ID", "Period",
> "Budget", "Extra time");
>          return 0;
>      }
>  
Can you paste the output of:

xl sched-rtds
xl sched-rtds -d 0
xl sched-rtds -d 0 -v 1
xl sched-rtds -d 0 -v all

with the series applied?

> @@ -785,8 +801,9 @@ int main_sched_rtds(int argc, char **argv)
>          goto out;
>      }
>      if (((v_index > b_index) && opt_b) || ((v_index > p_index) &&
> opt_p)
> -        || p_index != b_index) {
> -        fprintf(stderr, "Incorrect number of period and budget\n");
> +         || ((v_index > e_index) && opt_e) || p_index != b_index
> +         || p_index != e_index || b_index != e_index ) {
>
I don't think you need the `b_indes ! e_index` part. If p==b and p==e,
it's automatically true that b==e.

> @@ -820,7 +837,7 @@ int main_sched_rtds(int argc, char **argv)
>                  r = EXIT_FAILURE;
>                  goto out;
>              }
> -        } else if (!opt_p && !opt_b) {
> +        } else if (!opt_p && !opt_b && !opt_e) {
>              /* get per-vcpu rtds scheduling parameters */
>              libxl_vcpu_sched_params scinfo;
>              libxl_vcpu_sched_params_init(&scinfo);
> @@ -852,6 +869,7 @@ int main_sched_rtds(int argc, char **argv)
>                      scinfo.vcpus[i].vcpuid = vcpus[i];
>                      scinfo.vcpus[i].period = periods[i];
>                      scinfo.vcpus[i].budget = budgets[i];
> +                    scinfo.vcpus[i].extratime = extratimes[i] ? 1 :
> 0;
>                  }
>                  rc = sched_vcpu_set(domid, &scinfo);
>              } else { /* set params for all vcpus */
> @@ -860,6 +878,7 @@ int main_sched_rtds(int argc, char **argv)
>                                 xmalloc(sizeof(libxl_sched_params));
>                  scinfo.vcpus[0].period = periods[0];
>                  scinfo.vcpus[0].budget = budgets[0];
> +                scinfo.vcpus[0].extratime = extratimes[0] ? 1 : 0;
>
But does these two hunks mean that if I pass `-e 10`, that is
considered a legal way to enable extratime? Shouldn't we enforce
(either here in xl or in libxl) the value to be 0 or 1 ?

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 4/5] xentrace: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1587 bytes --]

On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> Change repl_budget event output for xentrace formats and xenalyze
> 
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> 
Right, thanks for doing this part too. However, can you merge this
inside patch 1?

> diff --git a/tools/xentrace/formats b/tools/xentrace/formats
> index f39182a..470ac5c 100644
> --- a/tools/xentrace/formats
> +++ b/tools/xentrace/formats
> @@ -75,7 +75,7 @@
>  0x00022801  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:tickle        [ cpu = %(1)d ]
>  0x00022802  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:runq_pick     [ dom:vcpu = 0x%(1)08x, cur_deadline = 0x%(3)08x%(2)08x, cur_budget = 0x%(5)08x%(4)08x ]
>  0x00022803  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:burn_budget   [ dom:vcpu = 0x%(1)08x, cur_budget = 0x%(3)08x%(2)08x, delta = %(4)d ]
> -0x00022804  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:repl_budget   [ dom:vcpu = 0x%(1)08x, cur_deadline = 0x%(3)08x%(2)08x, cur_budget = 0x%(5)08x%(4)08x ]
> +0x00022804  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:repl_budget   [ dom:vcpu = 0x%(1)08x, priority_level = 0x%(2)08d cur_deadline = 0x%(4)08x%(3)08x, cur_budget = 0x%(6)08x%(5)08x ]
>
Why 0x%(2)08d, and not just %(2)d ?

Have you tested it? What does it print?

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 668 bytes --]

On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> Change main_sched_rtds and related output functions to support
> per-VCPU extratime flag.
> 
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> ---
>  tools/xl/xl_cmdtable.c |  3 ++-
>  tools/xl/xl_sched.c    | 56 ++++++++++++++++++++++++++++++++++----
> ------------
>
Oh, and this patch must update docs/man/xl.pod.1.in as well.

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 5/5] docs: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 530 bytes --]

On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> Revise xl tool use case by adding -e option
> Remove work-conserving from TODO list
> 
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> 
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 1/5] xen:rtds: towards work conserving RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2014 bytes --]

On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> Make RTDS scheduler work conserving without breaking the real-time
> guarantees.
> 
> VCPU model:
> Each real-time VCPU is extended to have an extratime flag
> and a priority_level field.
> When a VCPU's budget is depleted in the current period,
> if it has extratime flag set,
> its priority_level will increase by 1 and its budget will be
> refilled;
> othewrise, the VCPU will be moved to the depletedq.
> 
> Scheduling policy is modified global EDF:
> A VCPU v1 has higher priority than another VCPU v2 if
> (i) v1 has smaller priority_leve; or
> (ii) v1 has the same priority_level but has a smaller deadline
> 
> Queue management:
> Run queue holds VCPUs with extratime flag set and VCPUs with
> remaining budget. Run queue is sorted in increasing order of VCPUs
> priorities.
> Depleted queue holds VCPUs which have extratime flag cleared and
> depleted budget.
> Replenished queue is not modified.
> 
Mmm.. didn't we agree about putting a word of explanation of how the
spare bandwidth ends up being distributed (i.e., in a way which is
proportional to the utilization)? Or is it there and it's me that am
not finding it?

> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c 
> @@ -245,6 +258,11 @@ static inline struct list_head *rt_replq(const
> struct scheduler *ops)
>      return &rt_priv(ops)->replq;
>  }
>  
> +static inline bool has_extratime(const struct rt_vcpu *svc)
> +{
> +    return (svc->flags & RTDS_extratime) ? 1 : 0;
> +}
> +
'true' and 'false'. But I think

 return svc->flags & RTDS_extratime

is just fine already, without any need for the ?: operator.

The rest of the patch looks fine to me.

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 1/5] xen:rtds: towards work conserving RTDS

[Meng Xu]

Hi Dario,

On Wed, Sep 13, 2017 at 9:06 PM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
>
> On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> > Make RTDS scheduler work conserving without breaking the real-time
> > guarantees.
> >
> > VCPU model:
> > Each real-time VCPU is extended to have an extratime flag
> > and a priority_level field.
> > When a VCPU's budget is depleted in the current period,
> > if it has extratime flag set,
> > its priority_level will increase by 1 and its budget will be
> > refilled;
> > othewrise, the VCPU will be moved to the depletedq.
> >
> > Scheduling policy is modified global EDF:
> > A VCPU v1 has higher priority than another VCPU v2 if
> > (i) v1 has smaller priority_leve; or
> > (ii) v1 has the same priority_level but has a smaller deadline
> >
> > Queue management:
> > Run queue holds VCPUs with extratime flag set and VCPUs with
> > remaining budget. Run queue is sorted in increasing order of VCPUs
> > priorities.
> > Depleted queue holds VCPUs which have extratime flag cleared and
> > depleted budget.
> > Replenished queue is not modified.
> >
> Mmm.. didn't we agree about putting a word of explanation of how the
> spare bandwidth ends up being distributed (i.e., in a way which is
> proportional to the utilization)?


I didn't recall that. I should have double checked that I have
resolved every comment in previous patch.
Anyway, I added the comment in the next version, which is comming soon.

>
> Or is it there and it's me that am
> not finding it?
>
> > --- a/xen/common/sched_rt.c
> > +++ b/xen/common/sched_rt.c
> > @@ -245,6 +258,11 @@ static inline struct list_head *rt_replq(const
> > struct scheduler *ops)
> >      return &rt_priv(ops)->replq;
> >  }
> >
> > +static inline bool has_extratime(const struct rt_vcpu *svc)
> > +{
> > +    return (svc->flags & RTDS_extratime) ? 1 : 0;
> > +}
> > +
> 'true' and 'false'. But I think
>
>  return svc->flags & RTDS_extratime
>
> is just fine already, without any need for the ?: operator.


OK. Either works for me. I will go with
return svc->flags & RTDS_extratime.
>
>
> The rest of the patch looks fine to me.


Thanks,

Meng

-- 
Meng Xu
Ph.D. Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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

Re: [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Meng Xu]

On Wed, Sep 13, 2017 at 8:16 PM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Fri, 2017-09-01 at 12:03 -0400, Meng Xu wrote:
>> On Fri, Sep 1, 2017 at 11:58 AM, Meng Xu <mengxu@cis.upenn.edu>
>> wrote:
>> > @@ -705,6 +717,12 @@ static int sched_rtds_domain_set(libxl__gc
>> > *gc, uint32_t domid,
>> >          sdom.period = scinfo->period;
>> >      if (scinfo->budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT)
>> >          sdom.budget = scinfo->budget;
>> > +    if (scinfo->extratime !=
>> > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
>> > +        if (scinfo->extratime)
>> > +            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
>> > +        else
>> > +            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
>> > +    }
>> >      if (sched_rtds_validate_params(gc, sdom.period, sdom.budget))
>> >          return ERROR_INVAL;
>>
>>
>> As you mentioned in the comment to the xl patch v1, I used
>> LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT for extratime flag as what
>> we did for period and budget. But the way we handle flags is exactly
>> the same with the way we handle period and budget.
>>
> Mmm... and (since you say 'But') is that a problem?

Sorry, the sentence should be "But the way we handle flags is *not* exactly
the same with the way we handle period and budget".
I missed "not" in the previous sentence.

>
>> I'm ok with what it is in this patch, although I feel that we can
>> kill the
>>  if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
>> because LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT is -1.
>>
> No, sorry, I don't understand what you mean here...

I was thinking about the following code:

    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
        if (scinfo->extratime)
            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
        else
            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
    }

This code can be changed to
        if (scinfo->extratime)
            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
        else
            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;

If the extratime uses default value (-1), we still set the extratime flag.

That's why I feel we may kill the
 if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)

Please correct me if I'm wrong.

For the next version, I plan to keep what it is right now. That is:
    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
        if (scinfo->extratime)
            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
        else
            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
    }


Best,

Meng

-- 
Meng Xu
Ph.D. Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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

Re: [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2677 bytes --]

On Fri, 2017-09-15 at 12:01 -0400, Meng Xu wrote:
> On Wed, Sep 13, 2017 at 8:16 PM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
> > 
> > > I'm ok with what it is in this patch, although I feel that we can
> > > kill the
> > >  if (scinfo->extratime !=
> > > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> > > because LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT is -1.
> > > 
> > 
> > No, sorry, I don't understand what you mean here...
> 
> I was thinking about the following code:
> 
>     if (scinfo->extratime !=
> LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
>         if (scinfo->extratime)
>             sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
>         else
>             sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
>     }
> 
> This code can be changed to
>         if (scinfo->extratime)
>             sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
>         else
>             sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
> 
> If the extratime uses default value (-1), we still set the extratime
> flag.
> 
> That's why I feel we may kill the
>  if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> 
Mmm... Ok, I see it now. Well, this is of course all up to the tools'
maintainers.

What I think it would be valauble to ask ourself here is, can, at this
point, scinfo->extratime be equal to
XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT?

And if it is, what does it mean, and what do we want to do?

I mean, if extratime is -1, it means that we've been called, without it
being touched by xl (although, remember that, as a library, libxl can
be linked to and called by other programs too, e.g., libvirt).

If you think that this is a serious programming bug, you can use
XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT to check that, and raise an
assert.

If you think it's an API misuse, you can use it to check for that, and
return an error.

If you think it's just fine, you can do whatever you want to do as
default (which, AFAIUI, it's set the flag). In this case, it's probably
fine to ignore XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT in actual code.
Although, I'd still put a reference to it in a comment, to explain
what's going on, and why we're doing things differently from budget and
period (since _their_ *_DEFAULT are checked).

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 0/5] Towards work-conserving RTDS

[Andrii Anisov]

Hello Meng Xu and Dario,

On 01.09.17 18:58, Meng Xu wrote:
> This series of patches make RTDS scheduler work-conserving
> without breaking real-time guarantees.
> VCPUs with extratime flag set can get extra time
> from the unreserved system resource.
> System administrators can decide which VCPUs have extratime flag set.
As I understand from threads and the code, the work conserving algorithm 
is quite simplistic and will prefer a vcpu with greater utilization.

 From our side we are looking for a bit different solution. I.e., in the 
same cpupool, running vcpus eager for RT characteristics under EDF 
conditions, and share the rest of resources between non-rt vcpus (i.e. 
in a credit manner).
Possible use-case could be a system with a domain hunger for resources, 
but not critical (some infotainment system) and an RT domain utilizing 
at most 20% of a single CPU core. Having a SoC with 4 cores, 
partitioning would be a significant resources wasting for described 
scenario.

How feasible is it from your point of view?

BTW, are you targeting XEN 4.10 with this patch series?

-- 

*Andrii Anisov*



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

Re: [PATCH v2 0/5] Towards work-conserving RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2883 bytes --]

On Mon, 2017-10-02 at 17:38 +0300, Andrii Anisov wrote:
> Hello Meng Xu and Dario,
> 
Hi,

> On 01.09.17 18:58, Meng Xu wrote:
> > This series of patches make RTDS scheduler work-conserving
> > without breaking real-time guarantees.
> > VCPUs with extratime flag set can get extra time
> > from the unreserved system resource.
> > System administrators can decide which VCPUs have extratime flag
> > set.
> 
> As I understand from threads and the code, the work conserving
> algorithm 
> is quite simplistic and will prefer a vcpu with greater utilization.
> 
>  From our side we are looking for a bit different solution. I.e., in
> the 
> same cpupool, running vcpus eager for RT characteristics under EDF 
> conditions, and share the rest of resources between non-rt vcpus
> (i.e. 
> in a credit manner).
> Possible use-case could be a system with a domain hunger for
> resources, 
> but not critical (some infotainment system) and an RT domain
> utilizing 
> at most 20% of a single CPU core. Having a SoC with 4 cores, 
> partitioning would be a significant resources wasting for described 
> scenario.
> 
IMO, this is interesting, but I think the proper way to achieve
something like this is not modify RTDS to also contain something like
Credit, nor to modify Credit to also contain something like RTDS.

The idea I have in mind to serve the use case you're describing is as
follows. Right now, a cpupool can only have a scheduler. If it's RTDS,
all the domains are scheduler with RTDS, if it's Credit, all the
domains are scheduled with Credit, etc.

My idea would be to allow a stack of schedulers in a cpupool.
Basically, you'd configure a cpupool with sched="rtds,credit2" and then
you specify, for each domain, what scheduler you want it to use.

The end result would be that, in the example above, domains scheduler
with Credit2 would run in the time left free by the domains scheduler
by RTDS. E.g., if you have a cpupool with only 1 CPU, an RTDS domain
with P=100,B=20, an RTDS domain with P=1000,B=40, and two Credit2
domains, one with weight 256 and the other with weight 512. Then, the
two RTDS domains will get 20% and 40% of the CPU, while the two Credit2
domains will share the remaining 40% (the one with w=512 getting twice
as much as the one with w=256).

This is kind of similar with what Linux does with scheduling classes,
but even more flexible.

I am not working on implementing this right now, because I'm busy with
other things, but I would like to do that at some point. And if you're
up for helping, that would be great! :-)

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 0/5] Towards work-conserving RTDS

[Meng Xu]

[-- Attachment #1.1: Type: text/plain, Size: 3276 bytes --]

On Mon, Oct 2, 2017 at 1:04 PM, Dario Faggioli <dario.faggioli@citrix.com>
wrote:

> On Mon, 2017-10-02 at 17:38 +0300, Andrii Anisov wrote:
> > Hello Meng Xu and Dario,
> >
> Hi,
>
> > On 01.09.17 18:58, Meng Xu wrote:
> > > This series of patches make RTDS scheduler work-conserving
> > > without breaking real-time guarantees.
> > > VCPUs with extratime flag set can get extra time
> > > from the unreserved system resource.
> > > System administrators can decide which VCPUs have extratime flag
> > > set.
> >
> > As I understand from threads and the code, the work conserving
> > algorithm
> > is quite simplistic and will prefer a vcpu with greater utilization.
> >
> >  From our side we are looking for a bit different solution. I.e., in
> > the
> > same cpupool, running vcpus eager for RT characteristics under EDF
> > conditions, and share the rest of resources between non-rt vcpus
> > (i.e.
> > in a credit manner).
> > Possible use-case could be a system with a domain hunger for
> > resources,
> > but not critical (some infotainment system) and an RT domain
> > utilizing
> > at most 20% of a single CPU core. Having a SoC with 4 cores,
> > partitioning would be a significant resources wasting for described
> > scenario.
> >
> IMO, this is interesting, but I think the proper way to achieve
> something like this is not modify RTDS to also contain something like
> Credit, nor to modify Credit to also contain something like RTDS.
>
> The idea I have in mind to serve the use case you're describing is as
> follows. Right now, a cpupool can only have a scheduler. If it's RTDS,
> all the domains are scheduler with RTDS, if it's Credit, all the
> domains are scheduled with Credit, etc.
>
> My idea would be to allow a stack of schedulers in a cpupool.
> Basically, you'd configure a cpupool with sched="rtds,credit2" and then
> you specify, for each domain, what scheduler you want it to use.
>
> The end result would be that, in the example above, domains scheduler
> with Credit2 would run in the time left free by the domains scheduler
> by RTDS. E.g., if you have a cpupool with only 1 CPU, an RTDS domain
> with P=100,B=20, an RTDS domain with P=1000,B=40, and two Credit2
> domains, one with weight 256 and the other with weight 512. Then, the
> two RTDS domains will get 20% and 40% of the CPU, while the two Credit2
> domains will share the remaining 40% (the one with w=512 getting twice
> as much as the one with w=256).
>
> This is kind of similar with what Linux does with scheduling classes,
> but even more flexible.
>

​I was thinking about Linux scheduling class as well. :)
I think this is a great idea. :)​


> I am not working on implementing this right now, because I'm busy with
> other things, but I would like to do that at some point. And if you're
> up for helping, that would be great! :-)
>

​Right now, I'm with busy with a deadline. I will take care of the
work-conserving RTDS next week.
As to supporting different scheduling class on the same cpupool, I'm not
yet sure when I'm available for this. :(

Best,

Meng
-- 
Meng Xu
Ph.D. Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

[-- Attachment #1.2: Type: text/html, Size: 4794 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Meng Xu]

On Tue, Sep 19, 2017 at 5:23 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
>
> On Fri, 2017-09-15 at 12:01 -0400, Meng Xu wrote:
> > On Wed, Sep 13, 2017 at 8:16 PM, Dario Faggioli
> > <dario.faggioli@citrix.com> wrote:
> > >
> > > > I'm ok with what it is in this patch, although I feel that we can
> > > > kill the
> > > >  if (scinfo->extratime !=
> > > > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> > > > because LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT is -1.
> > > >
> > >
> > > No, sorry, I don't understand what you mean here...
> >
> > I was thinking about the following code:
> >
> >     if (scinfo->extratime !=
> > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
> >         if (scinfo->extratime)
> >             sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
> >         else
> >             sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
> >     }
> >
> > This code can be changed to
> >         if (scinfo->extratime)
> >             sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
> >         else
> >             sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
> >
> > If the extratime uses default value (-1), we still set the extratime
> > flag.
> >
> > That's why I feel we may kill the
> >  if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> >
> Mmm... Ok, I see it now. Well, this is of course all up to the tools'
> maintainers.
>
> What I think it would be valauble to ask ourself here is, can, at this
> point, scinfo->extratime be equal to
> XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT?
>
> And if it is, what does it mean, and what do we want to do?
>
> I mean, if extratime is -1, it means that we've been called, without it
> being touched by xl (although, remember that, as a library, libxl can
> be linked to and called by other programs too, e.g., libvirt).
>
> If you think that this is a serious programming bug, you can use
> XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT to check that, and raise an
> assert.
>
> If you think it's an API misuse, you can use it to check for that, and
> return an error.
>
> If you think it's just fine, you can do whatever you want to do as
> default (which, AFAIUI, it's set the flag). In this case, it's probably
> fine to ignore XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT in actual code.
> Although, I'd still put a reference to it in a comment, to explain
> what's going on, and why we're doing things differently from budget and
> period (since _their_ *_DEFAULT are checked).


I think it should be fine for API to call the function without setting
extratime parameter. We set the extratime by default.

I will go with the following code for the next version.
>         if (scinfo->extratime)
>             sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
>         else
>             sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
>

Thank you very much!

Best,

Meng

-- 
Meng Xu
Ph.D. Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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

Re: [PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS

[Meng Xu]

On Wed, Sep 13, 2017 at 8:51 PM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
>
> On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> > diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> > index ba0159d..1b03d44 100644
> > --- a/tools/xl/xl_cmdtable.c
> > +++ b/tools/xl/xl_cmdtable.c
> > @@ -272,12 +272,13 @@ struct cmd_spec cmd_table[] = {
> >      { "sched-rtds",
> >        &main_sched_rtds, 0, 1,
> >        "Get/set rtds scheduler parameters",
> > -      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]]]",
> > +      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]] [-
> > e[=EXTRATIME]]]",
> >        "-d DOMAIN, --domain=DOMAIN     Domain to modify\n"
> >        "-v VCPUID/all, --vcpuid=VCPUID/all    VCPU to modify or
> > output;\n"
> >        "               Using '-v all' to modify/output all vcpus\n"
> >        "-p PERIOD, --period=PERIOD     Period (us)\n"
> >        "-b BUDGET, --budget=BUDGET     Budget (us)\n"
> > +      "-e EXTRATIME, --extratime=EXTRATIME EXTRATIME (1=yes, 0=no)\n"
>                                               Extratime
> ?

We need to provide the option to configure the extratime flag for each
vcpu, right?

>
> >      },
> >      { "domid",
> >        &main_domid, 0, 0,
> > diff --git a/tools/xl/xl_sched.c b/tools/xl/xl_sched.c
> > index 85722fe..5138012 100644
> > --- a/tools/xl/xl_sched.c
> > +++ b/tools/xl/xl_sched.c
> > @@ -251,7 +251,7 @@ static int sched_rtds_domain_output(
> >      libxl_domain_sched_params scinfo;
> >
> >      if (domid < 0) {
> > -        printf("%-33s %4s %9s %9s\n", "Name", "ID", "Period",
> > "Budget");
> > +        printf("%-33s %4s %9s %9s %10s\n", "Name", "ID", "Period",
> > "Budget", "Extra time");
> >          return 0;
> >      }
> >
> Can you paste the output of:
>

Sure

> xl sched-rtds

Cpupool Pool-0: sched=RTDS
Name                                ID    Period    Budget Extra time
Domain-0                             0     10000      4000        yes

> xl sched-rtds -d 0

Name                                ID    Period    Budget Extra time
Domain-0                             0     10000      4000        yes

> xl sched-rtds -d 0 -v 1

Name                                ID VCPU    Period    Budget Extra time
Domain-0                             0    1     10000      4000        yes


> xl sched-rtds -d 0 -v all

Name                                ID VCPU    Period    Budget Extra time
Domain-0                             0    0     10000      4000        yes
Domain-0                             0    1     10000      4000        yes
Domain-0                             0    2     10000      4000        yes
Domain-0                             0    3     10000      4000        yes
Domain-0                             0    4     10000      4000        yes
Domain-0                             0    5     10000      4000        yes
Domain-0                             0    6     10000      4000        yes
Domain-0                             0    7     10000      4000        yes
Domain-0                             0    8     10000      4000        yes
Domain-0                             0    9     10000      4000        yes
Domain-0                             0   10     10000      4000        yes
Domain-0                             0   11     10000      4000        yes

>
> with the series applied?
>
> > @@ -785,8 +801,9 @@ int main_sched_rtds(int argc, char **argv)
> >          goto out;
> >      }
> >      if (((v_index > b_index) && opt_b) || ((v_index > p_index) &&
> > opt_p)
> > -        || p_index != b_index) {
> > -        fprintf(stderr, "Incorrect number of period and budget\n");
> > +         || ((v_index > e_index) && opt_e) || p_index != b_index
> > +         || p_index != e_index || b_index != e_index ) {
> >
> I don't think you need the `b_indes ! e_index` part. If p==b and p==e,
> it's automatically true that b==e.

Right.

>
> > @@ -820,7 +837,7 @@ int main_sched_rtds(int argc, char **argv)
> >                  r = EXIT_FAILURE;
> >                  goto out;
> >              }
> > -        } else if (!opt_p && !opt_b) {
> > +        } else if (!opt_p && !opt_b && !opt_e) {
> >              /* get per-vcpu rtds scheduling parameters */
> >              libxl_vcpu_sched_params scinfo;
> >              libxl_vcpu_sched_params_init(&scinfo);
> > @@ -852,6 +869,7 @@ int main_sched_rtds(int argc, char **argv)
> >                      scinfo.vcpus[i].vcpuid = vcpus[i];
> >                      scinfo.vcpus[i].period = periods[i];
> >                      scinfo.vcpus[i].budget = budgets[i];
> > +                    scinfo.vcpus[i].extratime = extratimes[i] ? 1 :
> > 0;
> >                  }
> >                  rc = sched_vcpu_set(domid, &scinfo);
> >              } else { /* set params for all vcpus */
> > @@ -860,6 +878,7 @@ int main_sched_rtds(int argc, char **argv)
> >                                 xmalloc(sizeof(libxl_sched_params));
> >                  scinfo.vcpus[0].period = periods[0];
> >                  scinfo.vcpus[0].budget = budgets[0];
> > +                scinfo.vcpus[0].extratime = extratimes[0] ? 1 : 0;
> >
> But does these two hunks mean that if I pass `-e 10`, that is
> considered a legal way to enable extratime? Shouldn't we enforce
> (either here in xl or in libxl) the value to be 0 or 1 ?

Yes, we should enforce the extratime to 0 or 1. How about checking the
value of extratime when we parse each extratime value?
The change of the code will be like the following in xl_sched.c:

757     case 'e':
758         if (e_index >= e_size) { /* extratime array is full */
759             e_size *= 2;
760             extratimes = xrealloc(extratimes, e_size);
761         }
762         extratimes[e_index++] = strtol(optarg, NULL, 10);
763         if ( extratimes[e_index-1] != 0 && extratimes[e_index-1] != 1)
764         {
765             fprintf(stderr, "Invalid extratime.\n");
766             r = EXIT_FAILURE;
767             goto out;
768         }
769         opt_e = 1;
770         break;

What do you think?

Best,

Meng


-- 
Meng Xu
Ph.D. Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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

Re: [PATCH v2 3/5] xl: enable per-VCPU extratime flag for RTDS

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 3406 bytes --]

On Mon, 2017-10-09 at 12:13 -0400, Meng Xu wrote:
> On Wed, Sep 13, 2017 at 8:51 PM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
> > 
> > On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> > > diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> > > index ba0159d..1b03d44 100644
> > > --- a/tools/xl/xl_cmdtable.c
> > > +++ b/tools/xl/xl_cmdtable.c
> > > @@ -272,12 +272,13 @@ struct cmd_spec cmd_table[] = {
> > >      { "sched-rtds",
> > >        &main_sched_rtds, 0, 1,
> > >        "Get/set rtds scheduler parameters",
> > > -      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-
> > > b[=BUDGET]]]",
> > > +      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-
> > > b[=BUDGET]] [-
> > > e[=EXTRATIME]]]",
> > >        "-d DOMAIN, --domain=DOMAIN     Domain to modify\n"
> > >        "-v VCPUID/all, --vcpuid=VCPUID/all    VCPU to modify or
> > > output;\n"
> > >        "               Using '-v all' to modify/output all
> > > vcpus\n"
> > >        "-p PERIOD, --period=PERIOD     Period (us)\n"
> > >        "-b BUDGET, --budget=BUDGET     Budget (us)\n"
> > > +      "-e EXTRATIME, --extratime=EXTRATIME EXTRATIME (1=yes,
> > > 0=no)\n"
> > 
> >                                               Extratime
> > ?
> 
> We need to provide the option to configure the extratime flag for
> each
> vcpu, right?
> 
What I meant is that, that particular word, it should be written
'Extratime' and not 'EXTRATIME'.

> > xl sched-rtds
> 
> Cpupool Pool-0: sched=RTDS
> Name                                ID    Period    Budget Extra time
> Domain-0                             0     10000      4000        yes
> 
Ok (the others as well). I'd use 'Extratime' (no space in between the
two words), but that's not really a big deal

> > > @@ -860,6 +878,7 @@ int main_sched_rtds(int argc, char **argv)
> > >                                 xmalloc(sizeof(libxl_sched_params
> > > ));
> > >                  scinfo.vcpus[0].period = periods[0];
> > >                  scinfo.vcpus[0].budget = budgets[0];
> > > +                scinfo.vcpus[0].extratime = extratimes[0] ? 1 :
> > > 0;
> > > 
> > 
> > But does these two hunks mean that if I pass `-e 10`, that is
> > considered a legal way to enable extratime? Shouldn't we enforce
> > (either here in xl or in libxl) the value to be 0 or 1 ?
> 
> Yes, we should enforce the extratime to 0 or 1. How about checking
> the
> value of extratime when we parse each extratime value?
> The change of the code will be like the following in xl_sched.c:
> 
> 757     case 'e':
> 758         if (e_index >= e_size) { /* extratime array is full */
> 759             e_size *= 2;
> 760             extratimes = xrealloc(extratimes, e_size);
> 761         }
> 762         extratimes[e_index++] = strtol(optarg, NULL, 10);
> 763         if ( extratimes[e_index-1] != 0 && extratimes[e_index-1]
> != 1)
> 764         {
> 765             fprintf(stderr, "Invalid extratime.\n");
> 766             r = EXIT_FAILURE;
> 767             goto out;
> 768         }
> 769         opt_e = 1;
> 770         break;
> 
> What do you think?
> 
Err, yes, this looks fine to me.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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