From fd9ac6c3078ba1c0037c10bdb0412d84e0b76966 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Thu, 25 Nov 2021 15:01:38 -0800 Subject: [PATCH 1/6] mmap-cache: ref/unref MMapCache in fd add/free Preparatory commit; callers manually ref/unref MMapCaches alongside MMapFileDescriptor add/frees, when the latter should be sufficient. A subsequent commit will drop some of those manual MMapCache reference hoop-jumping, leaving the lifecycle bound to MMapFileDescriptors. --- src/libsystemd/sd-journal/mmap-cache.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-journal/mmap-cache.c b/src/libsystemd/sd-journal/mmap-cache.c index faf05c5507..4ecaaf2388 100644 --- a/src/libsystemd/sd-journal/mmap-cache.c +++ b/src/libsystemd/sd-journal/mmap-cache.c @@ -560,14 +560,14 @@ MMapFileDescriptor* mmap_cache_add_fd(MMapCache *m, int fd, int prot) { if (!f) return NULL; - f->cache = m; - f->fd = fd; - f->prot = prot; - r = hashmap_put(m->fds, FD_TO_PTR(fd), f); if (r < 0) return mfree(f); + f->cache = mmap_cache_ref(m); + f->fd = fd; + f->prot = prot; + return f; } @@ -584,8 +584,10 @@ void mmap_cache_fd_free(MMapFileDescriptor *f) { while (f->windows) window_free(f->windows); - if (f->cache) + if (f->cache) { assert_se(hashmap_remove(f->cache->fds, FD_TO_PTR(f->fd))); + f->cache = mmap_cache_unref(f->cache); + } free(f); } From 176bf8b82783b76c85e890610a6c2c6fafb5db6a Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Thu, 25 Nov 2021 15:07:39 -0800 Subject: [PATCH 2/6] mmap-cache: add MMapFileDescriptor.cache accessor Sometimes we want to reuse an existing MMapFileDescriptor's cache, but it's a private struct. This lets us access that pointer if necessary. --- src/libsystemd/sd-journal/mmap-cache.c | 6 ++++++ src/libsystemd/sd-journal/mmap-cache.h | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-journal/mmap-cache.c b/src/libsystemd/sd-journal/mmap-cache.c index 4ecaaf2388..124ee3f8c1 100644 --- a/src/libsystemd/sd-journal/mmap-cache.c +++ b/src/libsystemd/sd-journal/mmap-cache.c @@ -591,3 +591,9 @@ void mmap_cache_fd_free(MMapFileDescriptor *f) { free(f); } + +MMapCache* mmap_cache_fd_cache(MMapFileDescriptor *f) { + assert(f); + + return f->cache; +} diff --git a/src/libsystemd/sd-journal/mmap-cache.h b/src/libsystemd/sd-journal/mmap-cache.h index 28f699dbd1..907ebae843 100644 --- a/src/libsystemd/sd-journal/mmap-cache.h +++ b/src/libsystemd/sd-journal/mmap-cache.h @@ -22,7 +22,8 @@ int mmap_cache_fd_get( size_t size, struct stat *st, void **ret); -MMapFileDescriptor * mmap_cache_add_fd(MMapCache *m, int fd, int prot); +MMapFileDescriptor* mmap_cache_add_fd(MMapCache *m, int fd, int prot); +MMapCache* mmap_cache_fd_cache(MMapFileDescriptor *f); void mmap_cache_fd_free(MMapFileDescriptor *f); void mmap_cache_stats_log_debug(MMapCache *m); From 8b4fbbb0a121028c9304e96df322ce491f551e34 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Thu, 25 Nov 2021 15:24:48 -0800 Subject: [PATCH 3/6] journal: stop using JournalFile.mmap everywhere Preparatory commit; before JournalFile can stop hanging onto its copy of MMapCache, all these users need to find another way. Most of the time these callers already have the MMapCache onhand, so it's no big deal for them to just supply it. journal_file_rotate() in particular needed to change, and it seemed wise to not use the mmap_cache_fd_cache() accessor on f->cache_fd, instead requiring the caller supply the cache to use. This was done with an eye towards a potential future where the journal_file_archive() isolates the cache_fd to a private cache, which the newly rotated-to file wouldn't be allowed to use. It's no biggie for the existing callers to just provide the appropriate surviving cache. Basically the mmap_cache_fd_cache() accessor was added just for journal-verify.c's (ab)use of the mmap-cache. Which, if the ugly singleton MMapCache assumption ever goes away, can be cleaned up to simply use a separate MMapCache for those search arrays. --- src/journal-remote/journal-remote-write.c | 8 ++++---- src/journal/journald-file.c | 3 ++- src/journal/journald-file.h | 2 +- src/journal/journald-server.c | 2 +- src/journal/test-journal.c | 4 ++-- src/libsystemd/sd-journal/journal-file.h | 1 + src/libsystemd/sd-journal/journal-verify.c | 8 +++++--- 7 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/journal-remote/journal-remote-write.c b/src/journal-remote/journal-remote-write.c index fd7cb91f2c..b82cb10118 100644 --- a/src/journal-remote/journal-remote-write.c +++ b/src/journal-remote/journal-remote-write.c @@ -3,8 +3,8 @@ #include "alloc-util.h" #include "journal-remote.h" -static int do_rotate(JournaldFile **f, bool compress, bool seal) { - int r = journald_file_rotate(f, compress, UINT64_MAX, seal, NULL); +static int do_rotate(JournaldFile **f, MMapCache *m, bool compress, bool seal) { + int r = journald_file_rotate(f, m, compress, UINT64_MAX, seal, NULL); if (r < 0) { if (*f) log_error_errno(r, "Failed to rotate %s: %m", (*f)->file->path); @@ -71,7 +71,7 @@ int writer_write(Writer *w, if (journal_file_rotate_suggested(w->journal->file, 0, LOG_DEBUG)) { log_info("%s: Journal header limits reached or header out-of-date, rotating", w->journal->file->path); - r = do_rotate(&w->journal, compress, seal); + r = do_rotate(&w->journal, w->mmap, compress, seal); if (r < 0) return r; } @@ -87,7 +87,7 @@ int writer_write(Writer *w, return r; log_debug_errno(r, "%s: Write failed, rotating: %m", w->journal->file->path); - r = do_rotate(&w->journal, compress, seal); + r = do_rotate(&w->journal, w->mmap, compress, seal); if (r < 0) return r; else diff --git a/src/journal/journald-file.c b/src/journal/journald-file.c index 7f607d4aa6..be3fc1615a 100644 --- a/src/journal/journald-file.c +++ b/src/journal/journald-file.c @@ -403,6 +403,7 @@ JournaldFile* journald_file_initiate_close(JournaldFile *f, Set *deferred_closes int journald_file_rotate( JournaldFile **f, + MMapCache *mmap_cache, bool compress, uint64_t compress_threshold_bytes, bool seal, @@ -428,7 +429,7 @@ int journald_file_rotate( compress_threshold_bytes, seal, NULL, /* metrics */ - (*f)->file->mmap, + mmap_cache, deferred_closes, *f, /* template */ &new_file); diff --git a/src/journal/journald-file.h b/src/journal/journald-file.h index 341043c836..79c0ef8240 100644 --- a/src/journal/journald-file.h +++ b/src/journal/journald-file.h @@ -40,4 +40,4 @@ int journald_file_open_reliably( JournaldFile **ret); JournaldFile* journald_file_initiate_close(JournaldFile *f, Set *deferred_closes); -int journald_file_rotate(JournaldFile **f, bool compress, uint64_t compress_threshold_bytes, bool seal, Set *deferred_closes); +int journald_file_rotate(JournaldFile **f, MMapCache *mmap_cache, bool compress, uint64_t compress_threshold_bytes, bool seal, Set *deferred_closes); diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index a13bbeeee9..c04801d9fc 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -465,7 +465,7 @@ static int do_rotate( if (!*f) return -EINVAL; - r = journald_file_rotate(f, s->compress.enabled, s->compress.threshold_bytes, seal, s->deferred_closes); + r = journald_file_rotate(f, s->mmap, s->compress.enabled, s->compress.threshold_bytes, seal, s->deferred_closes); if (r < 0) { if (*f) return log_error_errno(r, "Failed to rotate %s: %m", (*f)->file->path); diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c index c2c4a75fef..17c6a73db3 100644 --- a/src/journal/test-journal.c +++ b/src/journal/test-journal.c @@ -98,8 +98,8 @@ static void test_non_empty(void) { assert_se(journal_file_move_to_entry_by_seqnum(f->file, 10, DIRECTION_DOWN, &o, NULL) == 0); - journald_file_rotate(&f, true, UINT64_MAX, true, NULL); - journald_file_rotate(&f, true, UINT64_MAX, true, NULL); + journald_file_rotate(&f, NULL, true, UINT64_MAX, true, NULL); + journald_file_rotate(&f, NULL, true, UINT64_MAX, true, NULL); (void) journald_file_close(f); diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index ecda2b3cc0..90c9dbe637 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -232,6 +232,7 @@ void journal_file_dump(JournalFile *f); void journal_file_print_header(JournalFile *f); int journal_file_archive(JournalFile *f, char **ret_previous_path); +JournalFile* journal_initiate_close(JournalFile *f, Set *deferred_closes); int journal_file_dispose(int dir_fd, const char *fname); diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index b6427105c2..8288ebcd6e 100644 --- a/src/libsystemd/sd-journal/journal-verify.c +++ b/src/libsystemd/sd-journal/journal-verify.c @@ -885,6 +885,7 @@ int journal_file_verify( unsigned i; bool found_last = false; const char *tmp_dir = NULL; + MMapCache *m; #if HAVE_GCRYPT uint64_t last_tag = 0; @@ -929,19 +930,20 @@ int journal_file_verify( goto fail; } - cache_data_fd = mmap_cache_add_fd(f->mmap, data_fd, PROT_READ|PROT_WRITE); + m = mmap_cache_fd_cache(f->cache_fd); + cache_data_fd = mmap_cache_add_fd(m, data_fd, PROT_READ|PROT_WRITE); if (!cache_data_fd) { r = log_oom(); goto fail; } - cache_entry_fd = mmap_cache_add_fd(f->mmap, entry_fd, PROT_READ|PROT_WRITE); + cache_entry_fd = mmap_cache_add_fd(m, entry_fd, PROT_READ|PROT_WRITE); if (!cache_entry_fd) { r = log_oom(); goto fail; } - cache_entry_array_fd = mmap_cache_add_fd(f->mmap, entry_array_fd, PROT_READ|PROT_WRITE); + cache_entry_array_fd = mmap_cache_add_fd(m, entry_array_fd, PROT_READ|PROT_WRITE); if (!cache_entry_array_fd) { r = log_oom(); goto fail; From a8da63309c9a288f741772d2fc35061f2a9babe4 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Thu, 25 Nov 2021 15:32:07 -0800 Subject: [PATCH 4/6] journal-file: goodbye JournalFile.mmap This gets rid of the manual MMapCache ref/unref goop in journal_file_{open,close}(), in favor of just letting the JournalFile.cache_fd MMapFileDescriptor carry the baton. --- src/libsystemd/sd-journal/journal-file.c | 20 +++++++++++--------- src/libsystemd/sd-journal/journal-file.h | 1 - 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 11b9da1cb5..4302a99f80 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -217,15 +217,13 @@ JournalFile* journal_file_close(JournalFile *f) { if (!f) return NULL; - if (f->mmap && f->cache_fd) + if (f->cache_fd) mmap_cache_fd_free(f->cache_fd); if (f->close_fd) safe_close(f->fd); free(f->path); - mmap_cache_unref(f->mmap); - ordered_hashmap_free_free(f->chain_cache); #if HAVE_COMPRESSION @@ -3252,6 +3250,7 @@ int journal_file_open( JournalFile **ret) { bool newly_created = false; + MMapCache *m = mmap_cache; JournalFile *f; void *h; int r; @@ -3320,11 +3319,9 @@ int journal_file_open( } } - if (mmap_cache) - f->mmap = mmap_cache_ref(mmap_cache); - else { - f->mmap = mmap_cache_new(); - if (!f->mmap) { + if (!m) { + m = mmap_cache_new(); + if (!m) { r = -ENOMEM; goto fail; } @@ -3371,12 +3368,17 @@ int journal_file_open( goto fail; } - f->cache_fd = mmap_cache_add_fd(f->mmap, f->fd, prot_from_flags(flags)); + /* On success this incs refcnt on *m, which mmap_cache_fd_free() will dec. */ + f->cache_fd = mmap_cache_add_fd(m, f->fd, prot_from_flags(flags)); if (!f->cache_fd) { r = -ENOMEM; goto fail; } + /* If we created *m just for this file, unref *m so only f->cache_fd's ref remains */ + if (!mmap_cache) + (void) mmap_cache_unref(m); + r = journal_file_fstat(f); if (r < 0) goto fail; diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index 90c9dbe637..b90e3a608a 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -92,7 +92,6 @@ typedef struct JournalFile { uint64_t current_xor_hash; JournalMetrics metrics; - MMapCache *mmap; sd_event_source *post_change_timer; usec_t post_change_timer_period; From 333d0672625b0b466f9d1d7b46e452105ec13a2b Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Tue, 7 Dec 2021 14:16:28 -0800 Subject: [PATCH 5/6] mmap-cache: add MMapCache trivial cleanup helpers Enable _cleanup_(mmap_cache_unrefp) style cleanup for MMapCache* --- src/libsystemd/sd-journal/mmap-cache.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libsystemd/sd-journal/mmap-cache.h b/src/libsystemd/sd-journal/mmap-cache.h index 907ebae843..4769414ded 100644 --- a/src/libsystemd/sd-journal/mmap-cache.h +++ b/src/libsystemd/sd-journal/mmap-cache.h @@ -13,6 +13,7 @@ typedef struct MMapFileDescriptor MMapFileDescriptor; MMapCache* mmap_cache_new(void); MMapCache* mmap_cache_ref(MMapCache *m); MMapCache* mmap_cache_unref(MMapCache *m); +DEFINE_TRIVIAL_CLEANUP_FUNC(MMapCache*, mmap_cache_unref); int mmap_cache_fd_get( MMapFileDescriptor *f, From 74fb5be62d163ce234cc37fd0166bc587cc23e07 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Tue, 7 Dec 2021 14:18:14 -0800 Subject: [PATCH 6/6] journal-file: require MMapCache* for journal_file_open() Previously the MMapCache* was optionally NULL, which open would handle by creating a new MMapCache* for the occasion. This produced some slightly circuitous refcount-handling code in the function, as well as arguably creating opportunities for weirdness where an MMapCache* was intended to be supplied but happened to be NULL, which this magic would then paper over. In any case, this was basically only being utilized by tests, apparently just to avoid having to create an MMapCache. So update the relevant tests to supply an MMapCache and make journal_file_open() treat a NULL MMapCache* as fatal w/assert. --- src/journal/test-journal-flush.c | 5 ++++- src/journal/test-journal-interleaving.c | 17 ++++++++++---- src/journal/test-journal-stream.c | 10 ++++++--- src/journal/test-journal-verify.c | 15 ++++++++++--- src/journal/test-journal.c | 28 +++++++++++++++++------- src/libsystemd/sd-journal/journal-file.c | 17 ++------------ 6 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/journal/test-journal-flush.c b/src/journal/test-journal-flush.c index f0a7024002..dc05126b57 100644 --- a/src/journal/test-journal-flush.c +++ b/src/journal/test-journal-flush.c @@ -14,6 +14,7 @@ #include "string-util.h" int main(int argc, char *argv[]) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; _cleanup_free_ char *fn = NULL; char dn[] = "/var/tmp/test-journal-flush.XXXXXX"; JournaldFile *new_journal = NULL; @@ -21,12 +22,14 @@ int main(int argc, char *argv[]) { unsigned n = 0; int r; + m = mmap_cache_new(); + assert_se(m != NULL); assert_se(mkdtemp(dn)); (void) chattr_path(dn, FS_NOCOW_FL, FS_NOCOW_FL, NULL); fn = path_join(dn, "test.journal"); - r = journald_file_open(-1, fn, O_CREAT|O_RDWR, 0644, false, 0, false, NULL, NULL, NULL, NULL, &new_journal); + r = journald_file_open(-1, fn, O_CREAT|O_RDWR, 0644, false, 0, false, NULL, m, NULL, NULL, &new_journal); assert_se(r >= 0); if (argc > 1) diff --git a/src/journal/test-journal-interleaving.c b/src/journal/test-journal-interleaving.c index 48be6a14bc..c543b87b69 100644 --- a/src/journal/test-journal-interleaving.c +++ b/src/journal/test-journal-interleaving.c @@ -34,8 +34,13 @@ _noreturn_ static void log_assert_errno(const char *text, int error, const char } while (false) static JournaldFile *test_open(const char *name) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; JournaldFile *f; - assert_ret(journald_file_open(-1, name, O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &f)); + + m = mmap_cache_new(); + assert_se(m != NULL); + + assert_ret(journald_file_open(-1, name, O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f)); return f; } @@ -198,15 +203,19 @@ static void test_skip(void (*setup)(void)) { static void test_sequence_numbers(void) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; char t[] = "/var/tmp/journal-seq-XXXXXX"; JournaldFile *one, *two; uint64_t seqnum = 0; sd_id128_t seqnum_id; + m = mmap_cache_new(); + assert_se(m != NULL); + mkdtemp_chdir_chattr(t); assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0644, - true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &one) == 0); + true, UINT64_MAX, false, NULL, m, NULL, NULL, &one) == 0); append_number(one, 1, &seqnum); printf("seqnum=%"PRIu64"\n", seqnum); @@ -223,7 +232,7 @@ static void test_sequence_numbers(void) { memcpy(&seqnum_id, &one->file->header->seqnum_id, sizeof(sd_id128_t)); assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0644, - true, UINT64_MAX, false, NULL, NULL, NULL, one, &two) == 0); + true, UINT64_MAX, false, NULL, m, NULL, one, &two) == 0); assert_se(two->file->header->state == STATE_ONLINE); assert_se(!sd_id128_equal(two->file->header->file_id, one->file->header->file_id)); @@ -254,7 +263,7 @@ static void test_sequence_numbers(void) { seqnum = 0; assert_se(journald_file_open(-1, "two.journal", O_RDWR, 0, - true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &two) == 0); + true, UINT64_MAX, false, NULL, m, NULL, NULL, &two) == 0); assert_se(sd_id128_equal(two->file->header->seqnum_id, seqnum_id)); diff --git a/src/journal/test-journal-stream.c b/src/journal/test-journal-stream.c index f7fc4332a9..115ce807c9 100644 --- a/src/journal/test-journal-stream.c +++ b/src/journal/test-journal-stream.c @@ -60,6 +60,7 @@ static void verify_contents(sd_journal *j, unsigned skip) { } static void run_test(void) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; JournaldFile *one, *two, *three; char t[] = "/var/tmp/journal-stream-XXXXXX"; unsigned i; @@ -69,13 +70,16 @@ static void run_test(void) { size_t l; dual_timestamp previous_ts = DUAL_TIMESTAMP_NULL; + m = mmap_cache_new(); + assert_se(m != NULL); + assert_se(mkdtemp(t)); assert_se(chdir(t) >= 0); (void) chattr_path(t, FS_NOCOW_FL, FS_NOCOW_FL, NULL); - assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &one) == 0); - assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &two) == 0); - assert_se(journald_file_open(-1, "three.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &three) == 0); + assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &one) == 0); + assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &two) == 0); + assert_se(journald_file_open(-1, "three.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &three) == 0); for (i = 0; i < N_ENTRIES; i++) { char *p, *q; diff --git a/src/journal/test-journal-verify.c b/src/journal/test-journal-verify.c index cf9692b3d2..6abfaeb0df 100644 --- a/src/journal/test-journal-verify.c +++ b/src/journal/test-journal-verify.c @@ -10,6 +10,7 @@ #include "journald-file.h" #include "journal-verify.h" #include "log.h" +#include "mmap-cache.h" #include "rm-rf.h" #include "terminal-util.h" #include "tests.h" @@ -38,10 +39,14 @@ static void bit_toggle(const char *fn, uint64_t p) { } static int raw_verify(const char *fn, const char *verification_key) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; JournalFile *f; int r; - r = journal_file_open(-1, fn, O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, NULL, NULL, &f); + m = mmap_cache_new(); + assert_se(m != NULL); + + r = journal_file_open(-1, fn, O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, &f); if (r < 0) return r; @@ -52,6 +57,7 @@ static int raw_verify(const char *fn, const char *verification_key) { } int main(int argc, char *argv[]) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; char t[] = "/var/tmp/journal-XXXXXX"; unsigned n; JournalFile *f; @@ -61,6 +67,9 @@ int main(int argc, char *argv[]) { struct stat st; uint64_t p; + m = mmap_cache_new(); + assert_se(m != NULL); + /* journald_file_open requires a valid machine id */ if (access("/etc/machine-id", F_OK) != 0) return log_tests_skipped("/etc/machine-id not found"); @@ -73,7 +82,7 @@ int main(int argc, char *argv[]) { log_info("Generating..."); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, !!verification_key, NULL, NULL, NULL, NULL, &df) == 0); + assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, NULL, &df) == 0); for (n = 0; n < N_ENTRIES; n++) { struct iovec iovec; @@ -95,7 +104,7 @@ int main(int argc, char *argv[]) { log_info("Verifying..."); - assert_se(journal_file_open(-1, "test.journal", O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, NULL, NULL, &f) == 0); + assert_se(journal_file_open(-1, "test.journal", O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, &f) == 0); /* journal_file_print_header(f); */ journal_file_dump(f); diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c index 17c6a73db3..11647504e9 100644 --- a/src/journal/test-journal.c +++ b/src/journal/test-journal.c @@ -24,6 +24,7 @@ static void mkdtemp_chdir_chattr(char *path) { } static void test_non_empty(void) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; dual_timestamp ts; JournaldFile *f; struct iovec iovec; @@ -35,9 +36,12 @@ static void test_non_empty(void) { test_setup_logging(LOG_DEBUG); + m = mmap_cache_new(); + assert_se(m != NULL); + mkdtemp_chdir_chattr(t); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, NULL, NULL, NULL, &f) == 0); + assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f) == 0); assert_se(dual_timestamp_get(&ts)); assert_se(sd_id128_randomize(&fake_boot_id) == 0); @@ -98,8 +102,8 @@ static void test_non_empty(void) { assert_se(journal_file_move_to_entry_by_seqnum(f->file, 10, DIRECTION_DOWN, &o, NULL) == 0); - journald_file_rotate(&f, NULL, true, UINT64_MAX, true, NULL); - journald_file_rotate(&f, NULL, true, UINT64_MAX, true, NULL); + journald_file_rotate(&f, m, true, UINT64_MAX, true, NULL); + journald_file_rotate(&f, m, true, UINT64_MAX, true, NULL); (void) journald_file_close(f); @@ -117,17 +121,21 @@ static void test_non_empty(void) { } static void test_empty(void) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; JournaldFile *f1, *f2, *f3, *f4; char t[] = "/var/tmp/journal-XXXXXX"; test_setup_logging(LOG_DEBUG); + m = mmap_cache_new(); + assert_se(m != NULL); + mkdtemp_chdir_chattr(t); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, false, NULL, NULL, NULL, NULL, &f1) == 0); - assert_se(journald_file_open(-1, "test-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &f2) == 0); - assert_se(journald_file_open(-1, "test-seal.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, true, NULL, NULL, NULL, NULL, &f3) == 0); - assert_se(journald_file_open(-1, "test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, NULL, NULL, NULL, &f4) == 0); + assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, false, NULL, m, NULL, NULL, &f1) == 0); + assert_se(journald_file_open(-1, "test-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f2) == 0); + assert_se(journald_file_open(-1, "test-seal.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, true, NULL, m, NULL, NULL, &f3) == 0); + assert_se(journald_file_open(-1, "test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f4) == 0); journal_file_print_header(f1->file); puts(""); @@ -156,6 +164,7 @@ static void test_empty(void) { #if HAVE_COMPRESSION static bool check_compressed(uint64_t compress_threshold, uint64_t data_size) { + _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; dual_timestamp ts; JournaldFile *f; struct iovec iovec; @@ -170,9 +179,12 @@ static bool check_compressed(uint64_t compress_threshold, uint64_t data_size) { test_setup_logging(LOG_DEBUG); + m = mmap_cache_new(); + assert_se(m != NULL); + mkdtemp_chdir_chattr(t); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, compress_threshold, true, NULL, NULL, NULL, NULL, &f) == 0); + assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, compress_threshold, true, NULL, m, NULL, NULL, &f) == 0); dual_timestamp_get(&ts); diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 4302a99f80..505e4f728d 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3250,13 +3250,13 @@ int journal_file_open( JournalFile **ret) { bool newly_created = false; - MMapCache *m = mmap_cache; JournalFile *f; void *h; int r; assert(ret); assert(fd >= 0 || fname); + assert(mmap_cache); if (!IN_SET((flags & O_ACCMODE), O_RDONLY, O_RDWR)) return -EINVAL; @@ -3319,14 +3319,6 @@ int journal_file_open( } } - if (!m) { - m = mmap_cache_new(); - if (!m) { - r = -ENOMEM; - goto fail; - } - } - if (fname) { f->path = strdup(fname); if (!f->path) { @@ -3368,17 +3360,12 @@ int journal_file_open( goto fail; } - /* On success this incs refcnt on *m, which mmap_cache_fd_free() will dec. */ - f->cache_fd = mmap_cache_add_fd(m, f->fd, prot_from_flags(flags)); + f->cache_fd = mmap_cache_add_fd(mmap_cache, f->fd, prot_from_flags(flags)); if (!f->cache_fd) { r = -ENOMEM; goto fail; } - /* If we created *m just for this file, unref *m so only f->cache_fd's ref remains */ - if (!mmap_cache) - (void) mmap_cache_unref(m); - r = journal_file_fstat(f); if (r < 0) goto fail;