[Fix] memstat: report per-process mempool counters

The aggregate mempool counters live in a MAP_SHARED mmap created in
rspamd_main before fork, so every worker reads and increments the same
physical page. Reporting that value per-worker made every row identical
(449.4M in a 28-worker test) and the "total" row N-counted it.

Mirror each shared-counter write into a process-local rspamd_mempool_stat_t
in BSS (which fork duplicates) and expose it via rspamd_mempool_stat_local().
Switch the memstat collector to use the local view so per-worker numbers
diverge and the total is meaningful. The original rspamd_mempool_stat()
keeps the shared semantics for /stat back-compat.

diff --git a/src/libserver/memory_stat.cxx b/src/libserver/memory_stat.cxx
index d67aa6777e0604bd061bf70cf33b4011a040fd6f..3da54307abef86c3d1e00c0a7f56bade84ab19a3 100644 (file)
--- a/src/libserver/memory_stat.cxx
+++ b/src/libserver/memory_stat.cxx
@@ -88,9 +88,14 @@ void emit_process_info(ucl_object_t *parent)
 uint64_t
 emit_mempool_info(ucl_object_t *parent)
 {
+       /*
+        * Use the per-process counters here: rspamd_mempool_stat() reads from
+        * a MAP_SHARED page and would return the same value in every worker,
+        * which makes per-worker reporting and the aggregate total useless.
+        */
        rspamd_mempool_stat_t agg;
        memset(&agg, 0, sizeof(agg));
-       rspamd_mempool_stat(&agg);
+       rspamd_mempool_stat_local(&agg);
 
        auto *mp = ucl_object_typed_new(UCL_OBJECT);
 

diff --git a/src/libutil/mem_pool.c b/src/libutil/mem_pool.c
index fe11d4911cef418ca014e21cb3b42198a1f35851..ab7e3b52bbc73169216079e513fc762c51909d4d 100644 (file)
--- a/src/libutil/mem_pool.c
+++ b/src/libutil/mem_pool.c
@@ -83,6 +83,15 @@ static khash_t(mempool_entry) *mempool_entries = NULL;
 
 /* Internal statistic */
 static rspamd_mempool_stat_t *mem_pool_stat = NULL;
+/*
+ * Per-process counters that mirror mem_pool_stat. Unlike the shared mmap
+ * above (which is created with MAP_ANON|MAP_SHARED in the parent before
+ * fork and therefore aggregates allocations across every worker), this
+ * struct lives in the BSS and is duplicated on fork, so each worker keeps
+ * its own running totals. The mirror is updated alongside every shared
+ * counter mutation.
+ */
+static rspamd_mempool_stat_t mem_pool_stat_local;
 /* Environment variable */
 static gboolean env_checked = FALSE;
 static gboolean always_malloc = FALSE;
@@ -219,7 +228,9 @@ rspamd_mempool_chain_new(gsize size, gsize alignment, enum rspamd_mempool_chain_
 #error No mmap methods are defined
 #endif
                g_atomic_int_inc(&mem_pool_stat->shared_chunks_allocated);
+               g_atomic_int_inc(&mem_pool_stat_local.shared_chunks_allocated);
                g_atomic_int_add(&mem_pool_stat->bytes_allocated, total_size);
+               g_atomic_int_add(&mem_pool_stat_local.bytes_allocated, total_size);
        }
        else {
 #ifdef HAVE_MALLOC_SIZE
@@ -237,7 +248,9 @@ rspamd_mempool_chain_new(gsize size, gsize alignment, enum rspamd_mempool_chain_
                chain = map;
                chain->begin = ((uint8_t *) chain) + sizeof(struct _pool_chain);
                g_atomic_int_add(&mem_pool_stat->bytes_allocated, total_size);
+               g_atomic_int_add(&mem_pool_stat_local.bytes_allocated, total_size);
                g_atomic_int_inc(&mem_pool_stat->chunks_allocated);
+               g_atomic_int_inc(&mem_pool_stat_local.chunks_allocated);
        }
 
        chain->pos = align_ptr(chain->begin, alignment);
@@ -428,6 +441,7 @@ rspamd_mempool_new_(gsize size, const char *tag, int flags, const char *loc)
        new_pool->tag.uid[enc_len] = '\0';
 
        mem_pool_stat->pools_allocated++;
+       mem_pool_stat_local.pools_allocated++;
 
        /* Now we can attach one chunk to speed up simple allocations */
        struct _pool_chain *nchain;
@@ -451,7 +465,10 @@ rspamd_mempool_new_(gsize size, const char *tag, int flags, const char *loc)
        /* Adjust stats */
        g_atomic_int_add(&mem_pool_stat->bytes_allocated,
                                         (int) size);
+       g_atomic_int_add(&mem_pool_stat_local.bytes_allocated,
+                                        (int) size);
        g_atomic_int_add(&mem_pool_stat->chunks_allocated, 1);
+       g_atomic_int_add(&mem_pool_stat_local.chunks_allocated, 1);
 
        return new_pool;
 }
@@ -539,8 +556,11 @@ memory_pool_alloc_common(rspamd_mempool_t *pool, gsize size, gsize alignment,
                        }
                        else {
                                mem_pool_stat->oversized_chunks++;
+                               mem_pool_stat_local.oversized_chunks++;
                                g_atomic_int_add(&mem_pool_stat->fragmented_size,
                                                                 free);
+                               g_atomic_int_add(&mem_pool_stat_local.fragmented_size,
+                                                                free);
                                pool->priv->entry->elts[pool->priv->entry->cur_elts].fragmentation += free;
                                new = rspamd_mempool_chain_new(size + pool->priv->elt_len, alignment,
                                                                                           pool_type);
@@ -984,7 +1004,10 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool)
                        {
                                g_atomic_int_add(&mem_pool_stat->bytes_allocated,
                                                                 -((int) cur->slice_size));
+                               g_atomic_int_add(&mem_pool_stat_local.bytes_allocated,
+                                                                -((int) cur->slice_size));
                                g_atomic_int_add(&mem_pool_stat->chunks_allocated, -1);
+                               g_atomic_int_add(&mem_pool_stat_local.chunks_allocated, -1);
 
                                len = cur->slice_size + sizeof(struct _pool_chain);
 
@@ -1002,6 +1025,7 @@ void rspamd_mempool_delete(rspamd_mempool_t *pool)
        }
 
        g_atomic_int_inc(&mem_pool_stat->pools_freed);
+       g_atomic_int_inc(&mem_pool_stat_local.pools_freed);
        POOL_MTX_UNLOCK();
        free(pool); /* allocated by posix_memalign */
 }
