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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

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@xxxxxxxx>
> ---
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel