Re: [Xen-devel] [PATCH v3 33/47] xen/sched: add a percpu resource index

[Jürgen Groß <jgross@xxxxxxxx>]

On 24.09.19 12:05, Jan Beulich wrote:
> On 14.09.2019 10:52, Juergen Gross wrote:
> > --- a/xen/common/schedule.c
> > +++ b/xen/common/schedule.c
> > @@ -71,6 +71,7 @@ static void poll_timer_fn(void *data);
> >   /* This is global for now so that private implementations can reach it */
> >   DEFINE_PER_CPU(struct scheduler *, scheduler);
> >   DEFINE_PER_CPU_READ_MOSTLY(struct sched_resource *, sched_res);
> > +static DEFINE_PER_CPU_READ_MOSTLY(unsigned int, sched_res_idx);
> It's of course not very helpful that this variable (right here) doesn't
> ever get written to, and hence one can't judge where / how this is to
> be done (without going look elsewhere). But I guess that calculation
> can't be moved into this patch (accepting that it would always yield
> zero for now)?
Not easily. I can set sched_res_idx to 0 explicitly when initializing
a new cpu if you like that better.

> > @@ -135,6 +136,12 @@ static struct scheduler sched_idle_ops = {
> >       .switch_sched   = sched_idle_switch_sched,
> >   };
> >   
> > +static inline struct vcpu *sched_unit2vcpu_cpu(struct sched_unit *unit,
> const (on the parameter)?
Okay.

> > +                                               unsigned int cpu)
> > +{
> > +    return unit->domain->vcpu[unit->unit_id + per_cpu(sched_res_idx, cpu)];
> > +}
> > +
> >   static inline struct scheduler *dom_scheduler(const struct domain *d)
> >   {
> >       if ( likely(d->cpupool != NULL) )
> > @@ -2008,7 +2015,7 @@ static void sched_slave(void)
> >   
> >       pcpu_schedule_unlock_irq(lock, cpu);
> >   
> > -    sched_context_switch(vprev, next->vcpu_list, now);
> > +    sched_context_switch(vprev, sched_unit2vcpu_cpu(next, cpu), now);
> >   }
> By the end of the series this will be
>
>      sched_context_switch(vprev, sched_unit2vcpu_cpu(next, cpu),
>                           is_idle_unit(next) && !is_idle_unit(prev), now);
>
> clarifying (I think) that "next" can indeed be an idle unit. I'm having
> difficulty seeing how can produce the correct result in all three cases
> - all vCPU-s in the unit belong to a real domain
> - all vCPU-s in the unit belong to the idle domain
> - there's a mix of "real" and (filler) "idle" vCPU-s
> My main question is what "next" is in the last of the 3 cases above.
next points to the real domain's unit.

> Considering that scheduling happens in terms of units, I'd expect it to
> be a real domain's unit, yet then I don't see how you'd get at the
> correct (idle) vCPU when calling sched_unit2vcpu_cpu(next, cpu).
What is the problem?

sched_unit2vcpu_cpu() will first get the real domain's vcpu for that cpu
(if exisiting) and replace it by the idle vcpu of that cpu if the real
domain's vcpu is either not existing or won't be running. The main trick
is that we use idle_vcpu[cpu] (which is existing today already).


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel