From 38d4eabf1a8e61b4facdd3f2cb385498471d1ea9 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Tue, 3 Mar 2020 18:44:11 +0100 Subject: [PATCH] mem-cache: sizes of free and data tables were made consistent Since size of "free table" didn't account for SSS_AVG_*_PAYLOAD factor only small fraction of "data table" was actually used. SSS_AVG_*_PAYLOAD differentiation for different payload types only affected size of hash table and was removed as unjustified. Resolves: https://pagure.io/SSSD/sssd/issue/4160 (cherry picked from PR999) Reviewed-by: Sumit Bose Reviewed-by: Sumit Bose --- src/responder/nss/nsssrv.c | 22 +++++++++++------- src/responder/nss/nsssrv_mmap_cache.c | 33 +++++++-------------------- src/responder/nss/nsssrv_mmap_cache.h | 2 -- src/util/mmap_cache.h | 3 --- 4 files changed, 22 insertions(+), 38 deletions(-) diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c index 87a3f1d50..d9fe28708 100644 --- a/src/responder/nss/nsssrv.c +++ b/src/responder/nss/nsssrv.c @@ -99,10 +99,9 @@ static int nss_clear_memcache(struct sbus_request *dbus_req, void *data) return ret; } - /* TODO: read cache sizes from configuration */ DEBUG(SSSDBG_TRACE_FUNC, "Clearing memory caches.\n"); ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid, - SSS_MC_CACHE_ELEMENTS, + -1, /* keep current size */ (time_t) memcache_timeout, &nctx->pwd_mc_ctx); if (ret != EOK) { @@ -112,7 +111,7 @@ static int nss_clear_memcache(struct sbus_request *dbus_req, void *data) } ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid, - SSS_MC_CACHE_ELEMENTS, + -1, /* keep current size */ (time_t) memcache_timeout, &nctx->grp_mc_ctx); if (ret != EOK) { @@ -122,7 +121,7 @@ static int nss_clear_memcache(struct sbus_request *dbus_req, void *data) } ret = sss_mmap_cache_reinit(nctx, nctx->mc_uid, nctx->mc_gid, - SSS_MC_CACHE_ELEMENTS, + -1, /* keep current size */ (time_t)memcache_timeout, &nctx->initgr_mc_ctx); if (ret != EOK) { @@ -257,6 +256,11 @@ static void nss_dp_reconnect_init(struct sbus_connection *conn, static int setup_memcaches(struct nss_ctx *nctx) { + /* TODO: read cache sizes from configuration */ + static const size_t SSS_MC_CACHE_PASSWD_SLOTS = 200000; /* 8mb */ + static const size_t SSS_MC_CACHE_GROUP_SLOTS = 150000; /* 6mb */ + static const size_t SSS_MC_CACHE_INITGROUP_SLOTS = 250000; /* 10mb */ + int ret; int memcache_timeout; @@ -286,11 +290,11 @@ static int setup_memcaches(struct nss_ctx *nctx) return EOK; } - /* TODO: read cache sizes from configuration */ ret = sss_mmap_cache_init(nctx, "passwd", nctx->mc_uid, nctx->mc_gid, SSS_MC_PASSWD, - SSS_MC_CACHE_ELEMENTS, (time_t)memcache_timeout, + SSS_MC_CACHE_PASSWD_SLOTS, + (time_t)memcache_timeout, &nctx->pwd_mc_ctx); if (ret) { DEBUG(SSSDBG_CRIT_FAILURE, "passwd mmap cache is DISABLED\n"); @@ -299,7 +303,8 @@ static int setup_memcaches(struct nss_ctx *nctx) ret = sss_mmap_cache_init(nctx, "group", nctx->mc_uid, nctx->mc_gid, SSS_MC_GROUP, - SSS_MC_CACHE_ELEMENTS, (time_t)memcache_timeout, + SSS_MC_CACHE_GROUP_SLOTS, + (time_t)memcache_timeout, &nctx->grp_mc_ctx); if (ret) { DEBUG(SSSDBG_CRIT_FAILURE, "group mmap cache is DISABLED\n"); @@ -308,7 +313,8 @@ static int setup_memcaches(struct nss_ctx *nctx) ret = sss_mmap_cache_init(nctx, "initgroups", nctx->mc_uid, nctx->mc_gid, SSS_MC_INITGROUPS, - SSS_MC_CACHE_ELEMENTS, (time_t)memcache_timeout, + SSS_MC_CACHE_INITGROUP_SLOTS, + (time_t)memcache_timeout, &nctx->initgr_mc_ctx); if (ret) { DEBUG(SSSDBG_CRIT_FAILURE, "initgroups mmap cache is DISABLED\n"); diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c index d5181d771..5296952a5 100644 --- a/src/responder/nss/nsssrv_mmap_cache.c +++ b/src/responder/nss/nsssrv_mmap_cache.c @@ -27,13 +27,6 @@ #include "responder/nss/nss_private.h" #include "responder/nss/nsssrv_mmap_cache.h" -/* arbitrary (avg of my /etc/passwd) */ -#define SSS_AVG_PASSWD_PAYLOAD (MC_SLOT_SIZE * 4) -/* short group name and no gids (private user group */ -#define SSS_AVG_GROUP_PAYLOAD (MC_SLOT_SIZE * 3) -/* average place for 40 supplementary groups + 2 names */ -#define SSS_AVG_INITGROUP_PAYLOAD (MC_SLOT_SIZE * 5) - #define MC_NEXT_BARRIER(val) ((((val) + 1) & 0x00ffffff) | 0xf0000000) #define MC_RAISE_BARRIER(m) do { \ @@ -1253,25 +1246,15 @@ errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, enum sss_mc_type type, size_t n_elem, time_t timeout, struct sss_mc_ctx **mcc) { + /* sss_mc_header alone occupies whole slot, + * so each entry takes 2 slots at the very least + */ + static const int PAYLOAD_FACTOR = 2; + struct sss_mc_ctx *mc_ctx = NULL; unsigned int rseed; - int payload; int ret, dret; - switch (type) { - case SSS_MC_PASSWD: - payload = SSS_AVG_PASSWD_PAYLOAD; - break; - case SSS_MC_GROUP: - payload = SSS_AVG_GROUP_PAYLOAD; - break; - case SSS_MC_INITGROUPS: - payload = SSS_AVG_INITGROUP_PAYLOAD; - break; - default: - return EINVAL; - } - mc_ctx = talloc_zero(mem_ctx, struct sss_mc_ctx); if (!mc_ctx) { return ENOMEM; @@ -1306,9 +1289,9 @@ errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name, /* hash table is double the size because it will store both forward and * reverse keys (name/uid, name/gid, ..) */ - mc_ctx->ht_size = MC_HT_SIZE(n_elem * 2); - mc_ctx->dt_size = MC_DT_SIZE(n_elem, payload); - mc_ctx->ft_size = MC_FT_SIZE(n_elem); + mc_ctx->ht_size = MC_HT_SIZE(2 * n_elem / PAYLOAD_FACTOR); + mc_ctx->dt_size = n_elem * MC_SLOT_SIZE; + mc_ctx->ft_size = n_elem / 8; /* 1 bit per slot */ mc_ctx->mmap_size = MC_HEADER_SIZE + MC_ALIGN64(mc_ctx->dt_size) + MC_ALIGN64(mc_ctx->ft_size) + diff --git a/src/responder/nss/nsssrv_mmap_cache.h b/src/responder/nss/nsssrv_mmap_cache.h index e06257949..c40af2fb4 100644 --- a/src/responder/nss/nsssrv_mmap_cache.h +++ b/src/responder/nss/nsssrv_mmap_cache.h @@ -22,8 +22,6 @@ #ifndef _NSSSRV_MMAP_CACHE_H_ #define _NSSSRV_MMAP_CACHE_H_ -#define SSS_MC_CACHE_ELEMENTS 50000 - struct sss_mc_ctx; enum sss_mc_type { diff --git a/src/util/mmap_cache.h b/src/util/mmap_cache.h index 63e096027..d3d92bc98 100644 --- a/src/util/mmap_cache.h +++ b/src/util/mmap_cache.h @@ -40,9 +40,6 @@ typedef uint32_t rel_ptr_t; #define MC_HT_SIZE(elems) ( (elems) * MC_32 ) #define MC_HT_ELEMS(size) ( (size) / MC_32 ) -#define MC_DT_SIZE(elems, payload) ( (elems) * (payload) ) -#define MC_FT_SIZE(elems) ( (elems) / 8 ) -/* ^^ 8 bits per byte so we need just elems/8 bytes to represent all blocks */ #define MC_PTR_ADD(ptr, bytes) (void *)((uint8_t *)(ptr) + (bytes)) #define MC_PTR_DIFF(ptr, base) ((uint8_t *)(ptr) - (uint8_t *)(base)) -- 2.21.1