[Xen-devel] [PATCH 0/3] xen: cpupools: avoid crashing when shutting down/suspending with free CPUs

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

In fact, in the following situation:

 root@Zhaman:~# xl cpupool-cpu-remove Pool-0 node:1,^9-11
 root@Zhaman:~# xl cpupool-list -c
 Name               CPU list
 Pool-0             0,1,2,3,4,5,6,7,9,10,11

Trying to power off the system would results in this splat:

(XEN) Preparing system for ACPI S5 state.
(XEN) Disabling non-boot CPUs ...
(XEN) Error taking CPU8 down: -16
(XEN) Xen BUG at cpu.c:191
(XEN) ----[ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82d080101757>] disable_nonboot_cpus+0xb5/0x138
(XEN) RFLAGS: 0000000000010246   CONTEXT: hypervisor
(XEN) rax: 0000000000000000   rbx: 0000000000000008   rcx: 0000000000000000
(XEN) rdx: ffff82d0802e8000   rsi: 000000000000000a   rdi: ffff82d08029b69c
(XEN) rbp: ffff82d0802efe30   rsp: ffff82d0802efe10   r8:  ffff830320700000
(XEN) r9:  0000000000000002   r10: 0000000000000010   r11: 0000000000000002
(XEN) r12: 0000000000000008   r13: ffff82d080288b80   r14: 00000000fffffff0
(XEN) r15: 0000000000000003   cr0: 000000008005003b   cr4: 00000000000026f0
(XEN) cr3: 00000000dba99000   cr2: ffff88000e8f2cb0
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) Xen stack trace from rsp=ffff82d0802efe10:
(XEN)    0000000000000000 0000000000000005 ffff82d080334ae0 ffff82d080334a08
(XEN)    ffff82d0802efe80 ffff82d0801a8824 ffff82d0802efe60 ffff82d080167e6e
(XEN)    ffff82d080334a08 ffff8303191c0680 ffff8300dbb3b000 ffff82d080334ae0
(XEN)    ffff82d080334a08 0000000000000003 ffff82d0802efea0 ffff82d08010614a
(XEN)    ffff8300dbb3b1c8 0000000000000000 ffff82d0802efec0 ffff82d0801320bd
(XEN)    ffff82d08012fef1 ffff82d080334af0 ffff82d0802efef0 ffff82d0801323f3
(XEN)    ffff8300dbb3b000 ffff82d0802e8000 ffff8300dbb3b000 00000000ffffff01
(XEN)    ffff82d0802eff10 ffff82d080163cb6 ffff82d08012f67d ffff8300dbdf4000
(XEN)    ffff82d0802efde8 00000000fee1dead 0000000000000000 0000000000002803
(XEN)    0000000000000005 ffff880012a53d48 00000000ffffffff 0000000000000246
(XEN)    0000000000000010 0000000000002803 0000000000002803 0000000000000000
(XEN)    ffffffff810010ea 00000000ffff0000 0000000000002803 00000000deadbeef
(XEN)    0000010000000000 ffffffff810010ea 000000000000e033 0000000000000246
(XEN)    ffff880012a53c90 000000000000e02b 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 ffff8300dbdf4000
(XEN)    0000000000000000 0000000000000000
(XEN) Xen call trace:
(XEN)    [<ffff82d080101757>] disable_nonboot_cpus+0xb5/0x138
(XEN)    [<ffff82d0801a8824>] enter_state_helper+0xbd/0x369
(XEN)    [<ffff82d08010614a>] continue_hypercall_tasklet_handler+0x4a/0xb1
(XEN)    [<ffff82d0801320bd>] do_tasklet_work+0x78/0xab
(XEN)    [<ffff82d0801323f3>] do_tasklet+0x5e/0x8a
(XEN)    [<ffff82d080163cb6>] idle_loop+0x56/0x6b
(XEN) 
(XEN) 
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Xen BUG at cpu.c:191
(XEN) ****************************************

(NB. Output is with patch #1 applied, so we can see that the problem arises
when tearing down CPU #8.)

That is because, for CPUs that are not assigned to any cpupool, -EBUSY is
returned, in cpupool_cpu_remove(), and that makes cpu_down() (invoked by
disable_nonboot_cpus() ) really, really angry.

The main focus of this series is hence to fix this, by just avoiding returning
-EBUSY for free CPUs, during system shutdown or suspend, while still not
allowing to hot-unplug a CPU that does not belong to cpupool0 (i.e., either
free or assigned to another pool), as it was by design. When we are shutting
down, we really don't care much. For the suspend/resume case, I think I managed
to put things in such a way that free CPUs at suspend time stay free after
resuming.

This happens in patch #3, together with a few other improvements to the
involved code, in the form of better (IMO, of course :-D) comments, and a
couple of ASSERT()-s. Patch 1 and 2 are mostly about improving debugging (patch
#1) and code cleanup (patch #2).

The only problem I have is that I don't have a box that suspends and then
resumes nicely, so I couldn't really test that path (I think I got it right,
though).

This is available as a git branch here:

 git://xenbits.xen.org/people/dariof/xen.git rel/cpupools/shutdown-fix-v1
 
http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/cpupools/shutdown-fix-v1

Thanks and Regards,
Dario
---
Dario Faggioli (3):
      xen: always print offending CPU on bringup/teardown failure
      xen: cpupool: assigning a CPU to a pool can fail
      xen: cpupools: avoid crashing if shutting down with free CPUs


 xen/common/cpu.c     |    4 +-
 xen/common/cpupool.c |   95 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 69 insertions(+), 30 deletions(-)
--
<<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)

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