From ef36ac0d1b72ab2c07ed6e0a3116b7265c3c0164 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Mon, 27 Feb 2017 22:37:00 -0800 Subject: xlator: do not call dlclose() when debugging Valgrind can not show the symbols if a .so after calling dlclose(). The unhelpful ??? in the output gets resolved properly with this change: ==25170== 344 bytes in 1 blocks are definitely lost in loss record 233 of 324 ==25170== at 0x4C29975: calloc (vg_replace_malloc.c:711) ==25170== by 0x52C7C0B: __gf_calloc (mem-pool.c:117) ==25170== by 0x12B0638A: ??? ==25170== by 0x528FCE6: __xlator_init (xlator.c:472) ==25170== by 0x528FE16: xlator_init (xlator.c:498) ==25170== by 0x52DA8D6: glusterfs_graph_init (graph.c:321) ==25170== by 0x52DB587: glusterfs_graph_activate (graph.c:695) ==25170== by 0x5046407: glfs_process_volfp (glfs-mgmt.c:79) ==25170== by 0x5043B9E: glfs_volumes_init (glfs.c:281) ==25170== by 0x5044FEC: glfs_init_common (glfs.c:986) ==25170== by 0x50451A7: glfs_init@@GFAPI_3.4.0 (glfs.c:1031) By not calling dlclose(), the dynamically loaded .so is still available upon program exit, and Valgrind is able to resolve the symbols. This will add an additional leak, so dlclose() is called for normal builds, but skipped when configuring with "./configure --enable-valgrind" or passing the "run-with-valgrind" xlator option. URL: http://valgrind.org/docs/manual/faq.html#faq.unhelpful Change-Id: I2044e21b1b8fcce32ad1a817fdd795218f967731 BUG: 1425623 Signed-off-by: Niels de Vos Reviewed-on: https://review.gluster.org/16809 Smoke: Gluster Build System NetBSD-regression: NetBSD Build System CentOS-regression: Gluster Build System Reviewed-by: Samikshan Bairagya Reviewed-by: Kaleb KEITHLEY --- configure.ac | 9 +++++++++ glusterfs.spec.in | 8 ++++++++ libglusterfs/src/ctx.c | 4 ++++ libglusterfs/src/glusterfs.h | 5 +++++ libglusterfs/src/xlator.c | 2 +- xlators/mgmt/glusterd/src/glusterd-rebalance.c | 13 +++++++++---- xlators/mgmt/glusterd/src/glusterd-snapd-svc.c | 2 +- xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c | 2 +- xlators/mgmt/glusterd/src/glusterd-tierd-svc.c | 2 +- xlators/mgmt/glusterd/src/glusterd-utils.c | 2 +- xlators/mgmt/glusterd/src/glusterd.c | 7 +++++-- xlators/mgmt/glusterd/src/glusterd.h | 1 - 12 files changed, 45 insertions(+), 12 deletions(-) diff --git a/configure.ac b/configure.ac index c2d03e7ee6..4bc5bf1d87 100644 --- a/configure.ac +++ b/configure.ac @@ -321,6 +321,15 @@ case $host_os in ;; 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_ARG_WITH([previous-options], [AS_HELP_STRING([--with-previous-options], [read config.status for configure options]) diff --git a/glusterfs.spec.in b/glusterfs.spec.in index 6225d12e63..7a50a8e5ec 100644 --- a/glusterfs.spec.in +++ b/glusterfs.spec.in @@ -13,6 +13,10 @@ # rpmbuild -ta @PACKAGE_NAME@-@PACKAGE_VERSION@.tar.gz --with debug %{?_with_debug:%global _with_debug --enable-debug} +# if you wish to compile an rpm to run all processes under valgrind... +# rpmbuild -ta @PACKAGE_NAME@-@PACKAGE_VERSION@.tar.gz --with valgrind +%{?_with_valgrind:%global _with_valgrind --enable-valgrind} + # if you wish to compile an rpm with cmocka unit testing... # rpmbuild -ta @PACKAGE_NAME@-@PACKAGE_VERSION@.tar.gz --with cmocka %{?_with_cmocka:%global _with_cmocka --enable-cmocka} @@ -550,6 +554,9 @@ Obsoletes: %{name}-geo-replication = %{version}-%{release} Requires: python-argparse %endif Requires: pyxattr +%if (0%{?_with_valgrind:1}) +Requires: valgrind +%endif %description server GlusterFS is a distributed file-system capable of scaling to several @@ -618,6 +625,7 @@ export CFLAGS %configure \ %{?_with_cmocka} \ %{?_with_debug} \ + %{?_with_valgrind} \ %{?_with_tmpfilesdir} \ %{?_without_bd} \ %{?_without_epoll} \ diff --git a/libglusterfs/src/ctx.c b/libglusterfs/src/ctx.c index b009e6270a..d3fb39822d 100644 --- a/libglusterfs/src/ctx.c +++ b/libglusterfs/src/ctx.c @@ -37,6 +37,10 @@ glusterfs_ctx_new () ctx->log.loglevel = DEFAULT_LOG_LEVEL; +#ifdef RUN_WITH_VALGRIND + ctx->cmd_args.valgrind = _gf_true; +#endif + /* lock is never destroyed! */ ret = LOCK_INIT (&ctx->lock); if (ret) { diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h index 4f1f27b585..ce0dde22d4 100644 --- a/libglusterfs/src/glusterfs.h +++ b/libglusterfs/src/glusterfs.h @@ -401,6 +401,11 @@ struct _cmd_args { #ifdef GF_LINUX_HOST_OS char *oom_score_adj; #endif + + /* Run this process with valgrind? Might want to prevent calling + * functions that prevent valgrind from working correctly, like + * dlclose(). */ + int valgrind; }; typedef struct _cmd_args cmd_args_t; diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index 408012e784..b1ed094e0a 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -673,7 +673,7 @@ xlator_members_free (xlator_t *xl) GF_FREE (xl->name); GF_FREE (xl->type); - if (xl->dlhandle) + if (!(xl->ctx && xl->ctx->cmd_args.valgrind) && xl->dlhandle) dlclose (xl->dlhandle); if (xl->options) dict_unref (xl->options); diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c index 9ff2541c63..60d39cbd77 100644 --- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c +++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c @@ -183,6 +183,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr, size_t len, int cmd, defrag_cbk_fn_t cbk, glusterd_op_t op) { + xlator_t *this = NULL; int ret = -1; glusterd_defrag_info_t *defrag = NULL; runner_t runner = {0,}; @@ -195,7 +196,11 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr, char valgrind_logfile[PATH_MAX] = {0,}; char *volfileserver = NULL; - priv = THIS->private; + this = THIS; + GF_VALIDATE_OR_GOTO ("glusterd", this, out); + + priv = this->private; + GF_VALIDATE_OR_GOTO ("glusterd", priv, out); GF_ASSERT (volinfo); GF_ASSERT (op_errstr); @@ -228,7 +233,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr, GLUSTERD_GET_DEFRAG_DIR (defrag_path, volinfo, priv); ret = mkdir_p (defrag_path, 0777, _gf_true); if (ret) { - gf_msg (THIS->name, GF_LOG_ERROR, errno, + gf_msg (this->name, GF_LOG_ERROR, errno, GD_MSG_CREATE_DIR_FAILED, "Failed to create " "directory %s", defrag_path); goto out; @@ -241,7 +246,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr, (cmd == GF_DEFRAG_CMD_START_TIER ? "tier":"rebalance")); runinit (&runner); - if (priv->valgrind) { + if (this->ctx->cmd_args.valgrind) { snprintf (valgrind_logfile, PATH_MAX, "%s/valgrind-%s-rebalance.log", DEFAULT_LOG_FILE_DIRECTORY, @@ -255,7 +260,7 @@ glusterd_handle_defrag_start (glusterd_volinfo_t *volinfo, char *op_errstr, snprintf (volname, sizeof(volname), "rebalance/%s", volinfo->volname); - if (dict_get_str (THIS->options, "transport.socket.bind-address", + if (dict_get_str (this->options, "transport.socket.bind-address", &volfileserver) == 0) { /*In the case of running multiple glusterds on a single machine, *we should ensure that log file and unix socket file shouls be diff --git a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c index b57542a85a..59d8fbdfa6 100644 --- a/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c +++ b/xlators/mgmt/glusterd/src/glusterd-snapd-svc.c @@ -280,7 +280,7 @@ glusterd_snapdsvc_start (glusterd_svc_t *svc, int flags) } runinit (&runner); - if (priv->valgrind) { + if (this->ctx->cmd_args.valgrind) { snprintf (valgrind_logfile, PATH_MAX, "%s/valgrind-snapd.log", svc->proc.logdir); diff --git a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c index d6e57a432c..4d60c85617 100644 --- a/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c +++ b/xlators/mgmt/glusterd/src/glusterd-svc-mgmt.c @@ -171,7 +171,7 @@ glusterd_svc_start (glusterd_svc_t *svc, int flags, dict_t *cmdline) runinit (&runner); - if (priv->valgrind) { + if (this->ctx->cmd_args.valgrind) { snprintf (valgrind_logfile, PATH_MAX, "%s/valgrind-%s.log", svc->proc.logfile, svc->name); diff --git a/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c b/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c index bfc879a343..638d494152 100644 --- a/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c +++ b/xlators/mgmt/glusterd/src/glusterd-tierd-svc.c @@ -304,7 +304,7 @@ glusterd_tierdsvc_start (glusterd_svc_t *svc, int flags) } runinit (&runner); - if (priv->valgrind) { + if (this->ctx->cmd_args.valgrind) { snprintf (valgrind_logfile, PATH_MAX, "%s/valgrind-tierd.log", svc->proc.logdir); diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 89bfa3d735..2a9407f3bb 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -1894,7 +1894,7 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t *volinfo, retry: runinit (&runner); - if (priv->valgrind) { + if (this->ctx->cmd_args.valgrind) { /* Run bricks with valgrind */ if (volinfo->logdir) { snprintf (valgrind_logfile, PATH_MAX, diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c index d26e959d3a..0f6d888b96 100644 --- a/xlators/mgmt/glusterd/src/glusterd.c +++ b/xlators/mgmt/glusterd/src/glusterd.c @@ -1424,6 +1424,7 @@ init (xlator_t *this) char *mountbroker_root = NULL; int i = 0; int total_transport = 0; + gf_boolean_t valgrind = _gf_false; char *valgrind_str = NULL; char *transport_type = NULL; char var_run_dir[PATH_MAX] = {0,}; @@ -1781,17 +1782,19 @@ init (xlator_t *this) } /* Set option to run bricks on valgrind if enabled in glusterd.vol */ - conf->valgrind = _gf_false; + this->ctx->cmd_args.valgrind = valgrind; 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, &(conf->valgrind))) { + if (gf_string2boolean (valgrind_str, &valgrind)) { 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; } } diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h index a773a2b622..55879aa22d 100644 --- a/xlators/mgmt/glusterd/src/glusterd.h +++ b/xlators/mgmt/glusterd/src/glusterd.h @@ -171,7 +171,6 @@ typedef struct { * transaction ids */ struct cds_list_head mount_specs; - gf_boolean_t valgrind; pthread_t brick_thread; void *hooks_priv; -- cgit