Re: [PATCH] xenstore: add memory allocation debugging capability

[Wei Liu <wei.liu2@citrix.com>]

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@suse.com>
> ---
> 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@lists.xen.org
https://lists.xen.org/xen-devel