Re: [PATCH v2 11/11] xen: credit2: implement true SMT support

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

On 15/07/16 15:50, Dario Faggioli wrote:
> In fact, right now, we recommend keepeing runqueues
> arranged per-core, so that it is the inter-runqueue load
> balancing code that automatically spreads the work in an
> SMT friendly way. This means that any other runq
> arrangement one may want to use falls short of SMT
> scheduling optimizations.
> 
> This commit implements SMT awareness --similar to the
> one we have in Credit1-- for any possible runq
> arrangement. This turned out to be pretty easy to do,
> as the logic can live entirely in runq_tickle()
> (although, in order to avoid for_each_cpu loops in
> that function, we use a new cpumask which indeed needs
> to be updated in other places).
> 
> In addition to disentangling SMT awareness from load
> balancing, this also allows us to support the
> sched_smt_power_savings parametar in Credit2 as well.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Reviewed-by: Anshul Makkar <anshul.makkar@citrix.com>
> ---
> Cc: George Dunlap <george.dunlap@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
> Changes from v1:
>  * make smt_idle_mask_set() const correct, and use a local variable for
>    per_cpu(cpu_sibling_masks) in there, as suggested during review;
>  * removed a stray XXX, as pointed out during review;
>  * full stop in a comment, as requested during review.
> ---
>  xen/common/sched_credit2.c |  142 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 128 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index b33ba7a..6ccc6f0 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c

[snip]

> +/*
> + * If all the siblings of cpu (including cpu itself) are in idlers,
> + * set all their bits in mask.
> + *
> + * In order to properly take into account tickling, idlers needs to be
> + * set qeual to something like:

*equal (I can fix this on check-in)

> + *
> + *  rqd->idle & (~rqd->tickled)
> + *
> + * This is because cpus that have been tickled will very likely pick up some
> + * work as soon as the manage to schedule, and hence we should really consider
> + * them as busy.

OK this is something that slightly confused me when I was reviewing the
patch the first time: that rqd->idle is *all* pcpus which are currently
idle (and thus we need to & (~tickled) when using it), but rqd->smt_idle
is meant to be maintained as *non-tickled* idle pcpus.

Are you planning at some point to have a follow-up patch which changes
rqd->idle to be non-tickled idle pcpus as well?  Unless I missed
something it looks like at the moment the only times rqd->idle is acted
upon is after &~-ing out rqd->tickled anyway.

 -George

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