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

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

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

On Thu, 2015-07-09 at 12:45 +0200, Juergen Gross wrote:
> On 07/09/2015 12:24 PM, Dario Faggioli wrote:
> > On Wed, 2015-07-08 at 17:13 +0200, Dario Faggioli wrote:
> >> On Tue, 2015-07-07 at 13:16 +0200, Juergen Gross wrote:
> >>>> +            /*
> >>>> +             * 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".
> 
> We are running on the vcpu which issued the hypercall resulting in
> pausing the domains. A vcpu can't pause itself.
> 
Hey, sorry it took me a bit to reply.

Actually, I'm still not getting the "we are running on the vcpu" part.
Perhaps it's all a terminology issue that we have, and it may not be
that much worthwhile to spend a lot of time on it.

However, the hypercall to suspend/shutdown the system has indeed been
issued by a dom0 vcpu. The call chain is this:

XENPF_enter_acpi_sleep:
  acpi_enter_sleep()
    continue_hypercall_on_cpu(0, enter_state_helper)
      enter_state()
        disable_nonboot_cpus()
          for_each_online_cpu { cpu_down() }
                                  stop_machine_run(take_cpu_down)
                                    take_cpu_down()
                                      __cpu_disable()
                                        cpu_disable_scheduler()

The doc comment of acpu_enter_sleep_state() is as follows:

/*
 * Dom0 issues this hypercall in place of writing pm1a_cnt. Xen then
 * takes over the control and put the system into sleep state really.
 */

The pausing happens from within enter_state(), before calling
disable_nonboot_cpus(), in freeze_domains(). freeze_domains() does this:

    /*
     * Note that we iterate in order of domain-id. Hence we will pause dom0
     * first which is required for correctness (as only dom0 can add domains to
     * the domain list). Otherwise we could miss concurrently-created domains.
     */
    for_each_domain ( d )
        domain_pause(d);

Looking inside domain_pause(), I found this ASSERT:

  ASSERT(d != current->domain);

meaning that, if one of the dom0 vcpus were running when we reach this
point, we would explode, wouldn't we?

I've added some quick-&-dirty printk in freeze_domains(), checking
what's current, and here's what they say:

(XEN) XXX freezing d0 from cpu 0, running d32767.v0
(XEN) XXX pausing d0.v0, while running d32767.v0
(XEN) XXX pausing d0.v1, while running d32767.v0
(XEN) XXX pausing d0.v2, while running d32767.v0
(XEN) XXX pausing d0.v3, while running d32767.v0
(XEN) XXX pausing d0.v4, while running d32767.v0
(XEN) XXX pausing d0.v5, while running d32767.v0
(XEN) XXX pausing d0.v6, while running d32767.v0
(XEN) XXX pausing d0.v7, while running d32767.v0

So, I'm probably missing what you actually mean with "we can be active
in dom0/Pool-0 only" and "We are running on the vcpu which issued the
hypercall resulting in pausing the domains".

The way I interpreted it was that a dom0 vcpu was current on some pcpu,
but that is clearly not the case.

> > So, I'm adding a line about things being paused. If you think I should
> > say anything more than that, let me know.
> 
> I think dom0/Pool-0 should be mentioned. The coding is written with this
> assumption so it should be documented.
> 
Yeah, but, given all the above, I'm not sure how...

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: 181 bytes --]

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

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