[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 4/4] xl: introduce rt scheduler



On 09/07/2014 08:41 PM, Meng Xu wrote:
Add xl command for rt scheduler
Note: VCPU's parameter (period, budget) is in microsecond (us).

Signed-off-by: Meng Xu <mengxu@xxxxxxxxxxxxx>
Signed-off-by: Sisu Xi <xisisu@xxxxxxxxx>
---
  docs/man/xl.pod.1         |   34 +++++++++++++
  tools/libxl/xl.h          |    1 +
  tools/libxl/xl_cmdimpl.c  |  119 +++++++++++++++++++++++++++++++++++++++++++++
  tools/libxl/xl_cmdtable.c |    8 +++
  4 files changed, 162 insertions(+)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 9d1c2a5..c2532cb 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1035,6 +1035,40 @@ Restrict output to domains in the specified cpupool.
=back +=item B<sched-rt> [I<OPTIONS>]

sched-rtds, I think.

+
+Set or get rt (Real Time) scheduler parameters. This rt scheduler applies
+Preemptive Global Earliest Deadline First real-time scheduling algorithm to
+schedule VCPUs in the system. Each VCPU has a dedicated period and budget.
+VCPUs in the same domain have the same period and budget (in Xen 4.5).
+While scheduled, a VCPU burns its budget.
+A VCPU has its budget replenished at the beginning of each of its periods;
+The VCPU discards its unused budget at the end of its periods.

I think I would say, "A VCPU has its budget replenished at the beginning of each period; unused budget is discarded at the end of each period."

+
+B<OPTIONS>
+
+=over 4
+
+=item B<-d DOMAIN>, B<--domain=DOMAIN>
+
+Specify domain for which scheduler parameters are to be modified or retrieved.
+Mandatory for modifying scheduler parameters.
+
+=item B<-p PERIOD>, B<--period=PERIOD>
+
+A VCPU replenish its budget in every period. Time unit is millisecond.

I think I'd say: "Period of time, in milliseconds, over which to replenish the budget."

+
+=item B<-b BUDGET>, B<--budget=BUDGET>
+
+A VCPU has BUDGET amount of time to run for each period.
+Time unit is millisecond.

"Amount of time, in milliseconds, that the VCPU will be allowed to run every period."

+
+=item B<-c CPUPOOL>, B<--cpupool=CPUPOOL>
+
+Restrict output to domains in the specified cpupool.
+
+=back
+
  =back
=head1 CPUPOOLS COMMANDS
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 10a2e66..51b634a 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -67,6 +67,7 @@ int main_memset(int argc, char **argv);
  int main_sched_credit(int argc, char **argv);
  int main_sched_credit2(int argc, char **argv);
  int main_sched_sedf(int argc, char **argv);
+int main_sched_rt(int argc, char **argv);

