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