xen/timers: Fix memory leak with cpu unplug/plug (take 2)

[Xen-devel] [PATCH] xen/timers: Fix memory leak with cpu unplug/plug (take 2)

Posted by Andrew Cooper 6 years, 6 months ago

Previous attempts to fix this leak failed to identify the root cause, and
ultimately failed.  The cause is the CPU_UP_PREPARE case (re)initialising
ts->heap back to dummy_heap, which leaks the previous allocation.

Rearrange the logic to only initialise ts once.  This also avoids the
redundant (but benign, due to ts->inactive always being empty) initialising of
the other ts fields.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Julien Grall <julien.grall@arm.com>
---
 xen/common/timer.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/common/timer.c b/xen/common/timer.c
index f60712a..650e398 100644
--- a/xen/common/timer.c
+++ b/xen/common/timer.c
@@ -657,9 +657,13 @@ static int cpu_callback(
     switch ( action )
     {
     case CPU_UP_PREPARE:
-        INIT_LIST_HEAD(&ts->inactive);
-        spin_lock_init(&ts->lock);
-        ts->heap = dummy_heap;
+        /* Only initialise ts once. */
+        if ( !ts->heap )
+        {
+            INIT_LIST_HEAD(&ts->inactive);
+            spin_lock_init(&ts->lock);
+            ts->heap = dummy_heap;
+        }
         break;
 
     case CPU_UP_CANCELED:
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] xen/timers: Fix memory leak with cpu unplug/plug (take 2)

Posted by Jan Beulich 6 years, 6 months ago

>>> On 23.04.19 at 17:49, <andrew.cooper3@citrix.com> wrote:
> Previous attempts to fix this leak failed to identify the root cause, and
> ultimately failed.  The cause is the CPU_UP_PREPARE case (re)initialising
> ts->heap back to dummy_heap, which leaks the previous allocation.
> 
> Rearrange the logic to only initialise ts once.  This also avoids the
> redundant (but benign, due to ts->inactive always being empty) initialising of
> the other ts fields.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



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