Re: [Xen-devel] [PATCH v2] xen/sched: fix cpu offlining with core scheduling

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

On 04.03.20 10:53, Jan Beulich wrote:
> On 03.03.2020 18:39, Juergen Gross wrote:
> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -2299,6 +2299,10 @@ void sched_context_switched(struct vcpu *vprev, struct 
> > vcpu *vnext)
> >       rcu_read_unlock(&sched_res_rculock);
> >   }
> >   
> > +/*
> > + * Switch to a new context or keep the current one running.
> > + * On x86 it won't return, so it will drop the still held sched_res_rculock.
> > + */
> >   static void sched_context_switch(struct vcpu *vprev, struct vcpu *vnext,
> >                                    bool reset_idle_unit, s_time_t now)
> >   {
> I don't follow the comment: There's
>
>          return continue_running(vprev);
>
> in the function which afaict can happen on all architectures.
> The lock gets dropped there too. I see no path through this
> function where the lock would not get dropped.
It was meant as reasoning: due to the fact that sched_context_switch()
won't return on x86, it is required that it will drop sched_res_rculock.

> > @@ -2408,6 +2412,9 @@ static struct vcpu *sched_force_context_switch(struct 
> > vcpu *vprev,
> >    * zero do_schedule() is called and the rendezvous counter for leaving
> >    * context_switch() is set. All other members will wait until the counter is
> >    * becoming zero, dropping the schedule lock in between.
> > + * Either returns the new unit to run, or NULL if no context switch is
> > + * required or (on ARM) has already been performed. If NULL is returned
> > + * sched_res_rculock has been dropped.
> I guess official Arm folks would like Arm to not be spelled all
> upper case anymore.
Okay.

> > @@ -2482,6 +2490,21 @@ static struct sched_unit 
> > *sched_wait_rendezvous_in(struct sched_unit *prev,
> >               atomic_set(&prev->next_task->rendezvous_out_cnt, 0);
> >               prev->rendezvous_in_cnt = 0;
> >           }
> > +
> > +        /*
> > +         * Check for scheduling resourced switched. This happens when we are
> > +         * moved away from our cpupool and cpus are subject of the idle
> > +         * scheduler now.
> > +         */
> The 'd' on both "resourced" and "switched" are odd to read at
> least to me, and hence make me uncertain whether I actually
> correctly understand what is meant here.
Should be "resources", of course.


Juergen


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