From 38d4eabf1a8e61b4facdd3f2cb385498471d1ea9 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
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 <sbose@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
---
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