@@ -1024,6 +1048,23 @@ void rspamd_mempool_stat_reset(void)
        if (mem_pool_stat != NULL) {
                memset(mem_pool_stat, 0, sizeof(rspamd_mempool_stat_t));
        }
+       memset(&mem_pool_stat_local, 0, sizeof(mem_pool_stat_local));
+}
+
+void rspamd_mempool_stat_local(rspamd_mempool_stat_t *st)
+{
+       if (st == NULL) {
+               return;
+       }
+
+       st->pools_allocated = mem_pool_stat_local.pools_allocated;
+       st->pools_freed = mem_pool_stat_local.pools_freed;
+       st->shared_chunks_allocated = mem_pool_stat_local.shared_chunks_allocated;
+       st->bytes_allocated = mem_pool_stat_local.bytes_allocated;
+       st->chunks_allocated = mem_pool_stat_local.chunks_allocated;
+       st->chunks_freed = mem_pool_stat_local.chunks_freed;
+       st->oversized_chunks = mem_pool_stat_local.oversized_chunks;
+       st->fragmented_size = mem_pool_stat_local.fragmented_size;
 }
 
 void rspamd_mempool_entries_foreach(rspamd_mempool_entry_cb cb, void *ud)

diff --git a/src/libutil/mem_pool.h b/src/libutil/mem_pool.h
index 61a9c74b03373c792caaf9f31479ef2611afb3de..3dbe89152ec2c527ee2579592310d1ab2d07934e 100644 (file)
--- a/src/libutil/mem_pool.h
+++ b/src/libutil/mem_pool.h
@@ -363,13 +363,25 @@ void rspamd_mempool_runlock_rwlock(rspamd_mempool_rwlock_t *lock);
 void rspamd_mempool_wunlock_rwlock(rspamd_mempool_rwlock_t *lock);
 
 /**
- * Get pool allocator statistics
+ * Get pool allocator statistics aggregated across every process that
+ * shares the rspamd mempool counters page (i.e. rspamd_main plus all
+ * forked workers, since the page is mmap'd MAP_SHARED before fork).
  * @param st stat pool struct
  */
 void rspamd_mempool_stat(rspamd_mempool_stat_t *st);
 
 /**
- * Reset memory pool stat
+ * Get the calling process's local view of the mempool counters. Unlike
+ * rspamd_mempool_stat(), the underlying counters live in the BSS and are
+ * duplicated on fork, so each worker reports its own running totals from
+ * program start (counters accumulated before fork are inherited as the
+ * baseline; resetting at fork time is the caller's responsibility).
+ * @param st destination stat struct, must be non-NULL
+ */
+void rspamd_mempool_stat_local(rspamd_mempool_stat_t *st);
+
+/**
+ * Reset memory pool stat (both the shared aggregate and the local copy).
  */
 void rspamd_mempool_stat_reset(void);