Merge pull request #21452 from vcaputo/mmap-cache-fd

mmap-cache: simplify MMapFileDescriptor-centric function signatures

5 files changed, 62 insertions, 72 deletions

diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c
index 2b36c03c38..b286ac5482 100644
--- a/src/libsystemd/sd-journal/journal-file.c
+++ b/src/libsystemd/sd-journal/journal-file.c
@@ -164,7 +164,7 @@ static int journal_file_set_offline_thread_join(JournalFile *f) {
 
         f->offline_state = OFFLINE_JOINED;
 
-        if (mmap_cache_got_sigbus(f->mmap, f->cache_fd))
+        if (mmap_cache_fd_got_sigbus(f->cache_fd))
                 return -EIO;
 
         return 0;
@@ -324,7 +324,7 @@ static int journal_file_set_online(JournalFile *f) {
                 }
         }
 
-        if (mmap_cache_got_sigbus(f->mmap, f->cache_fd))
+        if (mmap_cache_fd_got_sigbus(f->cache_fd))
                 return -EIO;
 
         switch (f->header->state) {
@@ -377,7 +377,7 @@ JournalFile* journal_file_close(JournalFile *f) {
         journal_file_set_offline(f, true);
 
         if (f->mmap && f->cache_fd)
-                mmap_cache_free_fd(f->mmap, f->cache_fd);
+                mmap_cache_fd_free(f->cache_fd);
 
         if (f->fd >= 0 && f->defrag_on_close) {
 
@@ -655,7 +655,7 @@ static int journal_file_allocate(JournalFile *f, uint64_t offset, uint64_t size)
         if (size > PAGE_ALIGN_DOWN(UINT64_MAX) - offset)
                 return -EINVAL;
 
-        if (mmap_cache_got_sigbus(f->mmap, f->cache_fd))
+        if (mmap_cache_fd_got_sigbus(f->cache_fd))
                 return -EIO;
 
         old_header_size = le64toh(READ_NOW(f->header->header_size));
@@ -755,7 +755,7 @@ static int journal_file_move_to(
                         return -EADDRNOTAVAIL;
         }
 
-        return mmap_cache_get(f->mmap, f->cache_fd, type_to_context(type), keep_always, offset, size, &f->last_stat, ret);
+        return mmap_cache_fd_get(f->cache_fd, type_to_context(type), keep_always, offset, size, &f->last_stat, ret);
 }
 
 static uint64_t minimum_header_size(Object *o) {
@@ -2188,7 +2188,7 @@ int journal_file_append_entry(
          * it is very likely just an effect of a nullified replacement
          * mapping page */
 
-        if (mmap_cache_got_sigbus(f->mmap, f->cache_fd))
+        if (mmap_cache_fd_got_sigbus(f->cache_fd))
                 r = -EIO;
 
         if (f->post_change_timer)
@@ -3564,7 +3564,7 @@ int journal_file_open(
                 goto fail;
         }
 
-        r = mmap_cache_get(f->mmap, f->cache_fd, CONTEXT_HEADER, true, 0, PAGE_ALIGN(sizeof(Header)), &f->last_stat, &h);
+        r = mmap_cache_fd_get(f->cache_fd, CONTEXT_HEADER, true, 0, PAGE_ALIGN(sizeof(Header)), &f->last_stat, &h);
         if (r == -EINVAL) {
                 /* Some file systems (jffs2 or p9fs) don't support mmap() properly (or only read-only
                  * mmap()), and return EINVAL in that case. Let's propagate that as a more recognizable error
@@ -3627,7 +3627,7 @@ int journal_file_open(
 #endif
         }
 
-        if (mmap_cache_got_sigbus(f->mmap, f->cache_fd)) {
+        if (mmap_cache_fd_got_sigbus(f->cache_fd)) {
                 r = -EIO;
                 goto fail;
         }
@@ -3649,7 +3649,7 @@ int journal_file_open(
         return 0;
 
 fail:
-        if (f->cache_fd && mmap_cache_got_sigbus(f->mmap, f->cache_fd))
+        if (f->cache_fd && mmap_cache_fd_got_sigbus(f->cache_fd))
                 r = -EIO;
 
         (void) journal_file_close(f);
@@ -3939,7 +3939,7 @@ int journal_file_copy_entry(JournalFile *from, JournalFile *to, Object *o, uint6
         r = journal_file_append_entry_internal(to, &ts, boot_id, xor_hash, items, n,
                                                NULL, NULL, NULL);
 
-        if (mmap_cache_got_sigbus(to->mmap, to->cache_fd))
+        if (mmap_cache_fd_got_sigbus(to->cache_fd))
                 return -EIO;
 
         return r;
diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c
index acb1b4c7e8..437a506d51 100644
--- a/src/libsystemd/sd-journal/journal-verify.c
+++ b/src/libsystemd/sd-journal/journal-verify.c
@@ -387,11 +387,10 @@ static int write_uint64(FILE *fp, uint64_t p) {
         return 0;
 }
 
-static int contains_uint64(MMapCache *m, MMapFileDescriptor *f, uint64_t n, uint64_t p) {
+static int contains_uint64(MMapFileDescriptor *f, uint64_t n, uint64_t p) {
         uint64_t a, b;
         int r;
 
-        assert(m);
         assert(f);
 
         /* Bisection ... */
@@ -402,7 +401,7 @@ static int contains_uint64(MMapCache *m, MMapFileDescriptor *f, uint64_t n, uint
 
                 c = (a + b) / 2;
 
-                r = mmap_cache_get(m, f, 0, false, c * sizeof(uint64_t), sizeof(uint64_t), NULL, (void **) &z);
+                r = mmap_cache_fd_get(f, 0, false, c * sizeof(uint64_t), sizeof(uint64_t), NULL, (void **) &z);
                 if (r < 0)
                         return r;
 
@@ -436,7 +435,7 @@ static int entry_points_to_data(
         assert(f);
         assert(cache_entry_fd);
 
-        if (!contains_uint64(f->mmap, cache_entry_fd, n_entries, entry_p)) {
+        if (!contains_uint64(cache_entry_fd, n_entries, entry_p)) {
                 error(data_p, "Data object references invalid entry at "OFSfmt, entry_p);
                 return -EBADMSG;
         }
@@ -550,7 +549,7 @@ static int verify_data(
                         return -EBADMSG;
                 }
 
-                if (!contains_uint64(f->mmap, cache_entry_array_fd, n_entry_arrays, a)) {
+                if (!contains_uint64(cache_entry_array_fd, n_entry_arrays, a)) {
                         error(p, "Invalid array offset "OFSfmt, a);
                         return -EBADMSG;
                 }
@@ -627,7 +626,7 @@ static int verify_data_hash_table(
                         Object *o;
                         uint64_t next;
 
-                        if (!contains_uint64(f->mmap, cache_data_fd, n_data, p)) {
+                        if (!contains_uint64(cache_data_fd, n_data, p)) {
                                 error(p, "Invalid data object at hash entry %"PRIu64" of %"PRIu64, i, n);
                                 return -EBADMSG;
                         }
@@ -716,7 +715,7 @@ static int verify_entry(
                 q = le64toh(o->entry.items[i].object_offset);
                 h = le64toh(o->entry.items[i].hash);
 
-                if (!contains_uint64(f->mmap, cache_data_fd, n_data, q)) {
+                if (!contains_uint64(cache_data_fd, n_data, q)) {
                         error(p, "Invalid data object of entry");
                         return -EBADMSG;
                 }
@@ -773,7 +772,7 @@ static int verify_entry_array(
                         return -EBADMSG;
                 }
 
-                if (!contains_uint64(f->mmap, cache_entry_array_fd, n_entry_arrays, a)) {
+                if (!contains_uint64(cache_entry_array_fd, n_entry_arrays, a)) {
                         error(a, "Invalid array %"PRIu64" of %"PRIu64, i, n);
                         return -EBADMSG;
                 }
@@ -799,7 +798,7 @@ static int verify_entry_array(
                         }
                         last = p;
 
-                        if (!contains_uint64(f->mmap, cache_entry_fd, n_entries, p)) {
+                        if (!contains_uint64(cache_entry_fd, n_entries, p)) {
                                 error(a, "Invalid array entry at %"PRIu64" of %"PRIu64, i, n);
                                 return -EBADMSG;
                         }
@@ -1357,9 +1356,9 @@ int journal_file_verify(
         if (show_progress)
                 flush_progress();
 
-        mmap_cache_free_fd(f->mmap, cache_data_fd);
-        mmap_cache_free_fd(f->mmap, cache_entry_fd);
-        mmap_cache_free_fd(f->mmap, cache_entry_array_fd);
+        mmap_cache_fd_free(cache_data_fd);
+        mmap_cache_fd_free(cache_entry_fd);
+        mmap_cache_fd_free(cache_entry_array_fd);
 
         if (first_contained)
                 *first_contained = le64toh(f->header->head_entry_realtime);
@@ -1381,13 +1380,13 @@ fail:
                   100 * p / f->last_stat.st_size);
 
         if (cache_data_fd)
-                mmap_cache_free_fd(f->mmap, cache_data_fd);
+                mmap_cache_fd_free(cache_data_fd);
 
         if (cache_entry_fd)
-                mmap_cache_free_fd(f->mmap, cache_entry_fd);
+                mmap_cache_fd_free(cache_entry_fd);
 
         if (cache_entry_array_fd)
-                mmap_cache_free_fd(f->mmap, cache_entry_array_fd);
+                mmap_cache_fd_free(cache_entry_array_fd);
 
         return r;
 }
diff --git a/src/libsystemd/sd-journal/mmap-cache.c b/src/libsystemd/sd-journal/mmap-cache.c
index 9e0be01d41..cc02d8c76c 100644
--- a/src/libsystemd/sd-journal/mmap-cache.c
+++ b/src/libsystemd/sd-journal/mmap-cache.c
@@ -300,7 +300,6 @@ static int make_room(MMapCache *m) {
 }
 
 static int try_context(
-                MMapCache *m,
                 MMapFileDescriptor *f,
                 unsigned context,
                 bool keep_always,
@@ -310,13 +309,13 @@ static int try_context(
 
         Context *c;
 
-        assert(m);
-        assert(m->n_ref > 0);
         assert(f);
+        assert(f->cache);
+        assert(f->cache->n_ref > 0);
         assert(size > 0);
         assert(ret);
 
-        c = m->contexts[context];
+        c = f->cache->contexts[context];
         if (!c)
                 return 0;
 
@@ -338,12 +337,12 @@ static int try_context(
         c->window->keep_always = c->window->keep_always || keep_always;
 
         *ret = (uint8_t*) c->window->ptr + (offset - c->window->offset);
+        f->cache->n_context_cache_hit++;
 
         return 1;
 }
 
 static int find_mmap(
-                MMapCache *m,
                 MMapFileDescriptor *f,
                 unsigned context,
                 bool keep_always,
@@ -354,9 +353,9 @@ static int find_mmap(
         Window *w;
         Context *c;
 
-        assert(m);
-        assert(m->n_ref > 0);
         assert(f);
+        assert(f->cache);
+        assert(f->cache->n_ref > 0);
         assert(size > 0);
 
         if (f->sigbus)
@@ -369,7 +368,7 @@ static int find_mmap(
         if (!w)
                 return 0;
 
-        c = context_add(m, context);
+        c = context_add(f->cache, context);
         if (!c)
                 return -ENOMEM;
 
@@ -377,14 +376,14 @@ static int find_mmap(
         w->keep_always = w->keep_always || keep_always;
 
         *ret = (uint8_t*) w->ptr + (offset - w->offset);
+        f->cache->n_window_list_hit++;
 
         return 1;
 }
 
-static int mmap_try_harder(MMapCache *m, void *addr, MMapFileDescriptor *f, int flags, uint64_t offset, size_t size, void **res) {
+static int mmap_try_harder(MMapFileDescriptor *f, void *addr, int flags, uint64_t offset, size_t size, void **res) {
         void *ptr;
 
-        assert(m);
         assert(f);
         assert(res);
 
@@ -397,7 +396,7 @@ static int mmap_try_harder(MMapCache *m, void *addr, MMapFileDescriptor *f, int
                 if (errno != ENOMEM)
                         return negative_errno();
 
-                r = make_room(m);
+                r = make_room(f->cache);
                 if (r < 0)
                         return r;
                 if (r == 0)
@@ -409,7 +408,6 @@ static int mmap_try_harder(MMapCache *m, void *addr, MMapFileDescriptor *f, int
 }
 
 static int add_mmap(
-                MMapCache *m,
                 MMapFileDescriptor *f,
                 unsigned context,
                 bool keep_always,
@@ -424,9 +422,9 @@ static int add_mmap(
         void *d;
         int r;
 
-        assert(m);
-        assert(m->n_ref > 0);
         assert(f);
+        assert(f->cache);
+        assert(f->cache->n_ref > 0);
         assert(size > 0);
         assert(ret);
 
@@ -459,15 +457,15 @@ static int add_mmap(
                         wsize = PAGE_ALIGN(st->st_size - woffset);
         }
 
-        r = mmap_try_harder(m, NULL, f, MAP_SHARED, woffset, wsize, &d);
+        r = mmap_try_harder(f, NULL, MAP_SHARED, woffset, wsize, &d);
         if (r < 0)
                 return r;
 
-        c = context_add(m, context);
+        c = context_add(f->cache, context);
         if (!c)
                 goto outofmem;
 
-        w = window_add(m, f, keep_always, woffset, wsize, d);
+        w = window_add(f->cache, f, keep_always, woffset, wsize, d);
         if (!w)
                 goto outofmem;
 
@@ -482,8 +480,7 @@ outofmem:
         return -ENOMEM;
 }
 
-int mmap_cache_get(
-                MMapCache *m,
+int mmap_cache_fd_get(
                 MMapFileDescriptor *f,
                 unsigned context,
                 bool keep_always,
@@ -494,31 +491,27 @@ int mmap_cache_get(
 
         int r;
 
-        assert(m);
-        assert(m->n_ref > 0);
         assert(f);
+        assert(f->cache);
+        assert(f->cache->n_ref > 0);
         assert(size > 0);
         assert(ret);
         assert(context < MMAP_CACHE_MAX_CONTEXTS);
 
         /* Check whether the current context is the right one already */
-        r = try_context(m, f, context, keep_always, offset, size, ret);
-        if (r != 0) {
-                m->n_context_cache_hit++;
+        r = try_context(f, context, keep_always, offset, size, ret);
+        if (r != 0)
                 return r;
-        }
 
         /* Search for a matching mmap */
-        r = find_mmap(m, f, context, keep_always, offset, size, ret);
-        if (r != 0) {
-                m->n_window_list_hit++;
+        r = find_mmap(f, context, keep_always, offset, size, ret);
+        if (r != 0)
                 return r;
-        }
 
-        m->n_missed++;
+        f->cache->n_missed++;
 
         /* Create a new mmap */
-        return add_mmap(m, f, context, keep_always, offset, size, st, ret);
+        return add_mmap(f, context, keep_always, offset, size, st, ret);
 }
 
 void mmap_cache_stats_log_debug(MMapCache *m) {
@@ -589,11 +582,10 @@ static void mmap_cache_process_sigbus(MMapCache *m) {
         }
 }
 
-bool mmap_cache_got_sigbus(MMapCache *m, MMapFileDescriptor *f) {
-        assert(m);
+bool mmap_cache_fd_got_sigbus(MMapFileDescriptor *f) {
         assert(f);
 
-        mmap_cache_process_sigbus(m);
+        mmap_cache_process_sigbus(f->cache);
 
         return f->sigbus;
 }
@@ -628,15 +620,15 @@ MMapFileDescriptor* mmap_cache_add_fd(MMapCache *m, int fd, int prot) {
         return f;
 }
 
-void mmap_cache_free_fd(MMapCache *m, MMapFileDescriptor *f) {
-        assert(m);
+void mmap_cache_fd_free(MMapFileDescriptor *f) {
         assert(f);
+        assert(f->cache);
 
         /* Make sure that any queued SIGBUS are first dispatched, so
          * that we don't end up with a SIGBUS entry we cannot relate
          * to any existing memory map */
 
-        mmap_cache_process_sigbus(m);
+        mmap_cache_process_sigbus(f->cache);
 
         while (f->windows)
                 window_free(f->windows);
diff --git a/src/libsystemd/sd-journal/mmap-cache.h b/src/libsystemd/sd-journal/mmap-cache.h
index 705e56e1f6..28f699dbd1 100644
--- a/src/libsystemd/sd-journal/mmap-cache.h
+++ b/src/libsystemd/sd-journal/mmap-cache.h
@@ -14,8 +14,7 @@ MMapCache* mmap_cache_new(void);
 MMapCache* mmap_cache_ref(MMapCache *m);
 MMapCache* mmap_cache_unref(MMapCache *m);
 
-int mmap_cache_get(
-        MMapCache *m,
+int mmap_cache_fd_get(
         MMapFileDescriptor *f,
         unsigned context,
         bool keep_always,
@@ -24,8 +23,8 @@ int mmap_cache_get(
         struct stat *st,
         void **ret);
 MMapFileDescriptor * mmap_cache_add_fd(MMapCache *m, int fd, int prot);
-void mmap_cache_free_fd(MMapCache *m, MMapFileDescriptor *f);
+void mmap_cache_fd_free(MMapFileDescriptor *f);
 
 void mmap_cache_stats_log_debug(MMapCache *m);
 
-bool mmap_cache_got_sigbus(MMapCache *m, MMapFileDescriptor *f);
+bool mmap_cache_fd_got_sigbus(MMapFileDescriptor *f);
diff --git a/src/libsystemd/sd-journal/test-mmap-cache.c b/src/libsystemd/sd-journal/test-mmap-cache.c
index c3212fe179..6ec36931fd 100644
--- a/src/libsystemd/sd-journal/test-mmap-cache.c
+++ b/src/libsystemd/sd-journal/test-mmap-cache.c
@@ -34,28 +34,28 @@ int main(int argc, char *argv[]) {
         assert_se(z >= 0);
         unlink(pz);
 
-        r = mmap_cache_get(m, fx, 0, false, 1, 2, NULL, &p);
+        r = mmap_cache_fd_get(fx, 0, false, 1, 2, NULL, &p);
         assert_se(r >= 0);
 
-        r = mmap_cache_get(m, fx, 0, false, 2, 2, NULL, &q);
+        r = mmap_cache_fd_get(fx, 0, false, 2, 2, NULL, &q);
         assert_se(r >= 0);
 
         assert_se((uint8_t*) p + 1 == (uint8_t*) q);
 
-        r = mmap_cache_get(m, fx, 1, false, 3, 2, NULL, &q);
+        r = mmap_cache_fd_get(fx, 1, false, 3, 2, NULL, &q);
         assert_se(r >= 0);
 
         assert_se((uint8_t*) p + 2 == (uint8_t*) q);
 
-        r = mmap_cache_get(m, fx, 0, false, 16ULL*1024ULL*1024ULL, 2, NULL, &p);
+        r = mmap_cache_fd_get(fx, 0, false, 16ULL*1024ULL*1024ULL, 2, NULL, &p);
         assert_se(r >= 0);
 
-        r = mmap_cache_get(m, fx, 1, false, 16ULL*1024ULL*1024ULL+1, 2, NULL, &q);
+        r = mmap_cache_fd_get(fx, 1, false, 16ULL*1024ULL*1024ULL+1, 2, NULL, &q);
         assert_se(r >= 0);
 
         assert_se((uint8_t*) p + 1 == (uint8_t*) q);
 
-        mmap_cache_free_fd(m, fx);
+        mmap_cache_fd_free(fx);
         mmap_cache_unref(m);
 
         safe_close(x);