[PATCH v2 0/5] xen/tools: sched: Credit1: improve handling of vCPU migration delay

[PATCH v2 0/5] xen/tools: sched: Credit1: improve handling of vCPU migration delay

[Dario Faggioli]

Hi,

Take 2 of this series:

https://lists.xenproject.org/archives/html/xen-devel/2018-02/msg02029.html

The only changes are a couple of fixes for issues I found myself, in patch 3,
while working on a follow-up series, and bumping SYSCTL interface version in
patch 5, as suggested by Jan.

Updated branches:

 git://xenbits.xen.org/people/dariof/xen.git rel/sched/credit/vcpu_migr_delay_percpool-v2
 http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/sched/credit/vcpu_migr_delay_percpool-v2

Or:

 https://github.com/fdario/xen/tree/rel/sched/credit/vcpu_migr_delay_percpool-v2

Regards,
Dario
---
Dario Faggioli (5):
      xen: sched/credit: convert scheduling parameter to s_time_t when set
      xen: sched/credit1: make vcpu_migration_delay per-cpupool
      tools: libxl/xl: allow to get/set Credit1's vcpu_migration_delay
      tools: xenpm: continue to support {set,get}-vcpu-migration-delay
      xen/libxc: suppress direct access to Credit1's migration delay

 docs/man/xl.pod.1.in          |   11 ++++
 tools/libxc/include/xenctrl.h |    2 -
 tools/libxc/xc_pm.c           |   30 ------------
 tools/libxl/libxl.h           |    7 +++
 tools/libxl/libxl_sched.c     |   10 ++++
 tools/libxl/libxl_types.idl   |    1 
 tools/misc/xenpm.c            |   22 +++++++--
 tools/xl/xl_cmdtable.c        |    1 
 tools/xl/xl_sched.c           |   23 ++++++---
 xen/common/sched_credit.c     |  102 +++++++++++++++++++++--------------------
 xen/drivers/acpi/pmstat.c     |   12 -----
 xen/include/public/sysctl.h   |    7 ++-
 xen/include/xen/sched.h       |    3 -
 13 files changed, 121 insertions(+), 110 deletions(-)
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

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

[PATCH v2 1/5] xen: sched/credit: convert scheduling parameter to s_time_t when set

[Dario Faggioli]

Basically, instead of converting integers to s_time_t
at usage time (hot paths), do the convertion when the
values are set (cold paths).

This applies to the timeslice and the ratelimit
parameters of Credit1.

Note that, when changing the type of the fields of
struct csched_private (from unsigned to s_time_t),
ncpus is moved up a bit, for better packing.

Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/common/sched_credit.c |   56 ++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index f8212db5fe..a2c5d43e33 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -210,11 +210,11 @@ struct csched_private {
     cpumask_var_t cpus;
     uint32_t *balance_bias;
     uint32_t runq_sort;
-    unsigned int ratelimit_us;
+    uint32_t ncpus;
 
     /* Period of master and tick in milliseconds */
-    unsigned int tslice_ms, tick_period_us, ticks_per_tslice;
-    uint32_t ncpus;
+    unsigned int tick_period_us, ticks_per_tslice;
+    s_time_t ratelimit, tslice;
 
     struct list_head active_sdom;
     uint32_t weight;
@@ -570,8 +570,7 @@ init_pdata(struct csched_private *prv, struct csched_pcpu *spc, int cpu)
     {
         prv->master = cpu;
         init_timer(&prv->master_ticker, csched_acct, prv, cpu);
-        set_timer(&prv->master_ticker,
-                  NOW() + MILLISECS(prv->tslice_ms));
+        set_timer(&prv->master_ticker, NOW() + prv->tslice);
     }
 
     cpumask_and(cpumask_scratch, prv->cpus, &node_to_cpumask(cpu_to_node(cpu)));
@@ -1224,14 +1223,14 @@ csched_dom_cntl(
 }
 
 static inline void
-__csched_set_tslice(struct csched_private *prv, unsigned timeslice)
+__csched_set_tslice(struct csched_private *prv, unsigned int timeslice_ms)
 {
-    prv->tslice_ms = timeslice;
+    prv->tslice = MILLISECS(timeslice_ms);
     prv->ticks_per_tslice = CSCHED_TICKS_PER_TSLICE;
-    if ( prv->tslice_ms < prv->ticks_per_tslice )
+    if ( timeslice_ms < prv->ticks_per_tslice )
         prv->ticks_per_tslice = 1;
-    prv->tick_period_us = prv->tslice_ms * 1000 / prv->ticks_per_tslice;
-    prv->credits_per_tslice = CSCHED_CREDITS_PER_MSEC * prv->tslice_ms;
+    prv->tick_period_us = timeslice_ms * 1000 / prv->ticks_per_tslice;
+    prv->credits_per_tslice = CSCHED_CREDITS_PER_MSEC * timeslice_ms;
     prv->credit = prv->credits_per_tslice * prv->ncpus;
 }
 
@@ -1257,17 +1256,17 @@ csched_sys_cntl(const struct scheduler *ops,
 
         spin_lock_irqsave(&prv->lock, flags);
         __csched_set_tslice(prv, params->tslice_ms);
-        if ( !prv->ratelimit_us && params->ratelimit_us )
+        if ( !prv->ratelimit && params->ratelimit_us )
             printk(XENLOG_INFO "Enabling context switch rate limiting\n");
-        else if ( prv->ratelimit_us && !params->ratelimit_us )
+        else if ( prv->ratelimit && !params->ratelimit_us )
             printk(XENLOG_INFO "Disabling context switch rate limiting\n");
-        prv->ratelimit_us = params->ratelimit_us;
+        prv->ratelimit = MICROSECS(params->ratelimit_us);
         spin_unlock_irqrestore(&prv->lock, flags);
 
         /* FALLTHRU */
     case XEN_SYSCTL_SCHEDOP_getinfo:
-        params->tslice_ms = prv->tslice_ms;
-        params->ratelimit_us = prv->ratelimit_us;
+        params->tslice_ms = prv->tslice / MILLISECS(1);
+        params->ratelimit_us = prv->ratelimit / MICROSECS(1);
         rc = 0;
         break;
     }
@@ -1576,8 +1575,7 @@ csched_acct(void* dummy)
     prv->runq_sort++;
 
 out:
-    set_timer( &prv->master_ticker,
-               NOW() + MILLISECS(prv->tslice_ms));
+    set_timer( &prv->master_ticker, NOW() + prv->tslice);
 }
 
 static void
