Re: [Xen-devel] [PATCH] xenstore: add memory allocation debugging capability

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jul 15, 2016 at 07:28:26AM +0200, Juergen Gross wrote:
> Add support for debugging memory allocation statistics to xenstored.
> Specifying "-M <file>" on the command line will enable the feature.
> Whenever xenstored receives SIGUSR1 it will dump out a full talloc
> report to <file>. This helps finding e.g. memory leaks in xenstored.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> To be applied on top of my "xenstore: fix memory leak of xenstored"
> series. In fact this patch was used to find the problem the series
> fixed and I used it to verify the patches are working.

This patch doesn't seem to apply cleanly anymore.

The hunk rejected is:

diff a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c 
(rejected hunks)
@@ -1860,9 +1872,10 @@ int main(int argc, char *argv[])
        bool outputpid = false;
        bool no_domain_init = false;
        const char *pidfile = NULL;
+       const char *memfile = NULL;
        int timeout;
 
-       while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:",
        options,
+       while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:M:",
options,
                                  NULL)) != -1) {
                switch (opt) {
                case 'D':
@@ -1942,6 +1958,11 @@ int main(int argc, char *argv[])
        /* Don't kill us with SIGPIPE. */
        signal(SIGPIPE, SIG_IGN);
 
+       if (memfile) {
+               talloc_enable_null_tracking();
+               signal(SIGUSR1, do_talloc_report);
+       }
+
        init_sockets(&sock, &ro_sock);
 
        init_pipe(reopen_log_pipe);


Doesn't seem to be immediately obvious to me why this is rejected.

Can you please rebase this patch and resend?

Wei.


> ---
>  tools/xenstore/xenstored_core.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 8cb12c7..ab737eb 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -71,6 +71,7 @@ static int reopen_log_pipe[2];
>  static int reopen_log_pipe0_pollfd_idx = -1;
>  static char *tracefile = NULL;
>  static TDB_CONTEXT *tdb_ctx = NULL;
> +static bool trigger_talloc_report = false;
>  
>  static void corrupt(struct connection *conn, const char *fmt, ...);
>  static void check_store(void);
> @@ -1743,6 +1744,10 @@ static void init_sockets(int **psock, int **pro_sock)
>       static int minus_one = -1;
>       *psock = *pro_sock = &minus_one;
>  }
> +
> +static void do_talloc_report(int sig)
> +{
> +}
>  #else
>  static int destroy_fd(void *_fd)
>  {
> @@ -1798,6 +1803,11 @@ static void init_sockets(int **psock, int **pro_sock)
>  
>  
>  }
> +
> +static void do_talloc_report(int sig)
> +{
> +     trigger_talloc_report = true;
> +}
>  #endif
>  
>  static void usage(void)
> @@ -1823,6 +1833,7 @@ static void usage(void)
>  "                          the store is corrupted (debug only),\n"
>  "  -I, --internal-db       store database in memory, not on disk\n"
>  "  -L, --preserve-local    to request that /local is preserved on 
> start-up,\n"
> +"  -M, --memory-debug <file>  support memory debugging to file,\n"
>  "  -V, --verbose           to request verbose execution.\n");
>  }
>  
> @@ -1845,6 +1856,7 @@ static struct option options[] = {
>       { "internal-db", 0, NULL, 'I' },
>       { "verbose", 0, NULL, 'V' },
>       { "watch-nb", 1, NULL, 'W' },
> +     { "memory-debug", 1, NULL, 'M' },
>       { NULL, 0, NULL, 0 } };
>  
>  extern void dump_conn(struct connection *conn); 
> @@ -1860,9 +1872,10 @@ int main(int argc, char *argv[])
>       bool outputpid = false;
>       bool no_domain_init = false;
>       const char *pidfile = NULL;
> +     const char *memfile = NULL;
>       int timeout;
>  
> -     while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
> +     while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:M:", options,
>                                 NULL)) != -1) {
>               switch (opt) {
>               case 'D':
> @@ -1916,6 +1929,9 @@ int main(int argc, char *argv[])
>               case 'p':
>                       priv_domid = strtol(optarg, NULL, 10);
>                       break;
> +             case 'M':
> +                     memfile = optarg;
> +                     break;
>               }
>       }
>       if (optind != argc)
> @@ -1942,6 +1958,11 @@ int main(int argc, char *argv[])
>       /* Don't kill us with SIGPIPE. */
>       signal(SIGPIPE, SIG_IGN);
>  
> +     if (memfile) {
> +             talloc_enable_null_tracking();
> +             signal(SIGUSR1, do_talloc_report);
> +     }
> +
>       init_sockets(&sock, &ro_sock);
>  
>       init_pipe(reopen_log_pipe);
> @@ -1978,6 +1999,17 @@ int main(int argc, char *argv[])
>       for (;;) {
>               struct connection *conn, *next;
>  
> +             if (trigger_talloc_report) {
> +                     FILE *out;
> +
> +                     trigger_talloc_report = false;
> +                     out = fopen(memfile, "a");
> +                     if (out) {
> +                             talloc_report_full(NULL, out);
> +                             fclose(out);
> +                     }
> +             }
> +
>               if (poll(fds, nr_fds, timeout) < 0) {
>                       if (errno == EINTR)
>                               continue;
> -- 
> 2.6.6
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel