diff options
| -rw-r--r-- | configure.ac | 2 | ||||
| -rw-r--r-- | doc/developer-guide/Developers-Index.md | 1 | ||||
| -rw-r--r-- | doc/developer-guide/identifying-resource-leaks.md | 176 | ||||
| -rw-r--r-- | glusterfs.spec.in | 1 | ||||
| -rw-r--r-- | tests/basic/gfapi/sink.t | 13 | ||||
| -rw-r--r-- | tests/basic/gfapi/sink.vol | 24 | ||||
| -rw-r--r-- | xlators/debug/Makefile.am | 2 | ||||
| -rw-r--r-- | xlators/debug/sink/Makefile.am | 2 | ||||
| -rw-r--r-- | xlators/debug/sink/src/Makefile.am | 14 | ||||
| -rw-r--r-- | xlators/debug/sink/src/sink.c | 79 |
10 files changed, 313 insertions, 1 deletions
diff --git a/configure.ac b/configure.ac index 86a8ec17bf7..af3644f4350 100644 --- a/configure.ac +++ b/configure.ac @@ -106,6 +106,8 @@ AC_CONFIG_FILES([Makefile xlators/performance/nl-cache/Makefile xlators/performance/nl-cache/src/Makefile xlators/debug/Makefile + xlators/debug/sink/Makefile + xlators/debug/sink/src/Makefile xlators/debug/trace/Makefile xlators/debug/trace/src/Makefile xlators/debug/error-gen/Makefile diff --git a/doc/developer-guide/Developers-Index.md b/doc/developer-guide/Developers-Index.md index 9bcbcdc4cbe..4c6346e83d4 100644 --- a/doc/developer-guide/Developers-Index.md +++ b/doc/developer-guide/Developers-Index.md @@ -67,6 +67,7 @@ Testing/Debugging Framework](./Using Gluster Test Framework.md) - Step by step instructions for running the Gluster Test Framework - [Coredump Analysis](./coredump-analysis.md) - Steps to analize coredumps generated by regression machines. +- [Identifying Resource Leaks](./identifying-resource-leaks.md) Release Process --------------- diff --git a/doc/developer-guide/identifying-resource-leaks.md b/doc/developer-guide/identifying-resource-leaks.md new file mode 100644 index 00000000000..851fc4424bc --- /dev/null +++ b/doc/developer-guide/identifying-resource-leaks.md @@ -0,0 +1,176 @@ +# Identifying Resource Leaks + +Like most other pieces of software, GlusterFS is not perfect in how it manages +its resources like memory, threads and the like. Gluster developers try hard to +prevent leaking resources but releasing and unallocating the used structures. +Unfortunately every now and then some resource leaks are unintentionally added. + +This document tries to explain a few helpful tricks to identify resource leaks +so that they can be addressed. + + +## Debug Builds + +There are certain techniques used in GlusterFS that make it difficult to use +tools like Valgrind for memory leak detection. There are some build options +that make it more practical to use Valgrind and other tools. When running +Valgrind, it is important to have GlusterFS builds that contain the +debuginfo/symbols. Some distributions (try to) strip the debuginfo to get +smaller executables. Fedora and RHEL based distributions have sub-packages +called ...-debuginfo that need to be installed for symbol resolving. + + +### Memory Pools + +By using memory pools, there are no allocation/freeing of single structures +needed. This improves performance, but also makes it impossible to track the +allocation and freeing of srtuctures. + +It is possible to disable the use of memory pools, and use standard `malloc()` +and `free()` functions provided by the C library. Valgrind is then able to +track the allocated areas and verify if they have been free'd. In order to +disable memory pools, the Gluster sources needs to be configured with the +`--enable-debug` option: + +```shell +./configure --enable-debug +``` + +When building RPMs, the `.spec` handles the `--with=debug` option too: + +```shell +make dist +rpmbuild -ta --with=debug glusterfs-....tar.gz +``` + +### Dynamically Loaded xlators + +Valgrind tracks the call chain of functions that do memory allocations. The +addresses of the functions are stored and before Valgrind exits the addresses +are resolved into human readable function names and offsets (line numbers in +source files). Because Gluster loads xlators dynamically, and unloads then +before exiting, Valgrind is not able to resolve the function addresses into +symbols anymore. Whenever this happend, Valgrind shows `???` in the output, +like + +``` + ==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) + ... +``` + +These `???` can be prevented by not calling `dlclose()` for unloading the +xlator. This will cause a small leak of the handle that was returned with +`dlopen()`, but for improved debugging this can be acceptible. For this and +other Valgrind features, a `--enable-valgrind` option is available to +`./configure`. When GlusterFS is built with this option, Valgrind will be able +to resolve the symbol names of the functions that do memory allocations inside +xlators. + +```shell +./configure --enable-valgrind +``` + +When building RPMs, the `.spec` handles the `--with=valgrind` option too: + +```shell +make dist +rpmbuild -ta --with=valgrind glusterfs-....tar.gz +``` + +## Running Valgrind against a single xlator + +Debugging a single xlator is not trivial. But there are some tools to make it +easier. The `sink` xlator does not do any memory allocations itself, but +contains just enough functionality to mount a volume with only the `sink` +xlator. There is a little gfapi application under `tests/basic/gfapi/` in the +GlusterFS sources that can be used to run only gfapi and the core GlusterFS +infrastructure with the `sink` xlator. By extending the `.vol` file to load +more xlators, each xlator can be debugged pretty much separately (as long as +the xlators have no dependencies on each other). A basic Valgrind run with the +suitable configure options looks like this: + +```shell +./autogen.sh +./configure --enable-debug --enable-valgrind +make && make install +cd tests/basic/gfapi/ +make gfapi-load-volfile +valgrind ./gfapi-load-volfile sink.vol +``` + +Combined with other very useful options to Valgrind, the following execution +shows many more useful details: + +```shell +valgrind \ + --fullpath-after= --leak-check=full --show-leak-kinds=all \ + ./gfapi-load-volfile sink.vol +``` + +Note that the `--fullpath-after=` option is left empty, this makes Valgrind +print the full path and filename that contains the functions: + +``` +==2450== 80 bytes in 1 blocks are definitely lost in loss record 8 of 60 +==2450== at 0x4C29975: calloc (/builddir/build/BUILD/valgrind-3.11.0/coregrind/m_replacemalloc/vg_replace_malloc.c:711) +==2450== by 0x52C6F73: __gf_calloc (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/mem-pool.c:117) +==2450== by 0x12F10CDA: init (/usr/src/debug/glusterfs-3.11dev/xlators/meta/src/meta.c:231) +==2450== by 0x528EFD5: __xlator_init (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/xlator.c:472) +==2450== by 0x528F105: xlator_init (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/xlator.c:498) +==2450== by 0x52D9D8B: glusterfs_graph_init (/usr/src/debug/glusterfs-3.11dev/libglusterfs/src/graph.c:321) +... +``` + +In the above example, the `init` function in `xlators/meta/src/meta.c` does a +memory allocation on line 231. This memory is never free'd again, and hence +Valgrind logs this call stack. When looking in the code, it seems that the +allocation of `priv` is assigned to the `this->private` member of the +`xlator_t` structure. Because the allocation is done in `init()`, free'ing is +expected to happen in `fini()`. Both functions are shown below, with the +inclusion of the empty `fini()`: + + +``` +226 int +227 init (xlator_t *this) +228 { +229 meta_priv_t *priv = NULL; +230 +231 priv = GF_CALLOC (sizeof(*priv), 1, gf_meta_mt_priv_t); +232 if (!priv) +233 return -1; +234 +235 GF_OPTION_INIT ("meta-dir-name", priv->meta_dir_name, str, out); +236 +237 this->private = priv; +238 out: +239 return 0; +240 } +241 +242 +243 int +244 fini (xlator_t *this) +245 { +246 return 0; +247 } +``` + +In this case, the resource leak can be addressed by adding a single line to the +`fini()` function: + +``` +243 int +244 fini (xlator_t *this) +245 { +246 GF_FREE (this->private); +247 return 0; +248 } +``` + +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. diff --git a/glusterfs.spec.in b/glusterfs.spec.in index 63bbd1eea73..89cc284fcb3 100644 --- a/glusterfs.spec.in +++ b/glusterfs.spec.in @@ -955,6 +955,7 @@ exit 0 %dir %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/error-gen.so %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/io-stats.so +%{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/sink.so %{_libdir}/glusterfs/%{version}%{?prereltag}/xlator/debug/trace.so %if ( ! ( 0%{?rhel} && 0%{?rhel} < 6 ) ) # RHEL-5 based distributions have a too old openssl diff --git a/tests/basic/gfapi/sink.t b/tests/basic/gfapi/sink.t new file mode 100644 index 00000000000..53af2ecf62d --- /dev/null +++ b/tests/basic/gfapi/sink.t @@ -0,0 +1,13 @@ +#!/bin/bash + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc + +cleanup + +TEST build_tester $(dirname ${0})/gfapi-load-volfile.c -lgfapi +TEST ./$(dirname ${0})/gfapi-load-volfile $(dirname $0)/sink.vol + +cleanup_tester $(dirname ${0})/gfapi-load-volfile + +cleanup diff --git a/tests/basic/gfapi/sink.vol b/tests/basic/gfapi/sink.vol new file mode 100644 index 00000000000..d1c92261448 --- /dev/null +++ b/tests/basic/gfapi/sink.vol @@ -0,0 +1,24 @@ +# +# The sink xlator does not do any memory allocations. It only passes the FOPs +# through to the next xlator. +# +# For testing, there is no next xlator needed, we are only interested in the +# resource usage of the Gluster core when gfapi is used. +# +# Note: The sink xlator does not handle any calls. Mounting is possible, but +# any I/O needs additional functionality in the sink xlator. +# +volume sink + type debug/sink + # an option is required, otherwise the graph parsing fails + option an-option-is-required yes +end-volume + +# +# It is possible to test the resource usage of other xlators by adding them in +# the graph before the "sink". +# +#volume mdcache-sink +# type performance/md-cache +# subvolumes sink +#end-volume diff --git a/xlators/debug/Makefile.am b/xlators/debug/Makefile.am index b655554efec..6e476152ddc 100644 --- a/xlators/debug/Makefile.am +++ b/xlators/debug/Makefile.am @@ -1,3 +1,3 @@ -SUBDIRS = trace error-gen io-stats +SUBDIRS = error-gen io-stats sink trace CLEANFILES = diff --git a/xlators/debug/sink/Makefile.am b/xlators/debug/sink/Makefile.am new file mode 100644 index 00000000000..f2689244371 --- /dev/null +++ b/xlators/debug/sink/Makefile.am @@ -0,0 +1,2 @@ +SUBDIRS = src + diff --git a/xlators/debug/sink/src/Makefile.am b/xlators/debug/sink/src/Makefile.am new file mode 100644 index 00000000000..f952c2ce6bc --- /dev/null +++ b/xlators/debug/sink/src/Makefile.am @@ -0,0 +1,14 @@ +xlator_LTLIBRARIES = sink.la +xlatordir = $(libdir)/glusterfs/$(PACKAGE_VERSION)/xlator/debug + +AM_CPPFLAGS = $(GF_CPPFLAGS) -I$(top_srcdir)/libglusterfs/src \ + -I$(top_builddir)/rpc/xdr/src +AM_CFLAGS = -Wall $(GF_CFLAGS) + +sink_la_LDFLAGS = -module $(GF_XLATOR_DEFAULT_LDFLAGS) + +sink_la_SOURCES = sink.c +sink_la_LIBADD = $(top_builddir)/libglusterfs/src/libglusterfs.la + +CLEANFILES = + diff --git a/xlators/debug/sink/src/sink.c b/xlators/debug/sink/src/sink.c new file mode 100644 index 00000000000..dfd5685a969 --- /dev/null +++ b/xlators/debug/sink/src/sink.c @@ -0,0 +1,79 @@ +/* + Copyright (c) 2017 Red Hat, Inc. <http://www.redhat.com> + This file is part of GlusterFS. + + This file is licensed to you under your choice of the GNU Lesser + General Public License, version 3 or any later version (LGPLv3 or + later), or the GNU General Public License, version 2 (GPLv2), in all + cases as published by the Free Software Foundation. +*/ + +#include "xlator.h" +#include "defaults.h" + +int32_t +init (xlator_t *this) +{ + return 0; +} + +void +fini (xlator_t *this) +{ + return; +} + +/* + * notify - when parent sends PARENT_UP, send CHILD_UP event from here + */ +int32_t +notify (xlator_t *this, int32_t event, void *data, ...) +{ + switch (event) { + case GF_EVENT_PARENT_UP: + /* Tell the parent that this xlator is up */ + default_notify (this, GF_EVENT_CHILD_UP, data); + break; + case GF_EVENT_PARENT_DOWN: + /* Tell the parent that this xlator is down */ + default_notify (this, GF_EVENT_CHILD_DOWN, data); + break; + default: + break; + } + + return 0; +} + +/* + * A lookup on "/" is done while mounting or glfs_init() is performed. This + * needs to return a valid directory for the root of the mountpoint. + * + * In case this xlator is used for more advanced debugging, it will need to be + * extended to support different LOOKUPs too. + */ +static int32_t +sink_lookup (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata) +{ + struct iatt stbuf = { 0, }; + struct iatt postparent = { 0, }; + + /* the root of the volume always need to be a directory */ + stbuf.ia_type = IA_IFDIR; + + STACK_UNWIND_STRICT (lookup, frame, 0, 0, loc ? loc->inode : NULL, + &stbuf, xdata, &postparent); + + return 0; +} + +struct xlator_fops fops = { + .lookup = sink_lookup, +}; + +struct xlator_cbks cbks = { +}; + +struct volume_options options[] = { + { .key = {NULL} }, +}; |