diff --git a/Makefile.in b/Makefile.in index a0131558..3a02b3fd 100644 --- a/Makefile.in +++ b/Makefile.in @@ -225,6 +225,7 @@ TESTS_UNIT := \ $(srcroot)test/unit/hook.c \ $(srcroot)test/unit/hpa.c \ $(srcroot)test/unit/hpa_background_thread.c \ + $(srcroot)test/unit/hpa_validate_conf.c \ $(srcroot)test/unit/hpdata.c \ $(srcroot)test/unit/huge.c \ $(srcroot)test/unit/inspect.c \ diff --git a/include/jemalloc/internal/jemalloc_internal_externs.h b/include/jemalloc/internal/jemalloc_internal_externs.h index ae03c644..64d9aa20 100644 --- a/include/jemalloc/internal/jemalloc_internal_externs.h +++ b/include/jemalloc/internal/jemalloc_internal_externs.h @@ -25,6 +25,7 @@ extern bool opt_junk_alloc; extern bool opt_junk_free; extern void (*JET_MUTABLE junk_free_callback)(void *ptr, size_t size); extern void (*JET_MUTABLE junk_alloc_callback)(void *ptr, size_t size); +extern void (*JET_MUTABLE invalid_conf_abort)(void); extern bool opt_utrace; extern bool opt_xmalloc; extern bool opt_experimental_infallible_new; diff --git a/src/jemalloc.c b/src/jemalloc.c index ccb20c81..c5a06f6e 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -144,6 +144,7 @@ static void default_junk_free(void *ptr, size_t usize) { void (*JET_MUTABLE junk_alloc_callback)(void *ptr, size_t size) = &default_junk_alloc; void (*JET_MUTABLE junk_free_callback)(void *ptr, size_t size) = &default_junk_free; +void (*JET_MUTABLE invalid_conf_abort)(void) = &abort; bool opt_utrace = false; bool opt_xmalloc = false; @@ -959,7 +960,7 @@ malloc_abort_invalid_conf(void) { assert(opt_abort_conf); malloc_printf(": Abort (abort_conf:true) on invalid conf " "value (see above).\n"); - abort(); + invalid_conf_abort(); } static void @@ -1081,6 +1082,46 @@ obtain_malloc_conf(unsigned which_source, char buf[PATH_MAX + 1]) { return ret; } +static void +validate_hpa_settings(void) { + if (!hpa_supported() || !opt_hpa || opt_hpa_opts.dirty_mult == (fxp_t)-1) { + return; + } + size_t hpa_threshold = fxp_mul_frac(HUGEPAGE, opt_hpa_opts.dirty_mult) + + opt_hpa_opts.hugification_threshold; + if (hpa_threshold > HUGEPAGE) { + return; + } + + had_conf_error = true; + char hpa_dirty_mult[FXP_BUF_SIZE]; + char hugification_threshold[FXP_BUF_SIZE]; + char normalization_message[256] = {0}; + fxp_print(opt_hpa_opts.dirty_mult, hpa_dirty_mult); + fxp_print(fxp_div(FXP_INIT_INT((unsigned) + (opt_hpa_opts.hugification_threshold >> LG_PAGE)), + FXP_INIT_INT(HUGEPAGE_PAGES)), hugification_threshold); + if (!opt_abort_conf) { + char normalized_hugification_threshold[FXP_BUF_SIZE]; + opt_hpa_opts.hugification_threshold += + HUGEPAGE - hpa_threshold; + fxp_print(fxp_div(FXP_INIT_INT((unsigned) + (opt_hpa_opts.hugification_threshold >> LG_PAGE)), + FXP_INIT_INT(HUGEPAGE_PAGES)), + normalized_hugification_threshold); + malloc_snprintf(normalization_message, + sizeof(normalization_message), ": Normalizing " + "HPA settings to avoid pathological behavior, setting " + "hpa_hugification_threshold_ratio: to %s.\n", + normalized_hugification_threshold); + } + malloc_printf( + ": Invalid combination of options " + "hpa_hugification_threshold_ratio: %s and hpa_dirty_mult: %s. " + "These values should sum to > 1.0.\n%s", hugification_threshold, + hpa_dirty_mult, normalization_message); +} + static void malloc_conf_init_helper(sc_data_t *sc_data, unsigned bin_shard_sizes[SC_NBINS], bool initial_call, const char *opts_cache[MALLOC_CONF_NSOURCES], @@ -1749,6 +1790,7 @@ malloc_conf_init_helper(sc_data_t *sc_data, unsigned bin_shard_sizes[SC_NBINS], /* Re-enable diagnostic "-Wtype-limits" */ JEMALLOC_DIAGNOSTIC_POP } + validate_hpa_settings(); if (opt_abort_conf && had_conf_error) { malloc_abort_invalid_conf(); } diff --git a/test/unit/hpa_background_thread.sh b/test/unit/hpa_background_thread.sh index 65a56a08..33b70e19 100644 --- a/test/unit/hpa_background_thread.sh +++ b/test/unit/hpa_background_thread.sh @@ -1,4 +1,4 @@ #!/bin/sh -export MALLOC_CONF="hpa_dirty_mult:0,hpa_min_purge_interval_ms:50,hpa_sec_nshards:0" +export MALLOC_CONF="hpa_dirty_mult:0.001,hpa_hugification_threshold_ratio:1.0,hpa_min_purge_interval_ms:50,hpa_sec_nshards:0" diff --git a/test/unit/hpa_validate_conf.c b/test/unit/hpa_validate_conf.c new file mode 100644 index 00000000..8c1847ba --- /dev/null +++ b/test/unit/hpa_validate_conf.c @@ -0,0 +1,56 @@ +#include "test/jemalloc_test.h" + +static bool abort_called = false; +static void (*default_malloc_message)(void *, const char *); + +static void +mock_invalid_conf_abort(void) { + abort_called = true; +} + +static void +null_malloc_message(void *_1, const char* _2) { +} + +TEST_BEGIN(test_hpa_validate_conf) { + test_skip_if(!hpa_supported()); + void *ptr = malloc(4096); + /* Need to restore this here to see any possible assert messages */ + malloc_message = default_malloc_message; + assert_true(abort_called, + "Should have aborted due to invalid values for hpa_dirty_mult and " + "hpa_hugification_threshold_ratio"); + free(ptr); +} +TEST_END + +/* + * We have to set `abort_conf:true` here and not via the `MALLOC_CONF` + * environment variable in the associated shell script for this test. This is + * because when testing on FreeBSD (where Jemalloc is the system allocator) in + * CI configs where HPA is not supported, setting `abort_conf:true` there would + * result in the system Jemalloc picking this up and aborting before we could + * ever even launch the test. + */ +const char *malloc_conf = "abort_conf:true"; + +int +main(void) { + /* + * OK, this is a sort of nasty hack. We don't want to add *another* + * config option for HPA (the intent is that it becomes available on + * more platforms over time, and we're trying to prune back config + * options generally. But we'll get initialization errors on other + * platforms if we set hpa:true in the MALLOC_CONF (even if we set + * abort_conf:false as well). So we reach into the internals and set + * them directly, but only if we know that we're actually going to do + * something nontrivial in the tests. + */ + if (hpa_supported()) { + default_malloc_message = malloc_message; + malloc_message = null_malloc_message; + opt_hpa = true; + invalid_conf_abort = mock_invalid_conf_abort; + } + return test_no_reentrancy(test_hpa_validate_conf); +} diff --git a/test/unit/hpa_validate_conf.sh b/test/unit/hpa_validate_conf.sh new file mode 100644 index 00000000..692c3da9 --- /dev/null +++ b/test/unit/hpa_validate_conf.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +export MALLOC_CONF='tcache:false,hpa_dirty_mult:0.25,hpa_hugification_threshold_ratio:0.6'