From 38e2c8fa9c4a2a0613609b8b88a355670a2f9770 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Thu, 17 Sep 2015 10:05:56 -0700 Subject: [PATCH 1/9] Fix ixallocx_prof_sample(). Fix ixallocx_prof_sample() to never modify nor create sampled small allocations. xallocx() is in general incapable of moving small allocations, so this fix removes buggy code without loss of generality. --- ChangeLog | 7 +++++++ src/jemalloc.c | 21 ++++----------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4498683e..619c522b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,13 @@ brevity. Much more detail can be found in the git revision history: https://github.com/jemalloc/jemalloc +* 4.0.2 (XXX) + + Bug fixes: + - Fix ixallocx_prof_sample() to never modify nor create sampled small + allocations. xallocx() is in general incapable of moving small allocations, + so this fix removes buggy code without loss of generality. + * 4.0.1 (September 15, 2015) This is a bugfix release that is somewhat high risk due to the amount of diff --git a/src/jemalloc.c b/src/jemalloc.c index ab7cf024..ad904eb5 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -2251,26 +2251,13 @@ ixallocx_helper(void *ptr, size_t old_usize, size_t size, size_t extra, static size_t ixallocx_prof_sample(void *ptr, size_t old_usize, size_t size, size_t extra, - size_t alignment, size_t usize_max, bool zero, prof_tctx_t *tctx) + size_t alignment, bool zero, prof_tctx_t *tctx) { size_t usize; if (tctx == NULL) return (old_usize); - /* Use minimum usize to determine whether promotion may happen. */ - if (((alignment == 0) ? s2u(size) : sa2u(size, alignment)) <= - SMALL_MAXCLASS) { - if (ixalloc(ptr, old_usize, SMALL_MAXCLASS+1, - (SMALL_MAXCLASS+1 >= size+extra) ? 0 : size+extra - - (SMALL_MAXCLASS+1), alignment, zero)) - return (old_usize); - usize = isalloc(ptr, config_prof); - if (usize_max < LARGE_MINCLASS) - arena_prof_promoted(ptr, usize); - } else { - usize = ixallocx_helper(ptr, old_usize, size, extra, alignment, - zero); - } + usize = ixallocx_helper(ptr, old_usize, size, extra, alignment, zero); return (usize); } @@ -2296,12 +2283,12 @@ ixallocx_prof(tsd_t *tsd, void *ptr, size_t old_usize, size_t size, tctx = prof_alloc_prep(tsd, usize_max, prof_active, false); if (unlikely((uintptr_t)tctx != (uintptr_t)1U)) { usize = ixallocx_prof_sample(ptr, old_usize, size, extra, - alignment, usize_max, zero, tctx); + alignment, zero, tctx); } else { usize = ixallocx_helper(ptr, old_usize, size, extra, alignment, zero); } - if (unlikely(usize == old_usize)) { + if (usize == old_usize) { prof_alloc_rollback(tsd, tctx, false); return (usize); } From 4be9c79f881066f4d3424d45d7845c03e1032d3c Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Thu, 17 Sep 2015 10:17:55 -0700 Subject: [PATCH 2/9] Fix irallocx_prof_sample(). Fix irallocx_prof_sample() to always allocate large regions, even when alignment is non-zero. --- ChangeLog | 2 ++ src/jemalloc.c | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 619c522b..b5e10c49 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,8 @@ brevity. Much more detail can be found in the git revision history: - Fix ixallocx_prof_sample() to never modify nor create sampled small allocations. xallocx() is in general incapable of moving small allocations, so this fix removes buggy code without loss of generality. + - Fix irallocx_prof_sample() to always allocate large regions, even when + alignment is non-zero. * 4.0.1 (September 15, 2015) diff --git a/src/jemalloc.c b/src/jemalloc.c index ad904eb5..b58252fd 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -2098,8 +2098,8 @@ label_oom: } static void * -irallocx_prof_sample(tsd_t *tsd, void *old_ptr, size_t old_usize, size_t size, - size_t alignment, size_t usize, bool zero, tcache_t *tcache, arena_t *arena, +irallocx_prof_sample(tsd_t *tsd, void *old_ptr, size_t old_usize, + size_t usize, size_t alignment, bool zero, tcache_t *tcache, arena_t *arena, prof_tctx_t *tctx) { void *p; @@ -2113,7 +2113,7 @@ irallocx_prof_sample(tsd_t *tsd, void *old_ptr, size_t old_usize, size_t size, return (NULL); arena_prof_promoted(p, usize); } else { - p = iralloct(tsd, old_ptr, old_usize, size, alignment, zero, + p = iralloct(tsd, old_ptr, old_usize, usize, alignment, zero, tcache, arena); } @@ -2133,8 +2133,8 @@ irallocx_prof(tsd_t *tsd, void *old_ptr, size_t old_usize, size_t size, old_tctx = prof_tctx_get(old_ptr); tctx = prof_alloc_prep(tsd, *usize, prof_active, true); if (unlikely((uintptr_t)tctx != (uintptr_t)1U)) { - p = irallocx_prof_sample(tsd, old_ptr, old_usize, size, - alignment, *usize, zero, tcache, arena, tctx); + p = irallocx_prof_sample(tsd, old_ptr, old_usize, *usize, + alignment, zero, tcache, arena, tctx); } else { p = iralloct(tsd, old_ptr, old_usize, size, alignment, zero, tcache, arena); From 3263be6efb5232963c0820da65e235d1693e404d Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Thu, 17 Sep 2015 10:19:28 -0700 Subject: [PATCH 3/9] Simplify imallocx_prof_sample(). Simplify imallocx_prof_sample() to always operate on usize rather than sometimes using size. This avoids redundant usize computations and more closely fits the style adopted by i[rx]allocx_prof_sample() to fix sampling bugs. --- src/jemalloc.c | 39 +++++++++++++-------------------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/src/jemalloc.c b/src/jemalloc.c index b58252fd..49c5f2ac 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -1965,41 +1965,29 @@ imallocx_flags(tsd_t *tsd, size_t usize, size_t alignment, bool zero, tcache_t *tcache, arena_t *arena) { - if (alignment != 0) + if (unlikely(alignment != 0)) return (ipalloct(tsd, usize, alignment, zero, tcache, arena)); - if (zero) + if (unlikely(zero)) return (icalloct(tsd, usize, tcache, arena)); return (imalloct(tsd, usize, tcache, arena)); } -JEMALLOC_ALWAYS_INLINE_C void * -imallocx_maybe_flags(tsd_t *tsd, size_t size, int flags, size_t usize, - size_t alignment, bool zero, tcache_t *tcache, arena_t *arena) -{ - - if (likely(flags == 0)) - return (imalloc(tsd, size)); - return (imallocx_flags(tsd, usize, alignment, zero, tcache, arena)); -} - static void * -imallocx_prof_sample(tsd_t *tsd, size_t size, int flags, size_t usize, - size_t alignment, bool zero, tcache_t *tcache, arena_t *arena) +imallocx_prof_sample(tsd_t *tsd, size_t usize, size_t alignment, bool zero, + tcache_t *tcache, arena_t *arena) { void *p; if (usize <= SMALL_MAXCLASS) { assert(((alignment == 0) ? s2u(LARGE_MINCLASS) : sa2u(LARGE_MINCLASS, alignment)) == LARGE_MINCLASS); - p = imallocx_maybe_flags(tsd, LARGE_MINCLASS, flags, - LARGE_MINCLASS, alignment, zero, tcache, arena); + p = imallocx_flags(tsd, LARGE_MINCLASS, alignment, zero, tcache, + arena); if (p == NULL) return (NULL); arena_prof_promoted(p, usize); - } else { - p = imallocx_maybe_flags(tsd, size, flags, usize, alignment, - zero, tcache, arena); - } + } else + p = imallocx_flags(tsd, usize, alignment, zero, tcache, arena); return (p); } @@ -2018,12 +2006,11 @@ imallocx_prof(tsd_t *tsd, size_t size, int flags, size_t *usize) &zero, &tcache, &arena))) return (NULL); tctx = prof_alloc_prep(tsd, *usize, prof_active_get_unlocked(), true); - if (likely((uintptr_t)tctx == (uintptr_t)1U)) { - p = imallocx_maybe_flags(tsd, size, flags, *usize, alignment, - zero, tcache, arena); - } else if ((uintptr_t)tctx > (uintptr_t)1U) { - p = imallocx_prof_sample(tsd, size, flags, *usize, alignment, - zero, tcache, arena); + if (likely((uintptr_t)tctx == (uintptr_t)1U)) + p = imallocx_flags(tsd, *usize, alignment, zero, tcache, arena); + else if ((uintptr_t)tctx > (uintptr_t)1U) { + p = imallocx_prof_sample(tsd, *usize, alignment, zero, tcache, + arena); } else p = NULL; if (unlikely(p == NULL)) { From 3ca0cf6a68c9eab7668be14d2b07645277f8b833 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Thu, 17 Sep 2015 14:47:39 -0700 Subject: [PATCH 4/9] Fix prof_alloc_rollback(). Fix prof_alloc_rollback() to read tdata from thread-specific data rather than dereferencing a potentially invalid tctx. --- ChangeLog | 2 ++ src/prof.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index b5e10c49..fb376b26 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,8 @@ brevity. Much more detail can be found in the git revision history: so this fix removes buggy code without loss of generality. - Fix irallocx_prof_sample() to always allocate large regions, even when alignment is non-zero. + - Fix prof_alloc_rollback() to read tdata from thread-specific data rather + than dereferencing a potentially invalid tctx. * 4.0.1 (September 15, 2015) diff --git a/src/prof.c b/src/prof.c index d68478fd..0a08062c 100644 --- a/src/prof.c +++ b/src/prof.c @@ -209,7 +209,7 @@ prof_alloc_rollback(tsd_t *tsd, prof_tctx_t *tctx, bool updated) */ tdata = prof_tdata_get(tsd, true); if (tdata != NULL) - prof_sample_threshold_update(tctx->tdata); + prof_sample_threshold_update(tdata); } if ((uintptr_t)tctx > (uintptr_t)1U) { From 4d0e162d2db0531624edee497613c7ecb1ef212d Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Thu, 17 Sep 2015 14:50:29 -0700 Subject: [PATCH 5/9] Expand check_integration_prof testing. Run integration tests with MALLOC_CONF="prof:true,prof_active:false" in addition to MALLOC_CONF="prof:true". --- Makefile.in | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.in b/Makefile.in index 01285afb..1ac6f292 100644 --- a/Makefile.in +++ b/Makefile.in @@ -353,6 +353,7 @@ check_unit: tests_unit check_unit_dir check_integration_prof: tests_integration check_integration_dir ifeq ($(enable_prof), 1) $(MALLOC_CONF)="prof:true" $(SHELL) $(objroot)test/test.sh $(TESTS_INTEGRATION:$(srcroot)%.c=$(objroot)%) + $(MALLOC_CONF)="prof:true,prof_active:false" $(SHELL) $(objroot)test/test.sh $(TESTS_INTEGRATION:$(srcroot)%.c=$(objroot)%) endif check_integration: tests_integration check_integration_dir $(SHELL) $(objroot)test/test.sh $(TESTS_INTEGRATION:$(srcroot)%.c=$(objroot)%) From 21523297fcd72128c14b40ebefbf8ccf114fbede Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Thu, 17 Sep 2015 15:27:28 -0700 Subject: [PATCH 6/9] Add mallocx() OOM tests. --- src/jemalloc.c | 2 ++ test/integration/mallocx.c | 70 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/jemalloc.c b/src/jemalloc.c index 49c5f2ac..5a2d3240 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -1923,6 +1923,7 @@ imallocx_flags_decode_hard(tsd_t *tsd, size_t size, int flags, size_t *usize, *alignment = MALLOCX_ALIGN_GET_SPECIFIED(flags); *usize = sa2u(size, *alignment); } + assert(*usize != 0); *zero = MALLOCX_ZERO_GET(flags); if ((flags & MALLOCX_TCACHE_MASK) != 0) { if ((flags & MALLOCX_TCACHE_MASK) == MALLOCX_TCACHE_NONE) @@ -2267,6 +2268,7 @@ ixallocx_prof(tsd_t *tsd, void *ptr, size_t old_usize, size_t size, */ usize_max = (alignment == 0) ? s2u(size+extra) : sa2u(size+extra, alignment); + assert(usize_max != 0); tctx = prof_alloc_prep(tsd, usize_max, prof_active, false); if (unlikely((uintptr_t)tctx != (uintptr_t)1U)) { usize = ixallocx_prof_sample(ptr, old_usize, size, extra, diff --git a/test/integration/mallocx.c b/test/integration/mallocx.c index 4b0e33f0..3973938b 100644 --- a/test/integration/mallocx.c +++ b/test/integration/mallocx.c @@ -1,5 +1,74 @@ #include "test/jemalloc_test.h" +static unsigned +get_nsizes_impl(const char *cmd) +{ + unsigned ret; + size_t z; + + z = sizeof(unsigned); + assert_d_eq(mallctl(cmd, &ret, &z, NULL, 0), 0, + "Unexpected mallctl(\"%s\", ...) failure", cmd); + + return (ret); +} + +static unsigned +get_nhuge(void) +{ + + return (get_nsizes_impl("arenas.nhchunks")); +} + +static size_t +get_size_impl(const char *cmd, size_t ind) +{ + size_t ret; + size_t z; + size_t mib[4]; + size_t miblen = 4; + + z = sizeof(size_t); + assert_d_eq(mallctlnametomib(cmd, mib, &miblen), + 0, "Unexpected mallctlnametomib(\"%s\", ...) failure", cmd); + mib[2] = ind; + z = sizeof(size_t); + assert_d_eq(mallctlbymib(mib, miblen, &ret, &z, NULL, 0), + 0, "Unexpected mallctlbymib([\"%s\", %zu], ...) failure", cmd, ind); + + return (ret); +} + +static size_t +get_huge_size(size_t ind) +{ + + return (get_size_impl("arenas.hchunk.0.size", ind)); +} + +TEST_BEGIN(test_oom) +{ + size_t hugemax, size, alignment; + + hugemax = get_huge_size(get_nhuge()-1); + + /* In practice hugemax is too large to be allocated. */ + assert_ptr_null(mallocx(hugemax, 0), + "Expected OOM for mallocx(size=%#zx, 0)", hugemax); + +#if LG_SIZEOF_PTR == 3 + size = ZU(0x8000000000000000); + alignment = ZU(0x8000000000000000); +#else + size = ZU(0x80000000); + alignment = ZU(0x80000000); +#endif + assert_ptr_null(mallocx(size, MALLOCX_ALIGN(alignment)), + "Expected OOM for mallocx(size=%#zx, MALLOCX_ALIGN(%#zx)", size, + alignment); +} +TEST_END + TEST_BEGIN(test_basic) { #define MAXSZ (((size_t)1) << 26) @@ -96,6 +165,7 @@ main(void) { return (test( + test_oom, test_basic, test_alignment_and_size)); } From e56b24e3a2db1edde23ede2477a94962ed006ae2 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 20 Sep 2015 09:58:10 -0700 Subject: [PATCH 7/9] Make arena_dalloc_large_locked_impl() static. --- src/arena.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arena.c b/src/arena.c index 2e888eaa..7f4a6cae 100644 --- a/src/arena.c +++ b/src/arena.c @@ -2560,7 +2560,7 @@ arena_dalloc_junk_large_t *arena_dalloc_junk_large = JEMALLOC_N(arena_dalloc_junk_large_impl); #endif -void +static void arena_dalloc_large_locked_impl(arena_t *arena, arena_chunk_t *chunk, void *ptr, bool junked) { From 66814c1a52de5a4a51569f9a88bae6c9b8a4c873 Mon Sep 17 00:00:00 2001 From: Craig Rodrigues Date: Sun, 20 Sep 2015 21:57:32 -0700 Subject: [PATCH 8/9] Fix tsd_boot1() to use explicit 'void' parameter list. --- include/jemalloc/internal/tsd.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/jemalloc/internal/tsd.h b/include/jemalloc/internal/tsd.h index 62a887e6..eed7aa01 100644 --- a/include/jemalloc/internal/tsd.h +++ b/include/jemalloc/internal/tsd.h @@ -190,7 +190,7 @@ a_name##tsd_boot0(void) \ return (false); \ } \ a_attr void \ -a_name##tsd_boot1() \ +a_name##tsd_boot1(void) \ { \ \ /* Do nothing. */ \ @@ -235,7 +235,7 @@ a_name##tsd_boot0(void) \ return (false); \ } \ a_attr void \ -a_name##tsd_boot1() \ +a_name##tsd_boot1(void) \ { \ \ /* Do nothing. */ \ @@ -345,7 +345,7 @@ a_name##tsd_boot0(void) \ return (false); \ } \ a_attr void \ -a_name##tsd_boot1() \ +a_name##tsd_boot1(void) \ { \ a_name##tsd_wrapper_t *wrapper; \ wrapper = (a_name##tsd_wrapper_t *) \ @@ -467,7 +467,7 @@ a_name##tsd_boot0(void) \ return (false); \ } \ a_attr void \ -a_name##tsd_boot1() \ +a_name##tsd_boot1(void) \ { \ a_name##tsd_wrapper_t *wrapper; \ wrapper = (a_name##tsd_wrapper_t *) \ From b8e966f121e55ffa0c904f9ff7d419797b872aa8 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Mon, 21 Sep 2015 10:19:37 -0700 Subject: [PATCH 9/9] Update ChangeLog for 4.0.2. --- ChangeLog | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index fb376b26..58e4462b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,7 +4,9 @@ brevity. Much more detail can be found in the git revision history: https://github.com/jemalloc/jemalloc -* 4.0.2 (XXX) +* 4.0.2 (September 21, 2015) + + This bugfix release addresses a few bugs specific to heap profiling. Bug fixes: - Fix ixallocx_prof_sample() to never modify nor create sampled small