Re: [Xen-devel] [PATCH] xen/timers: Fix memory leak with cpu hot unplug

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.03.19 at 17:57, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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 hot unplug and plug 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 hot unplugged and
> replugged.  In the cpu notifier, free the heap after migrating all other
> timers away.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Could I talk you into using online/offline instead of plug/unplug,
as the latter is pretty tied to the physical operation of adding /
removing a CPU to / from a system, whereas the issue here
also surfaces for pure software actions like suspend/resume
or the xen-hptool operations?

> --- a/xen/common/timer.c
> +++ b/xen/common/timer.c
> @@ -631,6 +631,10 @@ static int cpu_callback(
>      case CPU_UP_CANCELED:
>      case CPU_DEAD:
>          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;
>          break;

I think it would be worthwhile to add a comment to clarify that
the updating of per-CPU data here is not entirely pointless,
despite the zeroing done when bringing a CPU back up.

Additionally - is this really the right thing to do uniformly during
CPU_UP_CANCELED / CPU_DEAD? Shouldn't this be done
conditionally upon park_offline_cpus there, and get done for
CPU_REMOVE in the opposite case (like we do elsewhere, in
particular in cpu_percpu_callback() itself)?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel