xen/timers: Fix memory leak with cpu unplug/plug
authorAndrew Cooper <andrew.cooper3@citrix.com>
Fri, 29 Mar 2019 16:17:24 +0000 (16:17 +0000)
committerAndrew Cooper <andrew.cooper3@citrix.com>
Mon, 8 Apr 2019 10:16:06 +0000 (11:16 +0100)
timer_softirq_action() realloc's itself a larger timer heap whenever
necessary, which includes bootstrapping from the empty dummy_heap.  Nothing
ever freed this allocation.

CPU plug and unplug has the side effect of zeroing the percpu data area, which
clears ts->heap.  This in turn causes new timers to be put on the list rather
than the heap, and for timer_softirq_action() to bootstrap itself again.

This in practice leaks ts->heap every time a CPU is unplugged and replugged.

Implement free_percpu_timers() which includes freeing ts->heap when
appropriate, and update the notifier callback with the recent cpu parking
logic and free-avoidance across suspend.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
xen/common/timer.c

index 98f2c4800caf83835746acc1e1a9bbbae064b7ed..f265a362dd6887e162cab6421088f6c79b12ad9f 100644 (file)
@@ -615,6 +615,22 @@ static void migrate_timers_from_cpu(unsigned int old_cpu)
  */
 static struct timer *dummy_heap[1];
 
+static void free_percpu_timers(unsigned int cpu)
+{
+    struct timers *ts = &per_cpu(timers, cpu);
+
+    migrate_timers_from_cpu(cpu);
+
+    ASSERT(heap_metadata(ts->heap)->size == 0);
+    if ( heap_metadata(ts->heap)->limit )
+    {
+        xfree(ts->heap);
+        ts->heap = dummy_heap;
+    }
+    else
+        ASSERT(ts->heap == dummy_heap);
+}
+
 static int cpu_callback(
     struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
@@ -628,10 +644,19 @@ static int cpu_callback(
         spin_lock_init(&ts->lock);
         ts->heap = dummy_heap;
         break;
+
     case CPU_UP_CANCELED:
     case CPU_DEAD:
-        migrate_timers_from_cpu(cpu);
+    case CPU_RESUME_FAILED:
+        if ( !park_offline_cpus && system_state != SYS_STATE_suspend )
+            free_percpu_timers(cpu);
         break;
+
+    case CPU_REMOVE:
+        if ( park_offline_cpus )
+            free_percpu_timers(cpu);
+        break;
+
     default:
         break;
     }