Re: [Xen-devel] [PATCH 3/4] xen: sched: reorganize cpu_disable_scheduler()

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

Hey,

1 thing...

On Wed, 2015-07-08 at 17:13 +0200, Dario Faggioli wrote:
> On Tue, 2015-07-07 at 13:16 +0200, Juergen Gross wrote:

> > > @@ -645,25 +675,72 @@ int cpu_disable_scheduler(unsigned int cpu)
> > >                   cpumask_setall(v->cpu_hard_affinity);
> > >               }
> > >
> > > -            if ( v->processor == cpu )
> > > +            if ( v->processor != cpu )
> > >               {
> > > -                set_bit(_VPF_migrating, &v->pause_flags);
> > > +                /* This vcpu is not on cpu, so we can move on. */
> > >                   vcpu_schedule_unlock_irqrestore(lock, flags, v);
> > > -                vcpu_sleep_nosync(v);
> > > -                vcpu_migrate(v);
> > > +                continue;
> > >               }
> > > -            else
> > > -                vcpu_schedule_unlock_irqrestore(lock, flags, v);
> > >
> > >               /*
> > > -             * A vcpu active in the hypervisor will not be migratable.
> > > -             * The caller should try again after releasing and reaquiring
> > > -             * all locks.
> > > +             * If we're here, it means that the vcpu is on cpu. Let's 
> > > see how
> > > +             * it's best to send it away, depending on whether we are 
> > > shutting
> > > +             * down/suspending, or doing cpupool manipulations.
> > >                */
> > > -            if ( v->processor == cpu )
> > > -                ret = -EAGAIN;
> > > -        }
> > > +            set_bit(_VPF_migrating, &v->pause_flags);
> > > +            vcpu_schedule_unlock_irqrestore(lock, flags, v);
> > > +            vcpu_sleep_nosync(v);
> > > +
> > > +            /*
> > > +             * In case of shutdown/suspend, it is not necessary to ask 
> > > the
> > > +             * scheduler to chime in. In fact:
> > > +             *  * there is no reason for it: the end result we are after 
> > > is
> > > +             *    just 'all the vcpus on the boot pcpu, and no vcpu 
> > > anywhere
> > > +             *    else', so let's just go for it;
> > > +             *  * it's wrong, when dealing a cpupool with only non-boot 
> > > pcpus,
> > > +             *    as the scheduler will always fail to send the vcpus 
> > > away
> > > +             *    from the last online (non boot) pcpu!
> > 
> > I'd add a comment that in shutdown/suspend case all domains are being
> > paused, so we can be active in dom0/Pool-0 only.
> > 
> Sure, I'll add this.
> 
...while putting such a comment together, I'm realizing that I'm not
sure about what you meant, or what you wanted the comment itself to
express.

I mean, it is certainly true that all domains are being paused (they've
been paused already, actually), but that include Dom0 too. Also, we are
in Xen, in stop_machine context, so I'm not sure what you meant either
with "we can be active in dom0/Pool-0 only".

So, I'm adding a line about things being paused. If you think I should
say anything more than that, let me know.

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: signature.asc
Description: This is a digitally signed message part

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