Re: [Xen-devel] [PATCH 2/6] xen: credit: (micro) optimize csched_runq_steal().

[anshul makkar <anshul.makkar@xxxxxxxxxx>]

On 02/03/17 10:38, Dario Faggioli wrote:
> Chacking whether or not a vCPU can be 'stolen'
> from a peer pCPU's runqueue is relatively cheap.
>
> Therefore, let's do that  as early as possible,
> avoiding potentially useless complex checks, and
> cpumask manipulations.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> ---
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> ---
>   xen/common/sched_credit.c |   17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index 63a8675..2b13e99 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -708,12 +708,10 @@ static inline int
>   __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
>   {
>       /*
> -     * Don't pick up work that's in the peer's scheduling tail or hot on
> -     * peer PCPU. Only pick up work that prefers and/or is allowed to run
> -     * on our CPU.
> +     * Don't pick up work that's or hot on peer PCPU, or that can't (or
Not clear.
> +     * would prefer not to) run on cpu.
>        */
> -    return !vc->is_running &&
> -           !__csched_vcpu_is_cache_hot(vc) &&
> +    return !__csched_vcpu_is_cache_hot(vc) &&
>              cpumask_test_cpu(dest_cpu, mask);
!vc->is_running doesn't ease the complexity and doesn't save much on cpu 
cycles. Infact, I think (!vc->is_running) makes the intention to check 
for migration much more clear to understand.
Yeah, apart from the above reasons, its save to remove this check from here.
>   }
>   
> @@ -1622,7 +1620,9 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
>           BUG_ON( is_idle_vcpu(vc) );
>   
>           /*
> -         * If the vcpu has no useful soft affinity, skip this vcpu.
> +         * If the vcpu is still in peer_cpu's scheduling tail, or if it
> +         * has no useful soft affinity, skip it.
> +         *
>            * In fact, what we want is to check if we have any "soft-affine
>            * work" to steal, before starting to look at "hard-affine work".
>            *
> @@ -1633,8 +1633,9 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int 
> balance_step)
>            * vCPUs with useful soft affinities in some sort of bitmap
>            * or counter.
>            */
> -        if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY
> -             && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity) )
> +        if ( vc->is_running ||
> +             (balance_step == CSCHED_BALANCE_SOFT_AFFINITY
> +              && !__vcpu_has_soft_affinity(vc, vc->cpu_hard_affinity)) )
>               continue;
>   
>           csched_balance_cpumask(vc, balance_step, cpumask_scratch);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel

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