From 4e81a5e47aeee6f5d18387b7af161488cf0f30fa Mon Sep 17 00:00:00 2001 From: Dmitry Antipov Date: Mon, 31 Aug 2020 14:17:24 +0300 Subject: build: extend --enable-valgrind to support Memcheck and DRD Extend '-enable-valgrind' to '--enable=valgrind[=memcheck,drd]' to enable Memcheck or DRD Valgrind tool, respectively. Change-Id: I80d13d72ba9756e0cbcdbeb6766b5c98e3e8c002 Signed-off-by: Dmitry Antipov Updates: #1002 --- configure.ac | 28 +++++++++++++++++----- doc/developer-guide/identifying-resource-leaks.md | 24 +++++++++++++++++++ .../distributed-testing/distributed-test-runner.py | 12 ++++++---- libglusterfs/src/ctx.c | 8 +++++-- libglusterfs/src/glusterfs/glusterfs.h | 5 +++- libglusterfs/src/xlator.c | 2 +- xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c | 11 ++++++--- xlators/mgmt/glusterd/src/glusterd-rebalance.c | 18 +++++++++++--- xlators/mgmt/glusterd/src/glusterd-snapd-svc.c | 11 ++++++--- xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c | 19 ++++++++++----- xlators/mgmt/glusterd/src/glusterd-utils.c | 15 ++++++++---- xlators/mgmt/glusterd/src/glusterd.c | 28 ++++++++++++++++------ 12 files changed, 140 insertions(+), 41 deletions(-) diff --git a/configure.ac b/configure.ac index cf22267aada..a13a3dbba82 100644 --- a/configure.ac +++ b/configure.ac @@ -409,12 +409,27 @@ esac # --enable-valgrind prevents calling dlclose(), this leaks memory AC_ARG_ENABLE([valgrind], - AC_HELP_STRING([--enable-valgrind], - [Enable valgrind for resource leak debugging.])) -if test "x$enable_valgrind" = "xyes"; then - AC_DEFINE(RUN_WITH_VALGRIND, 1, [define if all processes should run under valgrind]) -fi - + AC_HELP_STRING([--enable-valgrind@<:@=memcheck,drd@:>@], + [Enable valgrind for resource leak (memcheck, which is + the default) or thread synchronization (drd) debugging.])) +case x$enable_valgrind in + xmemcheck|xyes) + AC_DEFINE(RUN_WITH_MEMCHECK, 1, + [Define if all processes should run under 'valgrind --tool=memcheck'.]) + VALGRIND_TOOL=memcheck + ;; + xdrd) + AC_DEFINE(RUN_WITH_DRD, 1, + [Define if all processes should run under 'valgrind --tool=drd'.]) + VALGRIND_TOOL=drd + ;; + x|xno) + VALGRIND_TOOL=no + ;; + *) + AC_MSG_ERROR([Please specify --enable-valgrind@<:@=memcheck,drd@:>@]) + ;; +esac AC_ARG_WITH([previous-options], [AS_HELP_STRING([--with-previous-options], @@ -1670,6 +1685,7 @@ echo "readline : $BUILD_READLINE" echo "georeplication : $BUILD_SYNCDAEMON" echo "Linux-AIO : $BUILD_LIBAIO" echo "Enable Debug : $BUILD_DEBUG" +echo "Run with Valgrind : $VALGRIND_TOOL" echo "Sanitizer enabled : $SANITIZER" echo "Use syslog : $USE_SYSLOG" echo "XML output : $BUILD_XML_OUTPUT" diff --git a/doc/developer-guide/identifying-resource-leaks.md b/doc/developer-guide/identifying-resource-leaks.md index 851fc4424bc..950cae79b0a 100644 --- a/doc/developer-guide/identifying-resource-leaks.md +++ b/doc/developer-guide/identifying-resource-leaks.md @@ -174,3 +174,27 @@ In this case, the resource leak can be addressed by adding a single line to the Running the same Valgrind command and comparing the output will show that the memory leak in `xlators/meta/src/meta.c:init` is not reported anymore. + +### Running DRD, the Valgrind thread error detector + +When configuring GlusterFS with: + +```shell +./configure --enable-valgrind +``` + +the default Valgrind tool (Memcheck) is enabled. But it's also possble to select +one of Memcheck or DRD by using: + +```shell +./configure --enable-valgrind=memcheck +``` + +or: + +```shell +./configure --enable-valgrind=drd +``` + +respectively. When using DRD, it's recommended to consult +https://valgrind.org/docs/manual/drd-manual.html before running. diff --git a/extras/distributed-testing/distributed-test-runner.py b/extras/distributed-testing/distributed-test-runner.py index 7bfb6c9652a..5a07e2feab1 100755 --- a/extras/distributed-testing/distributed-test-runner.py +++ b/extras/distributed-testing/distributed-test-runner.py @@ -383,14 +383,17 @@ class Handlers: return self.shell.call("make install") == 0 @synchronized - def prove(self, id, test, timeout, valgrind=False, asan_noleaks=True): + def prove(self, id, test, timeout, valgrind="no", asan_noleaks=True): assert id == self.client_id self.shell.cd(self.gluster_root) env = "DEBUG=1 " - if valgrind: + if valgrind == "memcheck" or valgrind == "yes": cmd = "valgrind" cmd += " --tool=memcheck --leak-check=full --track-origins=yes" cmd += " --show-leak-kinds=all -v prove -v" + elif valgrind == "drd": + cmd = "valgrind" + cmd += " --tool=drd -v prove -v" elif asan_noleaks: cmd = "prove -v" env += "ASAN_OPTIONS=detect_leaks=0 " @@ -827,8 +830,9 @@ parser.add_argument("--port", help="server port to listen", type=int, default=DEFAULT_PORT) # test role parser.add_argument("--tester", help="start tester", action="store_true") -parser.add_argument("--valgrind", help="run tests under valgrind", - action="store_true") +parser.add_argument("--valgrind[=memcheck,drd]", + help="run tests with valgrind tool 'memcheck' or 'drd'", + default="no") parser.add_argument("--asan", help="test with asan enabled", action="store_true") parser.add_argument("--asan-noleaks", help="test with asan but no mem leaks", diff --git a/libglusterfs/src/ctx.c b/libglusterfs/src/ctx.c index 4a001c29209..3d890b04ec9 100644 --- a/libglusterfs/src/ctx.c +++ b/libglusterfs/src/ctx.c @@ -37,8 +37,12 @@ glusterfs_ctx_new() ctx->log.loglevel = DEFAULT_LOG_LEVEL; -#ifdef RUN_WITH_VALGRIND - ctx->cmd_args.valgrind = _gf_true; +#if defined(RUN_WITH_MEMCHECK) + ctx->cmd_args.vgtool = _gf_memcheck; +#elif defined(RUN_WITH_DRD) + ctx->cmd_args.vgtool = _gf_drd; +#else + ctx->cmd_args.vgtool = _gf_none; #endif /* lock is never destroyed! */ diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h index fc69ad0b749..e6425618b7f 100644 --- a/libglusterfs/src/glusterfs/glusterfs.h +++ b/libglusterfs/src/glusterfs/glusterfs.h @@ -466,6 +466,8 @@ typedef struct _server_cmdline server_cmdline_t; #define GF_OPTION_DISABLE _gf_false #define GF_OPTION_DEFERRED 2 +typedef enum { _gf_none, _gf_memcheck, _gf_drd } gf_valgrind_tool; + struct _cmd_args { /* basic options */ char *volfile_server; @@ -558,7 +560,8 @@ struct _cmd_args { /* Run this process with valgrind? Might want to prevent calling * functions that prevent valgrind from working correctly, like * dlclose(). */ - int valgrind; + gf_valgrind_tool vgtool; + int localtime_logging; /* For the subdir mount */ diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index 539da7f4716..a54aadd76c7 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -821,7 +821,7 @@ xlator_members_free(xlator_t *xl) GF_FREE(xl->name); GF_FREE(xl->type); - if (!(xl->ctx && xl->ctx->cmd_args.valgrind) && xl->dlhandle) + if (!(xl->ctx && xl->ctx->cmd_args.vgtool != _gf_none) && xl->dlhandle) dlclose(xl->dlhandle); if (xl->options) dict_unref(xl->options); diff --git a/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c b/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c index b01fd4da24b..a0bfea41f0f 100644 --- a/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c +++ b/xlators/mgmt/glusterd/src/glusterd-gfproxyd-svc.c @@ -310,7 +310,7 @@ glusterd_gfproxydsvc_start(glusterd_svc_t *svc, int flags) } runinit(&runner); - if (this->ctx->cmd_args.valgrind) { + if (this->ctx->cmd_args.vgtool != _gf_none) { len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s", svc->proc.logdir, svc->proc.logfile); if ((len < 0) || (len >= PATH_MAX)) { @@ -318,8 +318,13 @@ glusterd_gfproxydsvc_start(glusterd_svc_t *svc, int flags) goto out; } - runner_add_args(&runner, "valgrind", "--leak-check=full", - "--trace-children=yes", "--track-origins=yes", NULL); + if (this->ctx->cmd_args.vgtool == _gf_memcheck) + runner_add_args(&runner, "valgrind", "--leak-check=full", + "--trace-children=yes", "--track-origins=yes", + NULL); + else + runner_add_args(&runner, "valgrind", "--tool=drd", NULL); + runner_argprintf(&runner, "--log-file=%s", valgrind_logfile); } diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c index 2afd0fe1b74..458bf168ede 100644 --- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c +++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c @@ -219,6 +219,9 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr, char valgrind_logfile[PATH_MAX] = { 0, }; + char msg[1024] = { + 0, + }; char *volfileserver = NULL; char *localtime_logging = NULL; @@ -270,12 +273,17 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr, "rebalance"); runinit(&runner); - if (this->ctx->cmd_args.valgrind) { + if (this->ctx->cmd_args.vgtool != _gf_none) { snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s-rebalance.log", priv->logdir, volinfo->volname); - runner_add_args(&runner, "valgrind", "--leak-check=full", - "--trace-children=yes", "--track-origins=yes", NULL); + if (this->ctx->cmd_args.vgtool == _gf_memcheck) + runner_add_args(&runner, "valgrind", "--leak-check=full", + "--trace-children=yes", "--track-origins=yes", + NULL); + else + runner_add_args(&runner, "valgrind", "--tool=drd", NULL); + runner_argprintf(&runner, "--log-file=%s", valgrind_logfile); } @@ -316,6 +324,10 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr, runner_add_arg(&runner, "--localtime-logging"); } + snprintf(msg, sizeof(msg), "Starting the rebalance service for volume %s", + volinfo->volname); + runner_log(&runner, this->name, GF_LOG_DEBUG, msg); + ret = runner_run_nowait(&runner); if (ret) { gf_msg_debug("glusterd", 0, "rebalance command failed"); diff --git a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c index 1f3f4909cbb..d75f249b29e 100644 --- a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c +++ b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c @@ -304,7 +304,7 @@ glusterd_snapdsvc_start(glusterd_svc_t *svc, int flags) } runinit(&runner); - if (this->ctx->cmd_args.valgrind) { + if (this->ctx->cmd_args.vgtool != _gf_none) { len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-snapd.log", svc->proc.logdir); if ((len < 0) || (len >= PATH_MAX)) { @@ -313,8 +313,13 @@ glusterd_snapdsvc_start(glusterd_svc_t *svc, int flags) goto out; } - runner_add_args(&runner, "valgrind", "--leak-check=full", - "--trace-children=yes", "--track-origins=yes", NULL); + if (this->ctx->cmd_args.vgtool == _gf_memcheck) + runner_add_args(&runner, "valgrind", "--leak-check=full", + "--trace-children=yes", "--track-origins=yes", + NULL); + else + runner_add_args(&runner, "valgrind", "--tool=drd", NULL); + runner_argprintf(&runner, "--log-file=%s", valgrind_logfile); } diff --git a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c index 99119d69e45..18b3fb13630 100644 --- a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c +++ b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c @@ -162,6 +162,9 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline) char *localtime_logging = NULL; char *log_level = NULL; char daemon_log_level[30] = {0}; + char msg[1024] = { + 0, + }; int32_t len = 0; this = THIS; @@ -187,7 +190,7 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline) runinit(&runner); - if (this->ctx->cmd_args.valgrind) { + if (this->ctx->cmd_args.vgtool != _gf_none) { len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s.log", svc->proc.logdir, svc->name); if ((len < 0) || (len >= PATH_MAX)) { @@ -195,9 +198,13 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline) goto unlock; } - runner_add_args(&runner, "valgrind", "--leak-check=full", - "--trace-children=yes", "--track-origins=yes", - NULL); + if (this->ctx->cmd_args.vgtool == _gf_memcheck) + runner_add_args(&runner, "valgrind", "--leak-check=full", + "--trace-children=yes", "--track-origins=yes", + NULL); + else + runner_add_args(&runner, "valgrind", "--tool=drd", NULL); + runner_argprintf(&runner, "--log-file=%s", valgrind_logfile); } @@ -226,8 +233,8 @@ glusterd_svc_start(glusterd_svc_t *svc, int flags, dict_t *cmdline) if (cmdline) dict_foreach(cmdline, svc_add_args, (void *)&runner); - gf_msg(this->name, GF_LOG_INFO, 0, GD_MSG_SVC_START_SUCCESS, - "Starting %s service", svc->name); + snprintf(msg, sizeof(msg), "Starting %s service", svc->name); + runner_log(&runner, this->name, GF_LOG_DEBUG, msg); if (flags == PROC_START_NO_WAIT) { ret = runner_run_nowait(&runner); diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 04790bbc75c..d7446a099a2 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -2076,8 +2076,8 @@ glusterd_volume_start_glusterfs(glusterd_volinfo_t *volinfo, retry: runinit(&runner); - if (this->ctx->cmd_args.valgrind) { - /* Run bricks with valgrind */ + if (this->ctx->cmd_args.vgtool != _gf_none) { + /* Run bricks with valgrind. */ if (volinfo->logdir) { len = snprintf(valgrind_logfile, PATH_MAX, "%s/valgrind-%s-%s.log", volinfo->logdir, volinfo->volname, exp_path); @@ -2091,8 +2091,13 @@ retry: goto out; } - runner_add_args(&runner, "valgrind", "--leak-check=full", - "--trace-children=yes", "--track-origins=yes", NULL); + if (this->ctx->cmd_args.vgtool == _gf_memcheck) + runner_add_args(&runner, "valgrind", "--leak-check=full", + "--trace-children=yes", "--track-origins=yes", + NULL); + else + runner_add_args(&runner, "valgrind", "--tool=drd", NULL); + runner_argprintf(&runner, "--log-file=%s", valgrind_logfile); } @@ -2205,7 +2210,7 @@ retry: if (is_brick_mx_enabled()) runner_add_arg(&runner, "--brick-mux"); - runner_log(&runner, "", 0, "Starting GlusterFS"); + runner_log(&runner, "", GF_LOG_DEBUG, "Starting GlusterFS"); brickinfo->port = port; brickinfo->rdma_port = rdma_port; diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c index 91c5f9ec5e3..7a86c2997b1 100644 --- a/xlators/mgmt/glusterd/src/glusterd.c +++ b/xlators/mgmt/glusterd/src/glusterd.c @@ -1423,7 +1423,7 @@ init(xlator_t *this) char *mountbroker_root = NULL; int i = 0; int total_transport = 0; - gf_boolean_t valgrind = _gf_false; + gf_valgrind_tool vgtool; char *valgrind_str = NULL; char *transport_type = NULL; char var_run_dir[PATH_MAX] = { @@ -1436,6 +1436,14 @@ init(xlator_t *this) int32_t len = 0; int op_version = 0; +#if defined(RUN_WITH_MEMCHECK) + vgtool = _gf_memcheck; +#elif defined(RUN_WITH_DRD) + vgtool = _gf_drd; +#else + vgtool = _gf_none; +#endif + #ifndef GF_DARWIN_HOST_OS { struct rlimit lim; @@ -1925,18 +1933,24 @@ init(xlator_t *this) } /* Set option to run bricks on valgrind if enabled in glusterd.vol */ - this->ctx->cmd_args.valgrind = valgrind; + this->ctx->cmd_args.vgtool = vgtool; ret = dict_get_str(this->options, "run-with-valgrind", &valgrind_str); if (ret < 0) { gf_msg_debug(this->name, 0, "cannot get run-with-valgrind value"); } if (valgrind_str) { - if (gf_string2boolean(valgrind_str, &valgrind)) { + gf_boolean_t vg = _gf_false; + + if (!strcmp(valgrind_str, "memcheck")) + this->ctx->cmd_args.vgtool = _gf_memcheck; + else if (!strcmp(valgrind_str, "drd")) + this->ctx->cmd_args.vgtool = _gf_drd; + else if (!gf_string2boolean(valgrind_str, &vg)) + this->ctx->cmd_args.vgtool = (vg ? _gf_memcheck : _gf_none); + else gf_msg(this->name, GF_LOG_WARNING, EINVAL, GD_MSG_INVALID_ENTRY, - "run-with-valgrind value not a boolean string"); - } else { - this->ctx->cmd_args.valgrind = valgrind; - } + "run-with-valgrind is neither boolean" + " nor one of 'memcheck' or 'drd'"); } /* Store ping-timeout in conf */ -- cgit