[v2,4/6] xen: don't free percpu areas during suspend

Commit Message

Jürgen Groß March 28, 2019, 12:06 p.m. UTC

Instead of freeing percpu areas during suspend and allocating them
again when resuming keep them. Only free an area in case a cpu didn't
come up again when resuming.

It should be noted that there is a potential change in behaviour as
the percpu areas are no longer zeroed out during suspend/resume. While
I have checked the called cpu notifier hooks to cope with that there
might be some well hidden dependency on the previous behaviour. OTOH
a component not registering itself for cpu down/up and expecting to
see a zeroed percpu variable after suspend/resume is kind of broken
already. And the opposite case, where a component is not registered
to be called for cpu down/up and is not expecting a percpu variable
suddenly to be zero due to suspend/resume is much more probable,
especially as the suspend/resume functionality seems not to be tested
that often.

Signed-off-by: Juergen Gross <[e-mail]>
Reviewed-by: Dario Faggioli <[e-mail]>
---
 xen/arch/x86/percpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jan Beulich March 28, 2019, 1:39 p.m. UTC | #1

>>> On 28.03.19 at 13:06, <[e-mail]> wrote:
> Instead of freeing percpu areas during suspend and allocating them
> again when resuming keep them. Only free an area in case a cpu didn't
> come up again when resuming.
> 
> It should be noted that there is a potential change in behaviour as
> the percpu areas are no longer zeroed out during suspend/resume. While
> I have checked the called cpu notifier hooks to cope with that there
> might be some well hidden dependency on the previous behaviour. OTOH
> a component not registering itself for cpu down/up and expecting to
> see a zeroed percpu variable after suspend/resume is kind of broken
> already.

Back at the time it was intentional to have this behavior, and code
was in fact written to rely on it. What I can't immediately tell is
whether all such code has gone away.

> And the opposite case, where a component is not registered
> to be called for cpu down/up and is not expecting a percpu variable
> suddenly to be zero due to suspend/resume is much more probable,
> especially as the suspend/resume functionality seems not to be tested
> that often.

Right, but CPU offline/online is pretty easy to test (and
independent of firmware support), and it would continue to
have the observed effect. I agree though that for suspend/
resume it is more likely to be wanted to have the data
retained. But don't forget that it was always implied that APs
would go fully down and then come back up during a suspend/
resume cycle.

Jan

Patch

diff --git a/xen/arch/x86/percpu.c b/xen/arch/x86/percpu.c
index 8be4ebddf4..5ea14b6ec3 100644
--- a/xen/arch/x86/percpu.c
+++ b/xen/arch/x86/percpu.c
@@ -76,7 +76,8 @@  static int cpu_percpu_callback(
         break;
     case CPU_UP_CANCELED:
     case CPU_DEAD:
-        if ( !park_offline_cpus )
+    case CPU_RESUME_FAILED:
+        if ( !park_offline_cpus && system_state != SYS_STATE_suspend )
             free_percpu_area(cpu);
         break;
     case CPU_REMOVE: