From: Nick Porter Date: Thu, 16 Feb 2023 18:18:00 +0000 (+0000) Subject: Add reserve_mru option to slab allocator X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=94c982abe424a79f2e70ef18b71a9cd09116249c;p=thirdparty%2Ffreeradius-server.git Add reserve_mru option to slab allocator With this option "true", reservations are taken from the tail of the dlists - which is where elements are returned when freed. This gives an approximation of MRU allocation. --- diff --git a/src/lib/util/slab.h b/src/lib/util/slab.h index 20b0675c8a6..e6f3c1ab90c 100644 --- a/src/lib/util/slab.h +++ b/src/lib/util/slab.h @@ -82,6 +82,7 @@ typedef struct { \ fr_ ## _type ## _slab_reserve_t reserve; \ void *uctx; \ bool release_reset; \ + bool reserve_mru; \ } fr_ ## _name ## _slab_list_t; \ \ typedef struct { \ @@ -159,6 +160,7 @@ DIAG_OFF(unused-function) \ * @param[in] reserve Optional callback run on element reserving. \ * @param[in] uctx to pass to callbacks. \ * @param[in] release_reset Should elements be reset and children freed when the element is released.\ + * @param[in] reserve_mru If true, the most recently used element will be returned when an element is reserved. \ */ \ static inline CC_HINT(nonnull(2)) int fr_ ## _name ## _slab_list_alloc(TALLOC_CTX *ctx, \ fr_ ## _name ## _slab_list_t **out, \ @@ -167,7 +169,8 @@ DIAG_OFF(unused-function) \ fr_ ## _type ## _slab_alloc_t alloc, \ fr_ ## _type ## _slab_reserve_t reserve, \ void *uctx, \ - bool release_reset) \ + bool release_reset, \ + bool reserve_mru) \ { \ MEM(*out = talloc_zero(ctx, fr_ ## _name ## _slab_list_t)); \ (*out)->el = el; \ @@ -178,6 +181,7 @@ DIAG_OFF(unused-function) \ (*out)->reserve = reserve; \ (*out)->uctx = uctx; \ (*out)->release_reset = release_reset; \ + (*out)->reserve_mru = reserve_mru; \ fr_ ## _name ## _slab_init(&(*out)->reserved); \ fr_ ## _name ## _slab_init(&(*out)->avail); \ if (el) (void) fr_event_timer_in(*out, el, &(*out)->ev, config->interval, _ ## _name ## _slab_cleanup, *out); \ @@ -218,7 +222,8 @@ DIAG_OFF(unused-function) \ fr_ ## _name ## _slab_t *slab; \ fr_ ## _name ## _slab_element_t *element = NULL; \ \ - slab = fr_ ## _name ## _slab_head(&slab_list->avail); \ + slab = slab_list->reserve_mru ? fr_ ## _name ## _slab_tail(&slab_list->avail) : \ + fr_ ## _name ## _slab_head(&slab_list->avail); \ if (!slab && ((fr_ ## _name ## _slab_num_elements(&slab_list->reserved) * \ slab_list->config.elements_per_slab) < slab_list->config.max_elements)) { \ fr_ ## _name ## _slab_element_t *new_element; \ @@ -264,7 +269,8 @@ DIAG_OFF(unused-function) \ slab_list->high_water_mark += fr_ ## _name ## _slab_element_num_elements(&slab->avail); \ } \ if (!slab && slab_list->config.at_max_fail) return NULL; \ - if (slab) element = fr_ ## _name ## _slab_element_pop_head(&slab->avail); \ + if (slab) element = slab_list->reserve_mru ? fr_ ## _name ## _slab_element_pop_tail(&slab->avail) : \ + fr_ ## _name ## _slab_element_pop_head(&slab->avail); \ if (element) { \ fr_ ## _name ## _slab_element_insert_tail(&slab->reserved, element); \ if (fr_ ## _name ## _slab_element_num_elements(&slab->avail) == 0) { \ diff --git a/src/lib/util/slab_tests.c b/src/lib/util/slab_tests.c index afe17222ed8..b7db3833c7e 100644 --- a/src/lib/util/slab_tests.c +++ b/src/lib/util/slab_tests.c @@ -62,7 +62,7 @@ static void test_alloc(void) /* * Each slab will contain 2 elements, maximum of 4 elements allocated from slabs. */ - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -136,7 +136,7 @@ static void test_alloc_fail(void) * Each slab will contain 2 elements, maximum of 4 elements allocated from slabs. */ slab_config.at_max_fail = true; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -172,7 +172,7 @@ static void test_reuse_reset(void) test_uctx_t test_uctx; int ret = -1; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -234,7 +234,7 @@ static void test_reuse_noreset(void) test_uctx_t test_uctx; int ret = -1; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, false); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, false, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -278,6 +278,56 @@ static void test_reuse_noreset(void) talloc_free(test_slab_list); } +/** Test that setting reserve_mru to true works + * + * After releasing an element, a subsequent reserve should return the same element + */ +static void test_reserve_mru(void) +{ + fr_test_slab_list_t *test_slab_list; + test_element_t *test_elements[2]; + int ret = -1; + + /* + * First use a slab list with reserve_mru = false to verify that the two reservations + * result in different elements being returned + */ + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false); + TEST_CHECK(ret == 0); + TEST_CHECK(test_slab_list != NULL); + if (!test_slab_list) return; + + test_elements[0] = fr_test_slab_reserve(test_slab_list); + TEST_CHECK(test_elements[0] != NULL); + + if (test_elements[0]) fr_test_slab_release(test_elements[0]); + + test_elements[1] = fr_test_slab_reserve(test_slab_list); + TEST_CHECK(test_elements[1] != NULL); + TEST_CHECK(test_elements[0] != test_elements[1]); + + talloc_free(test_slab_list); + + /* + * Now use a slab list with reserve_mru = true + */ + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, true); + TEST_CHECK(ret == 0); + TEST_CHECK(test_slab_list != NULL); + if (!test_slab_list) return; + + test_elements[0] = fr_test_slab_reserve(test_slab_list); + TEST_CHECK(test_elements[0] != NULL); + + if (test_elements[0]) fr_test_slab_release(test_elements[0]); + + test_elements[1] = fr_test_slab_reserve(test_slab_list); + TEST_CHECK(test_elements[1] != NULL); + TEST_CHECK(test_elements[0] == test_elements[1]); + + talloc_free(test_slab_list); +} + /** Test that talloc freeing an element results in destructor being called * */ @@ -288,7 +338,7 @@ static void test_free(void) test_uctx_t test_uctx; int ret = -1; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &def_slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -326,7 +376,7 @@ static void test_init(void) fr_slab_config_t slab_config = def_slab_config; slab_config.elements_per_slab = 1; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, test_element_alloc, NULL, &test_conf, false); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, test_element_alloc, NULL, &test_conf, false, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -366,7 +416,7 @@ static void test_reserve(void) fr_slab_config_t slab_config = def_slab_config; slab_config.elements_per_slab = 1; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, test_element_alloc, &test_conf, false); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, test_element_alloc, &test_conf, false, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -413,7 +463,7 @@ static void test_init_reserve(void) fr_slab_config_t slab_config = def_slab_config; slab_config.elements_per_slab = 1; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, test_element_alloc, test_element_reserve, &test_conf, false); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, test_element_alloc, test_element_reserve, &test_conf, false, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -457,7 +507,7 @@ static void test_clearup_1(void) fr_event_list_set_time_func(el, test_time); slab_config.max_elements = 6; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -513,7 +563,7 @@ static void test_clearup_2(void) slab_config.min_elements = 16; slab_config.max_elements = 20; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -579,7 +629,7 @@ static void test_clearup_3(void) slab_config.min_elements = 0; slab_config.max_elements = 20; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -674,7 +724,7 @@ static void test_realloc(void) slab_config.min_elements = 0; slab_config.max_elements = 20; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, el, &slab_config, NULL, NULL, NULL, true, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -733,7 +783,7 @@ static void test_child_alloc(void) slab_config.max_elements = 2; slab_config.num_children = 1; slab_config.child_pool_size = 128; - ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, NULL, NULL, false); + ret = fr_test_slab_list_alloc(NULL, &test_slab_list, NULL, &slab_config, NULL, NULL, NULL, false, false); TEST_CHECK(ret == 0); TEST_CHECK(test_slab_list != NULL); if (!test_slab_list) return; @@ -768,6 +818,7 @@ TEST_LIST = { { "test_alloc_fail", test_alloc_fail }, { "test_reuse_reset", test_reuse_reset }, { "test_reuse_noreset", test_reuse_noreset }, + { "test_reserve_mru", test_reserve_mru }, { "test_free", test_free }, { "test_init", test_init }, { "test_reserve", test_reserve }, diff --git a/src/modules/rlm_rest/rlm_rest.c b/src/modules/rlm_rest/rlm_rest.c index 24a3d567d80..d7ef00605de 100644 --- a/src/modules/rlm_rest/rlm_rest.c +++ b/src/modules/rlm_rest/rlm_rest.c @@ -1165,7 +1165,7 @@ static int mod_thread_instantiate(module_thread_inst_ctx_t const *mctx) t->inst = inst; if (fr_rest_slab_list_alloc(t, &t->slab, mctx->el, &inst->conn_config.reuse, - rest_conn_alloc, NULL, inst, false) < 0) { + rest_conn_alloc, NULL, inst, false, false) < 0) { ERROR("Connection handle pool instantiation failed"); return -1; }