On 30/06/2023 12:37 pm, George Dunlap wrote:
> The credit scheduler tries as hard as it can to ensure that it always
> runs scheduling units with positive credit (PRI_TS_UNDER) before
> running those with negative credit (PRI_TS_OVER). If the next
> runnable scheduling unit is of priority OVER, it will always run the
> load balancer, which will scour the system looking for another
> scheduling unit of the UNDER priority.
>
> Unfortunately, as the number of cores on a system has grown, the cost
> of the work-stealing algorithm has dramatically increased; a recent
> trace on a system with 128 cores showed this taking over 50
> microseconds.
>
> Add a parameter, load_balance_ratelimit, to limit the frequency of
> load balance operations on a given pcpu. Default this to 1
> millisecond.
>
> Invert the load balancing conditional to make it more clear, and line
> up more closely with the comment above it.
>
> Overall it might be cleaner to have the last_load_balance checking
> happen inside csched_load_balance(), but that would require either
> passing both now and spc into the function, or looking them up again;
> both of which seemed to be worse than simply checking and setting the
> values before calling it.
>
> Without this patch, on a system with a vcpu:pcpu ratio of 2:1, running
> Windows guests (which will end up calling YIELD during spinlock
> contention), this patch increased performance significantly.
>
> Signed-off-by: George Dunlap <george.dun...@cloud.com>
> ---
> CC: Dario Faggioli <dfaggi...@suse.com>
> CC: Andrew Cooper <andrew.coop...@citrix.com>
> CC: George Dunlap <george.dun...@citrix.com>
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Julien Grall <jul...@xen.org>
> CC: Stefano Stabellini <sstabell...@kernel.org>
> CC: Wei Liu <w...@xen.org>
> ---
> docs/misc/xen-command-line.pandoc | 6 +++++
> xen/common/sched/credit.c | 40 ++++++++++++++++++++++++++-----
> xen/include/public/sysctl.h | 6 +++++
Given this filelist, why the sysctl change?
There's no logic to drive this parameter in the xc/libxl param get/set.
The only two in-tree users I can see are xenpm, along with an
unconditional print to stderr saying it's deprecated and to use xl, and xl.
> diff --git a/docs/misc/xen-command-line.pandoc
> b/docs/misc/xen-command-line.pandoc
> index 4060ebdc5d..369557020f 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1856,6 +1856,12 @@ By default, Xen will use the INVPCID instruction for
> TLB management if
> it is available. This option can be used to cause Xen to fall back to
> older mechanisms, which are generally slower.
>
> +### load-balance-ratelimit
> +> `= <integer>`
> +
> +The minimum interval between load balancing events on a given pcpu.
> +At the moment only credit honors this parameter.
So this is intended to be a global scheduler parameter?
> diff --git a/xen/common/sched/credit.c b/xen/common/sched/credit.c
> index f2cd3d9da3..b8bdfd5f6a 100644
> --- a/xen/common/sched/credit.c
> +++ b/xen/common/sched/credit.c
> @@ -1267,7 +1272,8 @@ csched_sys_cntl(const struct scheduler *ops,
> && (params->ratelimit_us > XEN_SYSCTL_SCHED_RATELIMIT_MAX
> || params->ratelimit_us <
> XEN_SYSCTL_SCHED_RATELIMIT_MIN))
> || MICROSECS(params->ratelimit_us) >
> MILLISECS(params->tslice_ms)
> - || params->vcpu_migr_delay_us >
> XEN_SYSCTL_CSCHED_MGR_DLY_MAX_US )
> + || params->vcpu_migr_delay_us > XEN_SYSCTL_CSCHED_MGR_DLY_MAX_US
> + || params->load_balance_ratelimit_us >
> XEN_SYSCTL_CSCHED_LB_RATE_MAX_US)
Style (give or take this hunk being with some logic to drive the new
sysctl).
> @@ -1963,10 +1979,12 @@ static void cf_check csched_schedule(
> * urgent work... If not, csched_load_balance() will return snext,
> but
> * already removed from the runq.
> */
> - if ( snext->pri > CSCHED_PRI_TS_OVER )
> - __runq_remove(snext);
> - else
> + if ( snext->pri <= CSCHED_PRI_TS_OVER
> + && now - spc->last_load_balance > prv->load_balance_ratelimit) {
> + spc->last_load_balance = now;
> snext = csched_load_balance(prv, sched_cpu, snext, &migrated);
> + } else
> + __runq_remove(snext);
Style.
~Andrew
