pcp
[Top] [All Lists]

Re: Memory Leak in pmdalinx

To: Mahmoud Hanafi <mahmoud.hanafi@xxxxxxxx>
Subject: Re: Memory Leak in pmdalinx
From: fche@xxxxxxxxxx (Frank Ch. Eigler)
Date: Fri, 13 Dec 2013 20:45:04 -0500
Cc: <pcp@xxxxxxxxxxx>
Delivered-to: pcp@xxxxxxxxxxx
In-reply-to: <52AB4AD5.6090908@xxxxxxxx> (Mahmoud Hanafi's message of "Fri, 13 Dec 2013 09:58:45 -0800")
References: <52AB4AD5.6090908@xxxxxxxx>
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)
Hi -

mahmoud.hanafi wrote:

> Collecting kernel.all.interrupts.errors triggers 2 memory leak in
> pmdalinux. (see valgrid output)

Good catch!  I believe the following patch fixes it.  (It was found by
assistance of the following systemtap script, tracing each line of the
allocation-related functions, and noticing how $lines_count was
changing even at the steady state, and thus the _extend function was
being done over & over.

probe process("/var/lib/pcp/pmdas/linux/pmda_linux.so").
      statement("ext*interrupt*@interrupts.c:*")
{
        // The tokenize() stuff clips out only the function@file:line
        // substring of pp()
        tokenize(pp(),"\""); tokenize("","\""); tokenize("","\"");
        println(tokenize("","\""),
                " ", $$vars,                   // pretty-print all local vars
                " lines_count=",$lines_count$) // and the lines_count global
}' -c '/usr/libexec/pcp/bin/pmcd -f'



commit 6f386c30a83fabfe0e568741ee4a2a6dd1804f58 (HEAD, origin/fche/dev, 
fche/dev)
Author: Frank Ch. Eigler <fche@xxxxxxxxxx>
Date:   Fri Dec 13 20:36:01 2013 -0500

    linux pmda: plug memory leak in /proc/interrupts parsing
    
    Previous code in src/pmdas/linux/interrupts.c accidentally forgot that
    it had already parsed the structure of /proc/interrupts into the
    interrupt_lines/interrupt_other realloc variables by resetting the
    lines_count/other_count variables every refresh.  That caused an
    endless re-allocation growth.  Hey, teacher, leave those _counts alone!
    
    Reported-By: Mahmoud Hanafi <mahmoud.hanafi@xxxxxxxx>

diff --git a/src/pmdas/linux/interrupts.c b/src/pmdas/linux/interrupts.c
index 9f7409baeedd..4f5a72ed547d 100644
--- a/src/pmdas/linux/interrupts.c
+++ b/src/pmdas/linux/interrupts.c
@@ -253,13 +253,13 @@ refresh_interrupt_values(void)
     }
 
     /* next we parse each interrupt line row (starting with a digit) */
-    i = lines_count = 0;
+    i = 0;
     while (fgets(buf, sizeof(buf), fp))
        if (!extract_interrupt_lines(buf, ncolumns, i++))
            break;
 
     /* parse other per-CPU interrupt counter rows (starts non-digit) */
-    i = other_count = 0;
+    i = 0;
     while (fgets(buf, sizeof(buf), fp) != NULL) {
        if (extract_interrupt_errors(buf))
            continue;

<Prev in Thread] Current Thread [Next in Thread>