@@ -1901,21 +1899,21 @@ csched_schedule(
      */
     if ( !test_bit(CSCHED_FLAG_VCPU_YIELD, &scurr->flags)
          && !tasklet_work_scheduled
-         && prv->ratelimit_us
+         && prv->ratelimit
          && vcpu_runnable(current)
          && !is_idle_vcpu(current)
-         && runtime < MICROSECS(prv->ratelimit_us) )
+         && runtime < prv->ratelimit )
     {
         snext = scurr;
         snext->start_time += now;
         perfc_incr(delay_ms);
         /*
          * Next timeslice must last just until we'll have executed for
-         * ratelimit_us. However, to avoid setting a really short timer, which
+         * ratelimit. However, to avoid setting a really short timer, which
          * will most likely be inaccurate and counterproductive, we never go
          * below CSCHED_MIN_TIMER.
          */
-        tslice = MICROSECS(prv->ratelimit_us) - runtime;
+        tslice = prv->ratelimit - runtime;
         if ( unlikely(runtime < CSCHED_MIN_TIMER) )
             tslice = CSCHED_MIN_TIMER;
         if ( unlikely(tb_init_done) )
@@ -1934,7 +1932,7 @@ csched_schedule(
         ret.migrated = 0;
         goto out;
     }
-    tslice = MILLISECS(prv->tslice_ms);
+    tslice = prv->tslice;
 
     /*
      * Select next runnable local VCPU (ie top of local runq)
@@ -2112,8 +2110,8 @@ csched_dump(const struct scheduler *ops)
            "\tweight             = %u\n"
            "\trunq_sort          = %u\n"
            "\tdefault-weight     = %d\n"
-           "\ttslice             = %dms\n"
-           "\tratelimit          = %dus\n"
+           "\ttslice             = %"PRI_stime"ms\n"
+           "\tratelimit          = %"PRI_stime"us\n"
            "\tcredits per msec   = %d\n"
            "\tticks per tslice   = %d\n"
            "\tmigration delay    = %uus\n",
@@ -2124,8 +2122,8 @@ csched_dump(const struct scheduler *ops)
            prv->weight,
            prv->runq_sort,
            CSCHED_DEFAULT_WEIGHT,
-           prv->tslice_ms,
-           prv->ratelimit_us,
+           prv->tslice / MILLISECS(1),
+           prv->ratelimit / MICROSECS(1),
            CSCHED_CREDITS_PER_MSEC,
            prv->ticks_per_tslice,
            vcpu_migration_delay);
@@ -2206,11 +2204,11 @@ csched_init(struct scheduler *ops)
     {
         printk("WARNING: sched_ratelimit_us >" 
                "sched_credit_tslice_ms is undefined\n"
-               "Setting ratelimit_us to 1000 * tslice_ms\n");
-        prv->ratelimit_us = 1000 * prv->tslice_ms;
+               "Setting ratelimit to tslice\n");
+        prv->ratelimit = prv->tslice;
     }
     else
-        prv->ratelimit_us = sched_ratelimit_us;
+        prv->ratelimit = MICROSECS(sched_ratelimit_us);
     return 0;
 }
 


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

[PATCH v2 2/5] xen: sched/credit1: make vcpu_migration_delay per-cpupool

[Dario Faggioli]

Right now, vCPU migration delay is controlled by
the vcpu_migration_delay boot parameter. This means
the same value will always be used for every instance
of Credit1, in any cpupool that will be created.

Also, in order to get and set such value, a special
purpose libxc interface is defined, and used by the
xenpm tool. And this is problematic if Xen is built
without Credit1 support.

This commit adds a vcpu_migr_delay field inside
struct csched_private, so that we can get/set the
migration delay indepently for each Credit1 instance,
in different cpupools.

Getting and setting now happens via XEN_SYSCTL_SCHEDOP_*,
which is much better suited for this parameter.

The value of the boot time parameter is used for
initializing the vcpu_migr_delay field of the private
structure of all the scheduler instances, when they're
created.

While there, save reading NOW() and doing any s_time_t
operation, when the migration delay of a scheduler is
zero (as it is, by default), in
__csched_vcpu_is_cache_hot().

Finally, note that, from this commit on, using `xenpm
{set,get}-vcpu-migration-delay' is not effective any
longer.

Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/common/sched_credit.c   |   38 +++++++++++++++++++++++++++-----------
 xen/include/public/sysctl.h |    3 +++
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index a2c5d43e33..fd6dbd02aa 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -214,7 +214,7 @@ struct csched_private {
 
     /* Period of master and tick in milliseconds */
     unsigned int tick_period_us, ticks_per_tslice;
-    s_time_t ratelimit, tslice;
+    s_time_t ratelimit, tslice, vcpu_migr_delay;
 
     struct list_head active_sdom;
     uint32_t weight;
@@ -690,11 +690,11 @@ unsigned int get_vcpu_migration_delay(void)
     return vcpu_migration_delay;
 }
 
-static inline int
-__csched_vcpu_is_cache_hot(struct vcpu *v)
+static inline bool
+__csched_vcpu_is_cache_hot(const struct csched_private *prv, struct vcpu *v)
 {
-    int hot = ((NOW() - v->last_run_time) <
-               ((uint64_t)vcpu_migration_delay * 1000u));
+    bool hot = prv->vcpu_migr_delay &&
+               (NOW() - v->last_run_time) < prv->vcpu_migr_delay;
 
     if ( hot )
         SCHED_STAT_CRANK(vcpu_hot);
@@ -703,7 +703,8 @@ __csched_vcpu_is_cache_hot(struct vcpu *v)
 }
 
 static inline int
-__csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
+__csched_vcpu_is_migrateable(const struct csched_private *prv, struct vcpu *vc,
+                             int dest_cpu, cpumask_t *mask)
 {
     /*
      * Don't pick up work that's hot on peer PCPU, or that can't (or
@@ -714,7 +715,7 @@ __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
      */
     ASSERT(!vc->is_running);
 
-    return !__csched_vcpu_is_cache_hot(vc) &&
+    return !__csched_vcpu_is_cache_hot(prv, vc) &&
            cpumask_test_cpu(dest_cpu, mask);
 }
 
@@ -1251,7 +1252,9 @@ csched_sys_cntl(const struct scheduler *ops,
              || (params->ratelimit_us
                  && (params->ratelimit_us > XEN_SYSCTL_SCHED_RATELIMIT_MAX
                      || params->ratelimit_us < XEN_SYSCTL_SCHED_RATELIMIT_MIN))
-             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms) )
+             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms)
+             || (params->vcpu_migr_delay_us
+                 && MICROSECS(params->vcpu_migr_delay_us) >= XEN_SYSCTL_CSCHED_MGR_DLY_MAX) )
                 goto out;
 
         spin_lock_irqsave(&prv->lock, flags);
@@ -1261,12 +1264,14 @@ csched_sys_cntl(const struct scheduler *ops,
         else if ( prv->ratelimit && !params->ratelimit_us )
             printk(XENLOG_INFO "Disabling context switch rate limiting\n");
         prv->ratelimit = MICROSECS(params->ratelimit_us);
+        prv->vcpu_migr_delay = MICROSECS(params->vcpu_migr_delay_us);
         spin_unlock_irqrestore(&prv->lock, flags);
 
         /* FALLTHRU */
     case XEN_SYSCTL_SCHEDOP_getinfo:
         params->tslice_ms = prv->tslice / MILLISECS(1);
         params->ratelimit_us = prv->ratelimit / MICROSECS(1);
+        params->vcpu_migr_delay_us = prv->vcpu_migr_delay / MICROSECS(1);
         rc = 0;
         break;
     }
@@ -1608,6 +1613,7 @@ csched_tick(void *_cpu)
 static struct csched_vcpu *
 csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
 {
+    const struct csched_private * const prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
     const struct csched_pcpu * const peer_pcpu = CSCHED_PCPU(peer_cpu);
     struct csched_vcpu *speer;
     struct list_head *iter;
@@ -1657,7 +1663,7 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
             continue;
 
         affinity_balance_cpumask(vc, balance_step, cpumask_scratch);
-        if ( __csched_vcpu_is_migrateable(vc, cpu, cpumask_scratch) )
+        if ( __csched_vcpu_is_migrateable(prv, vc, cpu, cpumask_scratch) )
         {
             /* We got a candidate. Grab it! */
             TRACE_3D(TRC_CSCHED_STOLEN_VCPU, peer_cpu,
@@ -2114,7 +2120,7 @@ csched_dump(const struct scheduler *ops)
            "\tratelimit          = %"PRI_stime"us\n"
            "\tcredits per msec   = %d\n"
            "\tticks per tslice   = %d\n"
-           "\tmigration delay    = %uus\n",
+           "\tmigration delay    = %"PRI_stime"us\n",
            prv->ncpus,
            prv->master,
            prv->credit,
@@ -2126,7 +2132,7 @@ csched_dump(const struct scheduler *ops)
            prv->ratelimit / MICROSECS(1),
            CSCHED_CREDITS_PER_MSEC,
            prv->ticks_per_tslice,
-           vcpu_migration_delay);
+           prv->vcpu_migr_delay/ MICROSECS(1));
 
     cpumask_scnprintf(idlers_buf, sizeof(idlers_buf), prv->idlers);
     printk("idlers: %s\n", idlers_buf);
@@ -2209,6 +2215,16 @@ csched_init(struct scheduler *ops)
     }
     else
         prv->ratelimit = MICROSECS(sched_ratelimit_us);
