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

[George Dunlap <george.dunlap@citrix.com>]

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