main_sched_rtds

  int main_domid(int argc, char **argv);
  int main_domname(int argc, char **argv);
  int main_rename(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e6b9615..92037b1 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5212,6 +5212,47 @@ static int sched_sedf_domain_output(
      return 0;
  }
+static int sched_rt_domain_output(
+    int domid)
+{
+    char *domname;
+    libxl_domain_sched_params scinfo;
+    int rc = 0;
+
+    if (domid < 0) {
+        printf("%-33s %4s %9s %9s\n", "Name", "ID", "Period", "Budget");
+        return 0;
+    }
+
+    libxl_domain_sched_params_init(&scinfo);
+    rc = sched_domain_get(LIBXL_SCHEDULER_RT_DS, domid, &scinfo);

Hmm, the other callers of sched_domain_get() don't call libxl_domain_sched_params_init(); but reading through libxl.h looks like that's actually a mistake:

 * ...the user must
 * always call the "init" function before using a type, even if the
 * variable is simply being passed by reference as an out parameter
 * to a libxl function.

Meng, would you be willing to put on your "to-do list" to send a follow-up patch to clean this up?

I think what should probably actually be done is that sched_domain_get() should call libxl_domain_sched_params_init() before calling libxl_domain_sched_params_get(). But I'm sure IanJ will have opinions on that.

+    if (rc)
+        goto out;
+
+    domname = libxl_domid_to_name(ctx, domid);
+    printf("%-33s %4d %9d %9d\n",
+        domname,
+        domid,
+        scinfo.period,
+        scinfo.budget);
+    free(domname);
+
+out:
+    libxl_domain_sched_params_dispose(&scinfo);
+    return rc;
+}
+
+static int sched_rt_pool_output(uint32_t poolid)
+{
+    char *poolname;
+
+    poolname = libxl_cpupoolid_to_name(ctx, poolid);
+    printf("Cpupool %s: sched=EDF\n", poolname);

Should we change this to "RTDS"?

+
+    free(poolname);
+    return 0;
+}
+
  static int sched_default_pool_output(uint32_t poolid)
  {
      char *poolname;
@@ -5579,6 +5620,84 @@ int main_sched_sedf(int argc, char **argv)
      return 0;
  }
+/*
+ * <nothing>            : List all domain paramters and sched params
+ * -d [domid]           : List domain params for domain
+ * -d [domid] [params]  : Set domain params for domain
+ */
+int main_sched_rt(int argc, char **argv)
+{
+    const char *dom = NULL;
+    const char *cpupool = NULL;
+    int period = 10, opt_p = 0; /* period is in microsecond */
+    int budget = 4, opt_b = 0; /* budget is in microsecond */

We might as well make opt_p and opt_b  of type "bool".

Why are you setting the values for period and budget here? It looks like they're either never used (if either one or both are not set on the command line), or they're clobbered (when both are set).

If gcc doesn't complain, just leave them uninitizlized. If it does complian, then just initialize them to 0 -- that will make sure that it returns an error if there ever *is* a path which doesn't actually set the value.

+    int opt, rc;
+    static struct option opts[] = {
+        {"domain", 1, 0, 'd'},
+        {"period", 1, 0, 'p'},
+        {"budget", 1, 0, 'b'},
+        {"cpupool", 1, 0, 'c'},
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
+    };
+
+    SWITCH_FOREACH_OPT(opt, "d:p:b:c:h", opts, "sched-rt", 0) {
+    case 'd':
+        dom = optarg;
+        break;
+    case 'p':
+        period = strtol(optarg, NULL, 10);
+        opt_p = 1;
+        break;
+    case 'b':
+        budget = strtol(optarg, NULL, 10);
+        opt_b = 1;
+        break;
+    case 'c':
+        cpupool = optarg;
+        break;
+    }
+
+    if (cpupool && (dom || opt_p || opt_b)) {
+        fprintf(stderr, "Specifying a cpupool is not allowed with other 
options.\n");
+        return 1;
+    }
+    if (!dom && (opt_p || opt_b)) {
+        fprintf(stderr, "Must specify a domain.\n");
+        return 1;
+    }
+    if ((opt_p || opt_b) && (opt_p + opt_b != 2)) {

Maybe, "if (opt_p != opt_b)"?

+        fprintf(stderr, "Must specify period and budget\n");
+        return 1;
+    }
+
+    if (!dom) { /* list all domain's rt scheduler info */
+        return -sched_domain_output(LIBXL_SCHEDULER_RT_DS,
+                                    sched_rt_domain_output,
+                                    sched_rt_pool_output,
+                                    cpupool);
+    } else {
+        uint32_t domid = find_domain(dom);
+        if (!opt_p && !opt_b) { /* output rt scheduler info */
+            sched_rt_domain_output(-1);
+            return -sched_rt_domain_output(domid);
+        } else { /* set rt scheduler paramaters */
+            libxl_domain_sched_params scinfo;
+            libxl_domain_sched_params_init(&scinfo);
+            scinfo.sched = LIBXL_SCHEDULER_RT_DS;
+            scinfo.period = period;
+            scinfo.budget = budget;
+
+            rc = sched_domain_set(domid, &scinfo);
+            libxl_domain_sched_params_dispose(&scinfo);
+            if (rc)
+                return -rc;
+        }
+    }
+
+    return 0;
+}
+
  int main_domid(int argc, char **argv)
  {
      uint32_t domid;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 7b7fa92..0c0e06e 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -277,6 +277,14 @@ struct cmd_spec cmd_table[] = {
        "                               --period/--slice)\n"
        "-c CPUPOOL, --cpupool=CPUPOOL  Restrict output to CPUPOOL"
      },
+    { "sched-rt",

sched-rtds

Right, starting to get close. :-)

 -George

+      &main_sched_rt, 0, 1,
+      "Get/set rt scheduler parameters",
+      "[-d <Domain> [-p[=PERIOD]] [-b[=BUDGET]]]",
+      "-d DOMAIN, --domain=DOMAIN     Domain to modify\n"
+      "-p PERIOD, --period=PERIOD     Period (us)\n"
+      "-b BUDGET, --budget=BUDGET     Budget (us)\n"
+    },
      { "domid",
        &main_domid, 0, 0,
        "Convert a domain name to domain id",


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.