From 176f73272e6e3116caab3900eb553be54f520a68 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sat, 21 Sep 2024 12:30:49 -0700 Subject: [PATCH 1/2] mmap-cache: enforce an unused windows minimum With many fds the global windows count generally exceeds the minimum. This results in always reusing the unused entry if there is one, which becomes a sort of degenerate case where we're just constantly unmapping->mapping. Instead let's try always have at least several unused windows on the unused list before we resort to churning through it. Fixes #34516 --- src/libsystemd/sd-journal/mmap-cache.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-journal/mmap-cache.c b/src/libsystemd/sd-journal/mmap-cache.c index 973ade64c0f..249b98a968c 100644 --- a/src/libsystemd/sd-journal/mmap-cache.c +++ b/src/libsystemd/sd-journal/mmap-cache.c @@ -64,11 +64,13 @@ struct MMapCache { LIST_HEAD(Window, unused); Window *last_unused; + unsigned n_unused; Window *windows_by_category[_MMAP_CACHE_CATEGORY_MAX]; }; #define WINDOWS_MIN 64 +#define UNUSED_MIN 4 #if ENABLE_DEBUG_MMAP_CACHE /* Tiny windows increase mmap activity and the chance of exposing unsafe use. */ @@ -103,6 +105,7 @@ static Window* window_unlink(Window *w) { if (m->last_unused == w) m->last_unused = w->unused_prev; LIST_REMOVE(unused, m->unused, w); + m->n_unused--; } for (unsigned i = 0; i < _MMAP_CACHE_CATEGORY_MAX; i++) @@ -160,7 +163,7 @@ static Window* window_add(MMapFileDescriptor *f, uint64_t offset, size_t size, v MMapCache *m = mmap_cache_fd_cache(f); Window *w; - if (!m->last_unused || m->n_windows <= WINDOWS_MIN) { + if (!m->last_unused || m->n_windows < WINDOWS_MIN || m->n_unused < UNUSED_MIN) { /* Allocate a new window */ w = new(Window, 1); if (!w) @@ -202,6 +205,7 @@ static void category_detach_window(MMapCache *m, MMapCacheCategory c) { LIST_PREPEND(unused, m->unused, w); if (!m->last_unused) m->last_unused = w; + m->n_unused++; w->flags |= WINDOW_IN_UNUSED; #endif } @@ -222,6 +226,7 @@ static void category_attach_window(MMapCache *m, MMapCacheCategory c, Window *w) if (m->last_unused == w) m->last_unused = w->unused_prev; LIST_REMOVE(unused, m->unused, w); + m->n_unused--; w->flags &= ~WINDOW_IN_UNUSED; } @@ -239,7 +244,7 @@ static MMapCache* mmap_cache_free(MMapCache *m) { assert(hashmap_isempty(m->fds)); hashmap_free(m->fds); - assert(!m->unused); + assert(!m->unused && m->n_unused == 0); assert(m->n_windows == 0); return mfree(m); From 284802c597aa0194dc1504db65ee24941d9721eb Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sat, 21 Sep 2024 14:43:51 -0700 Subject: [PATCH 2/2] mmap-cache: add some stats about files/windows/unused Let's give some visibility into the ratio of files:windows:unused by the time we're done using the cache. --- src/libsystemd/sd-journal/mmap-cache.c | 4 ++-- src/libsystemd/sd-journal/sd-journal.c | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-journal/mmap-cache.c b/src/libsystemd/sd-journal/mmap-cache.c index 249b98a968c..ce8aff3c5c1 100644 --- a/src/libsystemd/sd-journal/mmap-cache.c +++ b/src/libsystemd/sd-journal/mmap-cache.c @@ -435,8 +435,8 @@ found: void mmap_cache_stats_log_debug(MMapCache *m) { assert(m); - log_debug("mmap cache statistics: %u category cache hit, %u window list hit, %u miss", - m->n_category_cache_hit, m->n_window_list_hit, m->n_missed); + log_debug("mmap cache statistics: %u category cache hit, %u window list hit, %u miss, %u files, %u windows, %u unused", + m->n_category_cache_hit, m->n_window_list_hit, m->n_missed, hashmap_size(m->fds), m->n_windows, m->n_unused); } static void mmap_cache_process_sigbus(MMapCache *m) { diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index daa9670ab4c..858aeca45ef 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2542,6 +2542,10 @@ _public_ void sd_journal_close(sd_journal *j) { sd_journal_flush_matches(j); + /* log stats before closing files so we can see the windows state */ + if (j->mmap) + mmap_cache_stats_log_debug(j->mmap); + ordered_hashmap_free_with_destructor(j->files, journal_file_close); iterated_cache_free(j->files_cache); @@ -2553,10 +2557,8 @@ _public_ void sd_journal_close(sd_journal *j) { safe_close(j->inotify_fd); - if (j->mmap) { - mmap_cache_stats_log_debug(j->mmap); + if (j->mmap) mmap_cache_unref(j->mmap); - } hashmap_free_free(j->errors);