Re: [PATCH 1/6] xen: credit1: simplify csched_runq_steal() a little bit.

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3413 bytes --]

On Fri, 2017-03-03 at 09:35 +0000, anshul makkar wrote:
> On 02/03/17 10:38, Dario Faggioli wrote:
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -1593,64 +1593,65 @@ static struct csched_vcpu *
> >   csched_runq_steal(int peer_cpu, int cpu, int pri, int
> > balance_step)
> >   {
> >       const struct csched_pcpu * const peer_pcpu =
> > CSCHED_PCPU(peer_cpu);
> > -    const struct vcpu * const peer_vcpu = curr_on_cpu(peer_cpu);
> >       struct csched_vcpu *speer;
> >       struct list_head *iter;
> >       struct vcpu *vc;
> >   
> > +    ASSERT(peer_pcpu != NULL);
> > +
> >       /*
> >        * Don't steal from an idle CPU's runq because it's about to
> >        * pick up work from it itself.
> >        */
> > -    if ( peer_pcpu != NULL && !is_idle_vcpu(peer_vcpu) )
> > +    if ( unlikely(is_idle_vcpu(curr_on_cpu(peer_cpu))) )
> > +        goto out;
> We can just use if (!is_idle_vcpu(peer_vcpu)). Why to replace it
> with 
> some code that introduces an unnecessary branch statement.
>
Mmm... I don't think I understand what this means.

> > +        /*
> > +         * If the vcpu has no useful soft affinity, skip this
> > vcpu.
> > +         * 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".
> > +         *
> > +         * Notice that, if not even one vCPU on this runq has a
> > useful
> > +         * soft affinity, we could have avoid considering this
> > runq for
> > +         * a soft balancing step in the first place. This, for
> > instance,
> > +         * can be implemented by taking note of on what runq there
> > are
> > +         * vCPUs with useful soft affinities in some sort of
> > bitmap
> > +         * or counter.
> > +         */
>
> Isn't it a better approach that now as we have came across a vcpu
> which 
> doesn't have a desired soft affinity but is a potential candidate
> for 
> migration, so instead of just forgetting it,  lets save the
> information 
> for such vcpus in some data structure in some order so that we don't 
> have to scan them again if we don't find anything useful in the
> present run.
>
So, AFAIUI, you're suggesting something like this:
 1. for each vcpu in the runqueue, we check soft-affinity. If it 
    matches, we're done;
 2. if it does not match, we check hard-affinity. If it matches, we 
    save that vcpu somewhere. We only need to save one vcpu, the first 
    one that we find to have matching hard-affinity;
 3. if we don't find any vcpu with matching soft affinity, we steal 
    the one we've saved.

Is this correct? If yes, if there's not anything I'm overlooking, I
think it could work.

It's a separate patch, of course. I can try putting that together,
unless of course you want to give this a go yourself. :-)

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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