+
+    if ( !vcpu_migration_delay && vcpu_migration_delay > MILLISECS(100) )
+    {
+        vcpu_migration_delay = 0;
+        printk("WARNING: vcpu_migration_delay outside of valid range [%d,%d]us.\n"
+               "Resetting to default: %u\n",
+               0, XEN_SYSCTL_CSCHED_MGR_DLY_MAX, vcpu_migration_delay);
+    }
+    prv->vcpu_migr_delay = MICROSECS(vcpu_migration_delay);
+
     return 0;
 }
 
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 3669e32524..ab62ffafa9 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -601,6 +601,9 @@ struct xen_sysctl_credit_schedule {
 #define XEN_SYSCTL_CSCHED_TSLICE_MIN 1
     unsigned tslice_ms;
     unsigned ratelimit_us;
+    /* How long to consider a vCPU cache-hot on the CPU where it is running */
+#define XEN_SYSCTL_CSCHED_MGR_DLY_MAX (100 * 1000) /* 100us, in millisecs */
+    unsigned vcpu_migr_delay_us;
 };
 
 struct xen_sysctl_credit2_schedule {


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

[PATCH v2 3/5] tools: libxl/xl: allow to get/set Credit1's vcpu_migration_delay

[Dario Faggioli]

Make it possible to get and set a (Credit1) scheduler's
vCPU migration delay via the SCHEDOP sysctl, from both
libxl and xl (no change needed in libxc).

Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes from v1:
* add missing 'break', fix using wrong variable in xl_sched.c.

Changes are small, but still I'm dropping Wei's Ack.
---
 docs/man/xl.pod.1.in        |   11 +++++++++++
 tools/libxl/libxl.h         |    7 +++++++
 tools/libxl/libxl_sched.c   |   10 ++++++++++
 tools/libxl/libxl_types.idl |    1 +
 tools/xl/xl_cmdtable.c      |    1 +
 tools/xl/xl_sched.c         |   23 ++++++++++++++++-------
 6 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 7fd35c9ae7..48da2052cc 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -1046,6 +1046,17 @@ we will allow a higher-priority VM to pre-empt it.  The default value
 is 1000 microseconds (1ms).  Valid range is 100 to 500000 (500ms).
 The ratelimit length must be lower than the timeslice length.
 
+=item B<-m DELAY>, B<--migration_delay_us=DELAY>
+
+Migration delay specifies for how long a vCPU, after it stopped running should
+be considered "cache-hot". Basically, if less than DELAY us passed since when
+the vCPU was executing on a CPU, it is likely that most of the vCPU's working
+set is still in the CPU's cache, and therefore the vCPU is not migrated.
+
+Default is 0. Maximum is 100 ms. This can be effective at preventing vCPUs
+to bounce among CPUs too quickly, but, at the same time, the scheduler stops
+being fully work-conserving.
+
 =back
 
 B<COMBINATION>
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index eca0ea2c50..edd244278a 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -299,6 +299,13 @@
  */
 #define LIBXL_HAVE_SCHED_CREDIT2_PARAMS 1
 
+/*
+ * LIBXL_HAVE_SCHED_CREDIT_MIGR_DELAY indicates that there is a field
+ * in libxl_sched_credit_params called vcpu_migr_delay_us which controls
+ * the resistance of the vCPUs of the cpupool to migrations among pCPUs.
+ */
+#define LIBXL_HAVE_SCHED_CREDIT_MIGR_DELAY
+
 /*
  * LIBXL_HAVE_VIRIDIAN_CRASH_CTL indicates that the 'crash_ctl' value
  * is present in the viridian enlightenment enumeration.
diff --git a/tools/libxl/libxl_sched.c b/tools/libxl/libxl_sched.c
index 512788f736..07289079ce 100644
--- a/tools/libxl/libxl_sched.c
+++ b/tools/libxl/libxl_sched.c
@@ -291,6 +291,7 @@ int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid,
 
     scinfo->tslice_ms = sparam.tslice_ms;
     scinfo->ratelimit_us = sparam.ratelimit_us;
+    scinfo->vcpu_migr_delay_us = sparam.vcpu_migr_delay_us;
 
     rc = 0;
  out:
@@ -321,9 +322,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
         rc = ERROR_INVAL;
         goto out;
     }
+    if (scinfo->vcpu_migr_delay_us
+        && scinfo->vcpu_migr_delay_us > XEN_SYSCTL_CSCHED_MGR_DLY_MAX) {
+        LOG(ERROR, "vcpu migration delay should be > 0 and < %d",
+            XEN_SYSCTL_CSCHED_MGR_DLY_MAX);
+        rc = ERROR_INVAL;
+        goto out;
+    }
 
     sparam.tslice_ms = scinfo->tslice_ms;
     sparam.ratelimit_us = scinfo->ratelimit_us;
+    sparam.vcpu_migr_delay_us = scinfo->vcpu_migr_delay_us;
 
     r = xc_sched_credit_params_set(ctx->xch, poolid, &sparam);
     if ( r < 0 ) {
@@ -334,6 +343,7 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
 
     scinfo->tslice_ms = sparam.tslice_ms;
     scinfo->ratelimit_us = sparam.ratelimit_us;
+    scinfo->vcpu_migr_delay_us = sparam.vcpu_migr_delay_us;
 
     rc = 0;
  out:
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 35038120ca..dbb287d6fe 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -973,6 +973,7 @@ libxl_pcitopology = Struct("pcitopology", [
 libxl_sched_credit_params = Struct("sched_credit_params", [
     ("tslice_ms", integer),
     ("ratelimit_us", integer),
+    ("vcpu_migr_delay_us", integer),
     ], dispose_fn=None)
 
 libxl_sched_credit2_params = Struct("sched_credit2_params", [
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 6d894394ca..bf2ced8140 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -257,6 +257,7 @@ struct cmd_spec cmd_table[] = {
       "-s         --schedparam           Query / modify scheduler parameters\n"
       "-t TSLICE, --tslice_ms=TSLICE     Set the timeslice, in milliseconds\n"
       "-r RLIMIT, --ratelimit_us=RLIMIT  Set the scheduling rate limit, in microseconds\n"
+      "-m DLY, --migration_delay_us=DLY  Set the migration delay, in microseconds\n"
       "-p CPUPOOL, --cpupool=CPUPOOL     Restrict output to CPUPOOL"
     },
     { "sched-credit2",
diff --git a/tools/xl/xl_sched.c b/tools/xl/xl_sched.c
index 7965ccbca0..73cd7040cd 100644
--- a/tools/xl/xl_sched.c
+++ b/tools/xl/xl_sched.c
@@ -172,10 +172,11 @@ static int sched_credit_pool_output(uint32_t poolid)
         printf("Cpupool %s: [sched params unavailable]\n",
                poolname);
     } else {
-        printf("Cpupool %s: tslice=%dms ratelimit=%dus\n",
+        printf("Cpupool %s: tslice=%dms ratelimit=%dus migration-delay=%dus\n",
                poolname,
                scparam.tslice_ms,
-               scparam.ratelimit_us);
+               scparam.ratelimit_us,
+               scparam.vcpu_migr_delay_us);
     }
     free(poolname);
     return 0;
@@ -469,10 +470,10 @@ int main_sched_credit(int argc, char **argv)
     const char *dom = NULL;
     const char *cpupool = NULL;
     int weight = 256, cap = 0;
-    int tslice = 0, ratelimit = 0;
+    int tslice = 0, ratelimit = 0, migrdelay = 0;
     bool opt_w = false, opt_c = false;
     bool opt_t = false, opt_r = false;
-    bool opt_s = false;
+    bool opt_s = false, opt_m = false;
     int opt, rc;
     static struct option opts[] = {
         {"domain", 1, 0, 'd'},
@@ -481,11 +482,12 @@ int main_sched_credit(int argc, char **argv)
         {"schedparam", 0, 0, 's'},
         {"tslice_ms", 1, 0, 't'},
         {"ratelimit_us", 1, 0, 'r'},
+        {"migration_delay_us", 1, 0, 'm'},
         {"cpupool", 1, 0, 'p'},
         COMMON_LONG_OPTS
     };
 
-    SWITCH_FOREACH_OPT(opt, "d:w:c:p:t:r:s", opts, "sched-credit", 0) {
+    SWITCH_FOREACH_OPT(opt, "d:w:c:p:t:r:m:s", opts, "sched-credit", 0) {
     case 'd':
         dom = optarg;
         break;
@@ -505,6 +507,10 @@ int main_sched_credit(int argc, char **argv)
         ratelimit = strtol(optarg, NULL, 10);
         opt_r = true;
         break;
+    case 'm':
+        migrdelay = strtol(optarg, NULL, 10);
+        opt_m = true;
+        break;
     case 's':
         opt_s = true;
         break;
@@ -522,7 +528,7 @@ int main_sched_credit(int argc, char **argv)
         fprintf(stderr, "Must specify a domain.\n");
         return EXIT_FAILURE;
     }
-    if (!opt_s && (opt_t || opt_r)) {
+    if (!opt_s && (opt_t || opt_r || opt_m)) {
         fprintf(stderr, "Must specify schedparam to set schedule "
                 "parameter values.\n");
         return EXIT_FAILURE;
@@ -541,7 +547,7 @@ int main_sched_credit(int argc, char **argv)
             }
         }
 
-        if (!opt_t && !opt_r) { /* Output scheduling parameters */
+        if (!opt_t && !opt_r && !opt_m) { /* Output scheduling parameters */
             if (sched_credit_pool_output(poolid))
                 return EXIT_FAILURE;
         } else { /* Set scheduling parameters*/
@@ -554,6 +560,9 @@ int main_sched_credit(int argc, char **argv)
             if (opt_r)
                 scparam.ratelimit_us = ratelimit;
 
+            if (opt_m)
+                scparam.vcpu_migr_delay_us = migrdelay;
+
             if (sched_credit_params_set(poolid, &scparam))
                 return EXIT_FAILURE;
         }


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

[PATCH v2 4/5] tools: xenpm: continue to support {set, get}-vcpu-migration-delay

[Dario Faggioli]

Now that it is possible to get and set the migration
delay via the SCHEDOP sysctl, use that in xenpm, instead
of the special purpose libxc interface (which will be
removed in a following commit).

The sysctl, however, requires a cpupool-id argument,
for knowing on which scheduler it is operating on. In
this case, since we don't want to alter xenpm's command
line interface, we always use '0', which means xenpm
will always act on the default cpupool ('Pool-0').

>From this commit on, `xenpm {set,get}-vcpu-migration-delay'
commands work again. But that is only for the sake of
backward compatibility, and their use is deprecated, in
favour of 'xl sched-credit -s [-c <poolid>] -m <delay>'.

Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/misc/xenpm.c |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index 762311e5a5..f34b7987af 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -1071,14 +1071,24 @@ void set_sched_smt_func(int argc, char *argv[])
 
 void set_vcpu_migration_delay_func(int argc, char *argv[])
 {
+    struct xen_sysctl_credit_schedule sparam;
     int value;
 
+    printf("WARNING: using xenpm for this purpose is deprecated."
+           " Check out `xl sched-credit -s -m DELAY'\n");
+
     if ( argc != 1 || (value = atoi(argv[0])) < 0 ) {
         fprintf(stderr, "Missing or invalid argument(s)\n");
         exit(EINVAL);
     }
 
-    if ( !xc_set_vcpu_migration_delay(xc_handle, value) )
+    if ( xc_sched_credit_params_get(xc_handle, 0, &sparam) < 0 ) {
+        fprintf(stderr, "getting Credit scheduler parameters failed\n");
+        exit(EINVAL);
+    }
+    sparam.vcpu_migr_delay_us = value;
+
+    if ( !xc_sched_credit_params_set(xc_handle, 0, &sparam) )
         printf("set vcpu migration delay to %d us succeeded\n", value);
     else
         fprintf(stderr, "set vcpu migration delay failed (%d - %s)\n",
@@ -1087,13 +1097,17 @@ void set_vcpu_migration_delay_func(int argc, char *argv[])
 
 void get_vcpu_migration_delay_func(int argc, char *argv[])
 {
-    uint32_t value;
+    struct xen_sysctl_credit_schedule sparam;
+
+    printf("WARNING: using xenpm for this purpose is deprecated."
+           " Check out `xl sched-credit -s'\n");
 
     if ( argc )
         fprintf(stderr, "Ignoring argument(s)\n");
 
-    if ( !xc_get_vcpu_migration_delay(xc_handle, &value) )
-        printf("Scheduler vcpu migration delay is %d us\n", value);
+    if ( !xc_sched_credit_params_get(xc_handle, 0, &sparam) )
+        printf("Scheduler vcpu migration delay is %d us\n",
+               sparam.vcpu_migr_delay_us);
     else
         fprintf(stderr,
                 "Failed to get scheduler vcpu migration delay (%d - %s)\n",


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

[PATCH v2 5/5] xen/libxc: suppress direct access to Credit1's migration delay

[Dario Faggioli]

Removes special purpose access to Credit1 vCPU
migration delay parameter.

This fixes a build breakage, occuring when Xen
is configured with SCHED_CREDIT=n.

Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
Changes from v1:
* bumped the interface version, as requested.
---
 tools/libxc/include/xenctrl.h |    2 --
 tools/libxc/xc_pm.c           |   30 ------------------------------
 xen/common/sched_credit.c     |   10 ----------
 xen/drivers/acpi/pmstat.c     |   12 ------------
 xen/include/public/sysctl.h   |    4 +---
 xen/include/xen/sched.h       |    3 ---
 6 files changed, 1 insertion(+), 60 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 543abfcb34..058e832c47 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1920,8 +1920,6 @@ int xc_set_cpufreq_para(xc_interface *xch, int cpuid,
 int xc_get_cpufreq_avgfreq(xc_interface *xch, int cpuid, int *avg_freq);
 
 int xc_set_sched_opt_smt(xc_interface *xch, uint32_t value);
-int xc_set_vcpu_migration_delay(xc_interface *xch, uint32_t value);
-int xc_get_vcpu_migration_delay(xc_interface *xch, uint32_t *value);
 
 int xc_get_cpuidle_max_cstate(xc_interface *xch, uint32_t *value);
 int xc_set_cpuidle_max_cstate(xc_interface *xch, uint32_t value);
diff --git a/tools/libxc/xc_pm.c b/tools/libxc/xc_pm.c
index ae917bc630..67e2418e3f 100644
--- a/tools/libxc/xc_pm.c
+++ b/tools/libxc/xc_pm.c
@@ -367,36 +367,6 @@ int xc_set_sched_opt_smt(xc_interface *xch, uint32_t value)
    return rc;
 }
 
-int xc_set_vcpu_migration_delay(xc_interface *xch, uint32_t value)
-{
-   int rc;
-   DECLARE_SYSCTL;
-
-   sysctl.cmd = XEN_SYSCTL_pm_op;
-   sysctl.u.pm_op.cmd = XEN_SYSCTL_pm_op_set_vcpu_migration_delay;
-   sysctl.u.pm_op.cpuid = 0;
-   sysctl.u.pm_op.u.set_vcpu_migration_delay = value;
-   rc = do_sysctl(xch, &sysctl);
-
-   return rc;
-}
-
-int xc_get_vcpu_migration_delay(xc_interface *xch, uint32_t *value)
-{
-   int rc;
-   DECLARE_SYSCTL;
-
-   sysctl.cmd = XEN_SYSCTL_pm_op;
-   sysctl.u.pm_op.cmd = XEN_SYSCTL_pm_op_get_vcpu_migration_delay;
-   sysctl.u.pm_op.cpuid = 0;
-   rc = do_sysctl(xch, &sysctl);
-
-   if (!rc && value)
-       *value = sysctl.u.pm_op.u.get_vcpu_migration_delay;
-
-   return rc;
-}
-
 int xc_get_cpuidle_max_cstate(xc_interface *xch, uint32_t *value)
 {
     int rc;
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index fd6dbd02aa..c46495ae95 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -680,16 +680,6 @@ __csched_vcpu_check(struct vcpu *vc)
 static unsigned int vcpu_migration_delay;
 integer_param("vcpu_migration_delay", vcpu_migration_delay);
 
-void set_vcpu_migration_delay(unsigned int delay)
-{
-    vcpu_migration_delay = delay;
-}
-
-unsigned int get_vcpu_migration_delay(void)
-{
-    return vcpu_migration_delay;
-}
-
 static inline bool
 __csched_vcpu_is_cache_hot(const struct csched_private *prv, struct vcpu *v)
 {
diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index 2a6c4c7444..a8fc52a35f 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -449,18 +449,6 @@ int do_pm_op(struct xen_sysctl_pm_op *op)
         break;
     }
 
-    case XEN_SYSCTL_pm_op_set_vcpu_migration_delay:
-    {
-        set_vcpu_migration_delay(op->u.set_vcpu_migration_delay);
-        break;
-    }
-
-    case XEN_SYSCTL_pm_op_get_vcpu_migration_delay:
-    {
-        op->u.get_vcpu_migration_delay = get_vcpu_migration_delay();
-        break;
-    }
-
     case XEN_SYSCTL_pm_op_get_max_cstate:
     {
         op->u.get_max_cstate = acpi_get_cstate_limit();
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index ab62ffafa9..237cfe7f1f 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
 #include "physdev.h"
 #include "tmem.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000010
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000011
 
 /*
  * Read console content from Xen buffer ring.
@@ -351,8 +351,6 @@ struct xen_sysctl_pm_op {
         uint32_t                    set_sched_opt_smt;
         uint32_t                    get_max_cstate;
         uint32_t                    set_max_cstate;
-        uint32_t                    get_vcpu_migration_delay;
-        uint32_t                    set_vcpu_migration_delay;
     } u;
 };
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 39f938644a..a82ee08dd6 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -903,9 +903,6 @@ static inline bool is_vcpu_online(const struct vcpu *v)
     return !test_bit(_VPF_down, &v->pause_flags);
 }
 
-void set_vcpu_migration_delay(unsigned int delay);
-unsigned int get_vcpu_migration_delay(void);
-
 extern bool sched_smt_power_savings;
 
 extern enum cpufreq_controller {


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

Re: [PATCH v2 3/5] tools: libxl/xl: allow to get/set Credit1's vcpu_migration_delay

[Wei Liu]

On Fri, Feb 23, 2018 at 05:41:48PM +0100, Dario Faggioli wrote:
> Make it possible to get and set a (Credit1) scheduler's
> vCPU migration delay via the SCHEDOP sysctl, from both
> libxl and xl (no change needed in libxc).
> 
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v2 1/5] xen: sched/credit: convert scheduling parameter to s_time_t when set

[George Dunlap]

On 02/23/2018 04:41 PM, Dario Faggioli wrote:
> Basically, instead of converting integers to s_time_t
> at usage time (hot paths), do the convertion when the
> values are set (cold paths).
> 
> This applies to the timeslice and the ratelimit
> parameters of Credit1.
> 
> Note that, when changing the type of the fields of
> struct csched_private (from unsigned to s_time_t),
> ncpus is moved up a bit, for better packing.
> 
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>

Acked-by: George Dunlap <george.dunlap@citrix.com>

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

Re: [PATCH v2 2/5] xen: sched/credit1: make vcpu_migration_delay per-cpupool

[George Dunlap]

On 02/23/2018 04:41 PM, Dario Faggioli wrote:
> Right now, vCPU migration delay is controlled by
> the vcpu_migration_delay boot parameter. This means
> the same value will always be used for every instance
> of Credit1, in any cpupool that will be created.
> 
> Also, in order to get and set such value, a special
> purpose libxc interface is defined, and used by the
> xenpm tool. And this is problematic if Xen is built
> without Credit1 support.
> 
> This commit adds a vcpu_migr_delay field inside
> struct csched_private, so that we can get/set the
> migration delay indepently for each Credit1 instance,
> in different cpupools.
> 
> Getting and setting now happens via XEN_SYSCTL_SCHEDOP_*,
> which is much better suited for this parameter.
> 
> The value of the boot time parameter is used for
> initializing the vcpu_migr_delay field of the private
> structure of all the scheduler instances, when they're
> created.
> 
> While there, save reading NOW() and doing any s_time_t
> operation, when the migration delay of a scheduler is
> zero (as it is, by default), in
> __csched_vcpu_is_cache_hot().
> 
> Finally, note that, from this commit on, using `xenpm
> {set,get}-vcpu-migration-delay' is not effective any
> longer.

I think "will have no effect" would probably be more clear; it's also
probably worth mentioning that you're planning on providing a
backwards-compatibility option in a subsequent patch.

[snip]

> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 3669e32524..ab62ffafa9 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -601,6 +601,9 @@ struct xen_sysctl_credit_schedule {
>  #define XEN_SYSCTL_CSCHED_TSLICE_MIN 1
>      unsigned tslice_ms;
>      unsigned ratelimit_us;
> +    /* How long to consider a vCPU cache-hot on the CPU where it is
running */
> +#define XEN_SYSCTL_CSCHED_MGR_DLY_MAX (100 * 1000) /* 100us, in
millisecs */

Is this supposed to be "100ms in microseconds"?  Unless I'm confused
there are some serious unit conversion mix-ups in this patch; I think it
would be better to add the '_US' suffix to this to help clarify things.

[snip]

> @@ -1251,7 +1252,9 @@ csched_sys_cntl(const struct scheduler *ops,
>               || (params->ratelimit_us
>                   && (params->ratelimit_us > XEN_SYSCTL_SCHED_RATELIMIT_MAX
>                       || params->ratelimit_us < XEN_SYSCTL_SCHED_RATELIMIT_MIN))
> -             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms) )
> +             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms)
> +             || (params->vcpu_migr_delay_us
> +                 && MICROSECS(params->vcpu_migr_delay_us) >= XEN_SYSCTL_CSCHED_MGR_DLY_MAX) 

Is there really a point to checking to see if it's zero, just to make
sure that it's not greater than XEN_SYSCTL_CSCHED_MGR_DLY_MAX?  If it's
zero it certainly won't be greater than that value, assuming that value
isn't negative.

Also, if both vcpu_migr_delay_us and XEN_SYSCTL_CSCHED_MGR_DLY_MAX are
in microseconds, why are you doing any conversion at all (much less
converting only one of them)?

> +    if ( !vcpu_migration_delay && vcpu_migration_delay > MILLISECS(100) )

Why is this hardcoded, rather than using MGR_DLY_MAX?  Also...

> +    {
> +        vcpu_migration_delay = 0;
> +        printk("WARNING: vcpu_migration_delay outside of valid range [%d,%d]us.\n"
> +               "Resetting to default: %u\n",
> +               0, XEN_SYSCTL_CSCHED_MGR_DLY_MAX, vcpu_migration_delay);
> +    }
> +    prv->vcpu_migr_delay = MICROSECS(vcpu_migration_delay);

Something here can't be right -- what unit is vcpu_migration_delay in?
If it's in microseconds, why are you comparing it to MILLISECS(100)
above (which would be in nanoseconds)?  If it's already in nanoseconds,
why are you converting it from us to ns here?

 -George

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

Re: [PATCH v2 2/5] xen: sched/credit1: make vcpu_migration_delay per-cpupool

[George Dunlap]

On 02/23/2018 04:41 PM, Dario Faggioli wrote:
> Right now, vCPU migration delay is controlled by
> the vcpu_migration_delay boot parameter. This means
> the same value will always be used for every instance
> of Credit1, in any cpupool that will be created.
> 
> Also, in order to get and set such value, a special
> purpose libxc interface is defined, and used by the
> xenpm tool. And this is problematic if Xen is built
> without Credit1 support.
> 
> This commit adds a vcpu_migr_delay field inside
> struct csched_private, so that we can get/set the
> migration delay indepently for each Credit1 instance,
> in different cpupools.
> 
> Getting and setting now happens via XEN_SYSCTL_SCHEDOP_*,
> which is much better suited for this parameter.
> 
> The value of the boot time parameter is used for
> initializing the vcpu_migr_delay field of the private
> structure of all the scheduler instances, when they're
> created.
> 
> While there, save reading NOW() and doing any s_time_t
> operation, when the migration delay of a scheduler is
> zero (as it is, by default), in
> __csched_vcpu_is_cache_hot().
> 
> Finally, note that, from this commit on, using `xenpm
> {set,get}-vcpu-migration-delay' is not effective any
> longer.
> 
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
> ---
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/common/sched_credit.c   |   38 +++++++++++++++++++++++++++-----------
>  xen/include/public/sysctl.h |    3 +++
>  2 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index a2c5d43e33..fd6dbd02aa 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -214,7 +214,7 @@ struct csched_private {
>  
>      /* Period of master and tick in milliseconds */
>      unsigned int tick_period_us, ticks_per_tslice;
> -    s_time_t ratelimit, tslice;
> +    s_time_t ratelimit, tslice, vcpu_migr_delay;
>  
>      struct list_head active_sdom;
>      uint32_t weight;
> @@ -690,11 +690,11 @@ unsigned int get_vcpu_migration_delay(void)
>      return vcpu_migration_delay;
>  }
>  
> -static inline int
> -__csched_vcpu_is_cache_hot(struct vcpu *v)
> +static inline bool
> +__csched_vcpu_is_cache_hot(const struct csched_private *prv, struct vcpu *v)
>  {
> -    int hot = ((NOW() - v->last_run_time) <
> -               ((uint64_t)vcpu_migration_delay * 1000u));
> +    bool hot = prv->vcpu_migr_delay &&
> +               (NOW() - v->last_run_time) < prv->vcpu_migr_delay;
>  
>      if ( hot )
>          SCHED_STAT_CRANK(vcpu_hot);
> @@ -703,7 +703,8 @@ __csched_vcpu_is_cache_hot(struct vcpu *v)
>  }
>  
>  static inline int
> -__csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
> +__csched_vcpu_is_migrateable(const struct csched_private *prv, struct vcpu *vc,
> +                             int dest_cpu, cpumask_t *mask)
>  {
>      /*
>       * Don't pick up work that's hot on peer PCPU, or that can't (or
> @@ -714,7 +715,7 @@ __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
>       */
>      ASSERT(!vc->is_running);
>  
> -    return !__csched_vcpu_is_cache_hot(vc) &&
> +    return !__csched_vcpu_is_cache_hot(prv, vc) &&
>             cpumask_test_cpu(dest_cpu, mask);
>  }
>  
> @@ -1251,7 +1252,9 @@ csched_sys_cntl(const struct scheduler *ops,
>               || (params->ratelimit_us
>                   && (params->ratelimit_us > XEN_SYSCTL_SCHED_RATELIMIT_MAX
>                       || params->ratelimit_us < XEN_SYSCTL_SCHED_RATELIMIT_MIN))
> -             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms) )
> +             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms)
> +             || (params->vcpu_migr_delay_us
> +                 && MICROSECS(params->vcpu_migr_delay_us) >= XEN_SYSCTL_CSCHED_MGR_DLY_MAX) )
>                  goto out;
>  
>          spin_lock_irqsave(&prv->lock, flags);
> @@ -1261,12 +1264,14 @@ csched_sys_cntl(const struct scheduler *ops,
>          else if ( prv->ratelimit && !params->ratelimit_us )
>              printk(XENLOG_INFO "Disabling context switch rate limiting\n");
>          prv->ratelimit = MICROSECS(params->ratelimit_us);
> +        prv->vcpu_migr_delay = MICROSECS(params->vcpu_migr_delay_us);
>          spin_unlock_irqrestore(&prv->lock, flags);
>  
>          /* FALLTHRU */
>      case XEN_SYSCTL_SCHEDOP_getinfo:
>          params->tslice_ms = prv->tslice / MILLISECS(1);
>          params->ratelimit_us = prv->ratelimit / MICROSECS(1);
> +        params->vcpu_migr_delay_us = prv->vcpu_migr_delay / MICROSECS(1);
>          rc = 0;
>          break;
>      }
> @@ -1608,6 +1613,7 @@ csched_tick(void *_cpu)
>  static struct csched_vcpu *
>  csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
>  {
> +    const struct csched_private * const prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
>      const struct csched_pcpu * const peer_pcpu = CSCHED_PCPU(peer_cpu);
>      struct csched_vcpu *speer;
>      struct list_head *iter;
> @@ -1657,7 +1663,7 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
>              continue;
>  
>          affinity_balance_cpumask(vc, balance_step, cpumask_scratch);
> -        if ( __csched_vcpu_is_migrateable(vc, cpu, cpumask_scratch) )
> +        if ( __csched_vcpu_is_migrateable(prv, vc, cpu, cpumask_scratch) )
>          {
>              /* We got a candidate. Grab it! */
>              TRACE_3D(TRC_CSCHED_STOLEN_VCPU, peer_cpu,
> @@ -2114,7 +2120,7 @@ csched_dump(const struct scheduler *ops)
>             "\tratelimit          = %"PRI_stime"us\n"
>             "\tcredits per msec   = %d\n"
>             "\tticks per tslice   = %d\n"
> -           "\tmigration delay    = %uus\n",
> +           "\tmigration delay    = %"PRI_stime"us\n",
>             prv->ncpus,
>             prv->master,
>             prv->credit,
> @@ -2126,7 +2132,7 @@ csched_dump(const struct scheduler *ops)
>             prv->ratelimit / MICROSECS(1),
>             CSCHED_CREDITS_PER_MSEC,
>             prv->ticks_per_tslice,
> -           vcpu_migration_delay);
> +           prv->vcpu_migr_delay/ MICROSECS(1));
>  
>      cpumask_scnprintf(idlers_buf, sizeof(idlers_buf), prv->idlers);
>      printk("idlers: %s\n", idlers_buf);
> @@ -2209,6 +2215,16 @@ csched_init(struct scheduler *ops)
>      }
>      else
>          prv->ratelimit = MICROSECS(sched_ratelimit_us);
> +
> +    if ( !vcpu_migration_delay && vcpu_migration_delay > MILLISECS(100) )

Also, this expression can never evaluate to true; the first element
requires 0, the second requires non-zero.  I think you want to get rid
of the `!`.

 -George

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

Re: [PATCH v2 2/5] xen: sched/credit1: make vcpu_migration_delay per-cpupool

[George Dunlap]

On 02/28/2018 02:23 PM, George Dunlap wrote:
> On 02/23/2018 04:41 PM, Dario Faggioli wrote:
>> Right now, vCPU migration delay is controlled by
>> the vcpu_migration_delay boot parameter. This means
>> the same value will always be used for every instance
>> of Credit1, in any cpupool that will be created.
>>
>> Also, in order to get and set such value, a special
>> purpose libxc interface is defined, and used by the
>> xenpm tool. And this is problematic if Xen is built
>> without Credit1 support.
>>
>> This commit adds a vcpu_migr_delay field inside
>> struct csched_private, so that we can get/set the
>> migration delay indepently for each Credit1 instance,
>> in different cpupools.
>>
>> Getting and setting now happens via XEN_SYSCTL_SCHEDOP_*,
>> which is much better suited for this parameter.
>>
>> The value of the boot time parameter is used for
>> initializing the vcpu_migr_delay field of the private
>> structure of all the scheduler instances, when they're
>> created.
>>
>> While there, save reading NOW() and doing any s_time_t
>> operation, when the migration delay of a scheduler is
>> zero (as it is, by default), in
>> __csched_vcpu_is_cache_hot().
>>
>> Finally, note that, from this commit on, using `xenpm
>> {set,get}-vcpu-migration-delay' is not effective any
>> longer.
>>
>> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
>> ---
>> Cc: George Dunlap <george.dunlap@eu.citrix.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>>  xen/common/sched_credit.c   |   38 +++++++++++++++++++++++++++-----------
>>  xen/include/public/sysctl.h |    3 +++
>>  2 files changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
>> index a2c5d43e33..fd6dbd02aa 100644
>> --- a/xen/common/sched_credit.c
>> +++ b/xen/common/sched_credit.c
>> @@ -214,7 +214,7 @@ struct csched_private {
>>  
>>      /* Period of master and tick in milliseconds */
>>      unsigned int tick_period_us, ticks_per_tslice;
>> -    s_time_t ratelimit, tslice;
>> +    s_time_t ratelimit, tslice, vcpu_migr_delay;
>>  
>>      struct list_head active_sdom;
>>      uint32_t weight;
>> @@ -690,11 +690,11 @@ unsigned int get_vcpu_migration_delay(void)
>>      return vcpu_migration_delay;
>>  }
>>  
>> -static inline int
>> -__csched_vcpu_is_cache_hot(struct vcpu *v)
>> +static inline bool
>> +__csched_vcpu_is_cache_hot(const struct csched_private *prv, struct vcpu *v)
>>  {
>> -    int hot = ((NOW() - v->last_run_time) <
>> -               ((uint64_t)vcpu_migration_delay * 1000u));
>> +    bool hot = prv->vcpu_migr_delay &&
>> +               (NOW() - v->last_run_time) < prv->vcpu_migr_delay;
>>  
>>      if ( hot )
>>          SCHED_STAT_CRANK(vcpu_hot);
>> @@ -703,7 +703,8 @@ __csched_vcpu_is_cache_hot(struct vcpu *v)
>>  }
>>  
>>  static inline int
>> -__csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
>> +__csched_vcpu_is_migrateable(const struct csched_private *prv, struct vcpu *vc,
>> +                             int dest_cpu, cpumask_t *mask)
>>  {
>>      /*
>>       * Don't pick up work that's hot on peer PCPU, or that can't (or
>> @@ -714,7 +715,7 @@ __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
>>       */
>>      ASSERT(!vc->is_running);
>>  
>> -    return !__csched_vcpu_is_cache_hot(vc) &&
>> +    return !__csched_vcpu_is_cache_hot(prv, vc) &&
>>             cpumask_test_cpu(dest_cpu, mask);
>>  }
>>  
>> @@ -1251,7 +1252,9 @@ csched_sys_cntl(const struct scheduler *ops,
>>               || (params->ratelimit_us
>>                   && (params->ratelimit_us > XEN_SYSCTL_SCHED_RATELIMIT_MAX
>>                       || params->ratelimit_us < XEN_SYSCTL_SCHED_RATELIMIT_MIN))
>> -             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms) )
>> +             || MICROSECS(params->ratelimit_us) > MILLISECS(params->tslice_ms)
>> +             || (params->vcpu_migr_delay_us
>> +                 && MICROSECS(params->vcpu_migr_delay_us) >= XEN_SYSCTL_CSCHED_MGR_DLY_MAX) )
>>                  goto out;
>>  
>>          spin_lock_irqsave(&prv->lock, flags);
>> @@ -1261,12 +1264,14 @@ csched_sys_cntl(const struct scheduler *ops,
>>          else if ( prv->ratelimit && !params->ratelimit_us )
>>              printk(XENLOG_INFO "Disabling context switch rate limiting\n");
>>          prv->ratelimit = MICROSECS(params->ratelimit_us);
>> +        prv->vcpu_migr_delay = MICROSECS(params->vcpu_migr_delay_us);
>>          spin_unlock_irqrestore(&prv->lock, flags);
>>  
>>          /* FALLTHRU */
>>      case XEN_SYSCTL_SCHEDOP_getinfo:
>>          params->tslice_ms = prv->tslice / MILLISECS(1);
>>          params->ratelimit_us = prv->ratelimit / MICROSECS(1);
>> +        params->vcpu_migr_delay_us = prv->vcpu_migr_delay / MICROSECS(1);
>>          rc = 0;
>>          break;
>>      }
>> @@ -1608,6 +1613,7 @@ csched_tick(void *_cpu)
>>  static struct csched_vcpu *
>>  csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
>>  {
>> +    const struct csched_private * const prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
>>      const struct csched_pcpu * const peer_pcpu = CSCHED_PCPU(peer_cpu);
>>      struct csched_vcpu *speer;
>>      struct list_head *iter;
>> @@ -1657,7 +1663,7 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
>>              continue;
>>  
>>          affinity_balance_cpumask(vc, balance_step, cpumask_scratch);
>> -        if ( __csched_vcpu_is_migrateable(vc, cpu, cpumask_scratch) )
>> +        if ( __csched_vcpu_is_migrateable(prv, vc, cpu, cpumask_scratch) )
>>          {
>>              /* We got a candidate. Grab it! */
>>              TRACE_3D(TRC_CSCHED_STOLEN_VCPU, peer_cpu,
>> @@ -2114,7 +2120,7 @@ csched_dump(const struct scheduler *ops)
>>             "\tratelimit          = %"PRI_stime"us\n"
>>             "\tcredits per msec   = %d\n"
>>             "\tticks per tslice   = %d\n"
>> -           "\tmigration delay    = %uus\n",
>> +           "\tmigration delay    = %"PRI_stime"us\n",
>>             prv->ncpus,
>>             prv->master,
>>             prv->credit,
>> @@ -2126,7 +2132,7 @@ csched_dump(const struct scheduler *ops)
>>             prv->ratelimit / MICROSECS(1),
>>             CSCHED_CREDITS_PER_MSEC,
>>             prv->ticks_per_tslice,
>> -           vcpu_migration_delay);
>> +           prv->vcpu_migr_delay/ MICROSECS(1));
>>  
>>      cpumask_scnprintf(idlers_buf, sizeof(idlers_buf), prv->idlers);
>>      printk("idlers: %s\n", idlers_buf);
>> @@ -2209,6 +2215,16 @@ csched_init(struct scheduler *ops)
>>      }
>>      else
>>          prv->ratelimit = MICROSECS(sched_ratelimit_us);
>> +
>> +    if ( !vcpu_migration_delay && vcpu_migration_delay > MILLISECS(100) )
> 
> Also, this expression can never evaluate to true; the first element
> requires 0, the second requires non-zero.  I think you want to get rid
> of the `!`.

Or, by the same argument, get rid of the first clause altogether, as if
it's > 100 then it cannot possibly be zero. :-)

 -George

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

Re: [PATCH v2 3/5] tools: libxl/xl: allow to get/set Credit1's vcpu_migration_delay

[George Dunlap]

On 02/23/2018 04:41 PM, Dario Faggioli wrote:
> Make it possible to get and set a (Credit1) scheduler's
> vCPU migration delay via the SCHEDOP sysctl, from both
> libxl and xl (no change needed in libxc).
> 
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes from v1:
> * add missing 'break', fix using wrong variable in xl_sched.c.
> 
> Changes are small, but still I'm dropping Wei's Ack.
> ---
>  docs/man/xl.pod.1.in        |   11 +++++++++++
>  tools/libxl/libxl.h         |    7 +++++++
>  tools/libxl/libxl_sched.c   |   10 ++++++++++
>  tools/libxl/libxl_types.idl |    1 +
>  tools/xl/xl_cmdtable.c      |    1 +
>  tools/xl/xl_sched.c         |   23 ++++++++++++++++-------
>  6 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
> index 7fd35c9ae7..48da2052cc 100644
> --- a/docs/man/xl.pod.1.in
> +++ b/docs/man/xl.pod.1.in
> @@ -1046,6 +1046,17 @@ we will allow a higher-priority VM to pre-empt it.  The default value
>  is 1000 microseconds (1ms).  Valid range is 100 to 500000 (500ms).
>  The ratelimit length must be lower than the timeslice length.
>  
> +=item B<-m DELAY>, B<--migration_delay_us=DELAY>
> +
> +Migration delay specifies for how long a vCPU, after it stopped running should
> +be considered "cache-hot". Basically, if less than DELAY us passed since when
> +the vCPU was executing on a CPU, it is likely that most of the vCPU's working
> +set is still in the CPU's cache, and therefore the vCPU is not migrated.
> +
> +Default is 0. Maximum is 100 ms. This can be effective at preventing vCPUs
> +to bounce among CPUs too quickly, but, at the same time, the scheduler stops
> +being fully work-conserving.
> +
>  =back
>  
>  B<COMBINATION>
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index eca0ea2c50..edd244278a 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -299,6 +299,13 @@
>   */
>  #define LIBXL_HAVE_SCHED_CREDIT2_PARAMS 1
>  
> +/*
> + * LIBXL_HAVE_SCHED_CREDIT_MIGR_DELAY indicates that there is a field
> + * in libxl_sched_credit_params called vcpu_migr_delay_us which controls
> + * the resistance of the vCPUs of the cpupool to migrations among pCPUs.
> + */
> +#define LIBXL_HAVE_SCHED_CREDIT_MIGR_DELAY
> +
>  /*
>   * LIBXL_HAVE_VIRIDIAN_CRASH_CTL indicates that the 'crash_ctl' value
>   * is present in the viridian enlightenment enumeration.
> diff --git a/tools/libxl/libxl_sched.c b/tools/libxl/libxl_sched.c
> index 512788f736..07289079ce 100644
> --- a/tools/libxl/libxl_sched.c
> +++ b/tools/libxl/libxl_sched.c
> @@ -291,6 +291,7 @@ int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid,
>  
>      scinfo->tslice_ms = sparam.tslice_ms;
>      scinfo->ratelimit_us = sparam.ratelimit_us;
> +    scinfo->vcpu_migr_delay_us = sparam.vcpu_migr_delay_us;
>  
>      rc = 0;
>   out:
> @@ -321,9 +322,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
>          rc = ERROR_INVAL;
>          goto out;
>      }
> +    if (scinfo->vcpu_migr_delay_us
> +        && scinfo->vcpu_migr_delay_us > XEN_SYSCTL_CSCHED_MGR_DLY_MAX) {

Same idea -- no need to check if it's zero and then check to see if it's
greater than DLY_MAX.

Other than that, looks good.

 -George

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

Re: [PATCH v2 4/5] tools: xenpm: continue to support {set, get}-vcpu-migration-delay

[George Dunlap]

On 02/23/2018 04:41 PM, Dario Faggioli wrote:
> Now that it is possible to get and set the migration
> delay via the SCHEDOP sysctl, use that in xenpm, instead
> of the special purpose libxc interface (which will be
> removed in a following commit).
> 
> The sysctl, however, requires a cpupool-id argument,
> for knowing on which scheduler it is operating on. In
> this case, since we don't want to alter xenpm's command
> line interface, we always use '0', which means xenpm
> will always act on the default cpupool ('Pool-0').
> 
>>From this commit on, `xenpm {set,get}-vcpu-migration-delay'
> commands work again. But that is only for the sake of
> backward compatibility, and their use is deprecated, in
> favour of 'xl sched-credit -s [-c <poolid>] -m <delay>'.
> 
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

If we cared about strict behavioral compatibility, we would modify the
xenpm function to set the migration delay for all cpupools; but I think
we can worry about that if anyone complains. :-)

One comment...

> @@ -1087,13 +1097,17 @@ void set_vcpu_migration_delay_func(int argc, char *argv[])
>  
>  void get_vcpu_migration_delay_func(int argc, char *argv[])
>  {
> -    uint32_t value;
> +    struct xen_sysctl_credit_schedule sparam;
> +
> +    printf("WARNING: using xenpm for this purpose is deprecated."
> +           " Check out `xl sched-credit -s'\n");

Should these warnings be to stderr rather than stdout, so that if
anything is manually parsing stout it will continue to work?

Everything else looks good, thanks.

 -George

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

Re: [PATCH v2 4/5] tools: xenpm: continue to support {set, get}-vcpu-migration-delay

[Dario Faggioli]

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

On Wed, 2018-02-28 at 15:02 +0000, George Dunlap wrote:
> On 02/23/2018 04:41 PM, Dario Faggioli wrote:
> > From this commit on, `xenpm {set,get}-vcpu-migration-delay'
> > 
> > commands work again. But that is only for the sake of
> > backward compatibility, and their use is deprecated, in
> > favour of 'xl sched-credit -s [-c <poolid>] -m <delay>'.
> > 
> > Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> If we cared about strict behavioral compatibility, we would modify
> the
> xenpm function to set the migration delay for all cpupools; but I
> think
> we can worry about that if anyone complains. :-)
> 
Yes, I thought about that. But, considering what would be required to
implement such behavior, as compared to how many people (as far as we
can tell, of course) use this feature, and use xenpm to change it, I
also thought we are indeed fine "waiting and seeing".

> > @@ -1087,13 +1097,17 @@ void set_vcpu_migration_delay_func(int
> > argc, char *argv[])
> >  
> >  void get_vcpu_migration_delay_func(int argc, char *argv[])
> >  {
> > -    uint32_t value;
> > +    struct xen_sysctl_credit_schedule sparam;
> > +
> > +    printf("WARNING: using xenpm for this purpose is deprecated."
> > +           " Check out `xl sched-credit -s'\n");
> 
> Should these warnings be to stderr rather than stdout, so that if
> anything is manually parsing stout it will continue to work?
> 
Again, I considered this, for that same reason. The reason why I
decided for stdout is for maximizing the chances that anyone actually
using this will see the warning (even if, e.g., they're redirecting
stderr).

But I am fine with both, actually, just let me know what's considered
best.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

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

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

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

Re: [PATCH v2 4/5] tools: xenpm: continue to support {set, get}-vcpu-migration-delay

[George Dunlap]

On 02/28/2018 03:17 PM, Dario Faggioli wrote:
> On Wed, 2018-02-28 at 15:02 +0000, George Dunlap wrote:
>> On 02/23/2018 04:41 PM, Dario Faggioli wrote:
>>> From this commit on, `xenpm {set,get}-vcpu-migration-delay'
>>>
>>> commands work again. But that is only for the sake of
>>> backward compatibility, and their use is deprecated, in
>>> favour of 'xl sched-credit -s [-c <poolid>] -m <delay>'.
>>>
>>> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>
>> If we cared about strict behavioral compatibility, we would modify
>> the
>> xenpm function to set the migration delay for all cpupools; but I
>> think
>> we can worry about that if anyone complains. :-)
>>
> Yes, I thought about that. But, considering what would be required to
> implement such behavior, as compared to how many people (as far as we
> can tell, of course) use this feature, and use xenpm to change it, I
> also thought we are indeed fine "waiting and seeing".
> 
>>> @@ -1087,13 +1097,17 @@ void set_vcpu_migration_delay_func(int
>>> argc, char *argv[])
>>>  
>>>  void get_vcpu_migration_delay_func(int argc, char *argv[])
>>>  {
>>> -    uint32_t value;
>>> +    struct xen_sysctl_credit_schedule sparam;
>>> +
>>> +    printf("WARNING: using xenpm for this purpose is deprecated."
>>> +           " Check out `xl sched-credit -s'\n");
>>
>> Should these warnings be to stderr rather than stdout, so that if
>> anything is manually parsing stout it will continue to work?
>>
> Again, I considered this, for that same reason. The reason why I
> decided for stdout is for maximizing the chances that anyone actually
> using this will see the warning (even if, e.g., they're redirecting
> stderr).

If they're redirecting stderr then they have explicitly decided to
ignore warnings and errors -- if they want to miss messages like this
they should be allowed to.  I would think it much more likely that
stdout would be redirected somewhere than stderr.

 -George

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

Re: [PATCH v2 4/5] tools: xenpm: continue to support {set, get}-vcpu-migration-delay

[Dario Faggioli]

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

Il 28 Feb 2018 17:03, "George Dunlap" <george.dunlap@citrix.com> ha scritto:


> Again, I considered this, for that same reason. The reason why I
> decided for stdout is for maximizing the chances that anyone actually
> using this will see the warning (even if, e.g., they're redirecting
> stderr).

If they're redirecting stderr then they have explicitly decided to
ignore warnings and errors -- if they want to miss messages like this
they should be allowed to.  I would think it much more likely that
stdout would be redirected somewhere than stderr.


This last one is a good point. IAC, as I said, I had doubts myself already,
so I'm fine turning this into stderr.

Thanks and Regards,
Dario

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

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

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