From: Vito Caputo <vcaputo@pengaru.com>
Date: Tue, 31 Aug 2021 01:20:53 +0000 (-0700)
Subject: sd-journal: use FILE streams to buffer write_uint64()
X-Git-Tag: v250-rc1~736
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a4121e965f43bc323ec3c0ac212cee9d339f7be0;p=thirdparty%2Fsystemd.git

sd-journal: use FILE streams to buffer write_uint64()

journal_file_verify() uses a set of tmpfs files to create lists
of object positions by type.

The existing code used a bare write() call for every object
position written, incurring a syscall per listed object.

This commit encapsulates the bare file descriptors in FILE *'s
and replaces the bare write with fwrite, buffering the writes so
there's less syscalls.

Cached `journalctl --verify` tests showed a ~8% faster runtime
with this change on a release build, verifying 1.3GiB of
production journals across 16 files.
---

diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c
index 211fc2ed858..dbea50fd705 100644
--- a/src/libsystemd/sd-journal/journal-verify.c
+++ b/src/libsystemd/sd-journal/journal-verify.c
@@ -380,14 +380,9 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o
         return 0;
 }
 
-static int write_uint64(int fd, uint64_t p) {
-        ssize_t k;
-
-        k = write(fd, &p, sizeof(p));
-        if (k < 0)
+static int write_uint64(FILE *fp, uint64_t p) {
+        if (fwrite(&p, sizeof(p), 1, fp) != 1)
                 return -errno;
-        if (k != sizeof(p))
-                return -EIO;
 
         return 0;
 }
@@ -843,7 +838,8 @@ int journal_file_verify(
         bool entry_seqnum_set = false, entry_monotonic_set = false, entry_realtime_set = false, found_main_entry_array = false;
         uint64_t n_weird = 0, n_objects = 0, n_entries = 0, n_data = 0, n_fields = 0, n_data_hash_tables = 0, n_field_hash_tables = 0, n_entry_arrays = 0, n_tags = 0;
         usec_t last_usec = 0;
-        int data_fd = -1, entry_fd = -1, entry_array_fd = -1;
+        _cleanup_close_ int data_fd = -1, entry_fd = -1, entry_array_fd = -1;
+        _cleanup_fclose_ FILE *data_fp = NULL, *entry_fp = NULL, *entry_array_fp = NULL;
         MMapFileDescriptor *cache_data_fd = NULL, *cache_entry_fd = NULL, *cache_entry_array_fd = NULL;
         unsigned i;
         bool found_last = false;
@@ -910,6 +906,24 @@ int journal_file_verify(
                 goto fail;
         }
 
+        r = take_fdopen_unlocked(&data_fd, "w+", &data_fp);
+        if (r < 0) {
+                log_error_errno(r, "Failed to open data file stream: %m");
+                goto fail;
+        }
+
+        r = take_fdopen_unlocked(&entry_fd, "w+", &entry_fp);
+        if (r < 0) {
+                log_error_errno(r, "Failed to open entry file stream: %m");
+                goto fail;
+        }
+
+        r = take_fdopen_unlocked(&entry_array_fd, "w+", &entry_array_fp);
+        if (r < 0) {
+                log_error_errno(r, "Failed to open entry array file stream: %m");
+                goto fail;
+        }
+
         if (le32toh(f->header->compatible_flags) & ~HEADER_COMPATIBLE_SUPPORTED) {
                 log_error("Cannot verify file with unknown extensions.");
                 r = -EOPNOTSUPP;
@@ -984,7 +998,7 @@ int journal_file_verify(
                 switch (o->object.type) {
 
                 case OBJECT_DATA:
-                        r = write_uint64(data_fd, p);
+                        r = write_uint64(data_fp, p);
                         if (r < 0)
                                 goto fail;
 
@@ -1002,7 +1016,7 @@ int journal_file_verify(
                                 goto fail;
                         }
 
-                        r = write_uint64(entry_fd, p);
+                        r = write_uint64(entry_fp, p);
                         if (r < 0)
                                 goto fail;
 
@@ -1089,7 +1103,7 @@ int journal_file_verify(
                         break;
 
                 case OBJECT_ENTRY_ARRAY:
-                        r = write_uint64(entry_array_fd, p);
+                        r = write_uint64(entry_array_fp, p);
                         if (r < 0)
                                 goto fail;
 
@@ -1279,6 +1293,21 @@ int journal_file_verify(
                 goto fail;
         }
 
+        if (fflush(data_fp) != 0) {
+                r = log_error_errno(errno, "Failed to flush data file stream: %m");
+                goto fail;
+        }
+
+        if (fflush(entry_fp) != 0) {
+                r = log_error_errno(errno, "Failed to flush entry file stream: %m");
+                goto fail;
+        }
+
+        if (fflush(entry_array_fp) != 0) {
+                r = log_error_errno(errno, "Failed to flush entry array file stream: %m");
+                goto fail;
+        }
+
         /* Second iteration: we follow all objects referenced from the
          * two entry points: the object hash table and the entry
          * array. We also check that everything referenced (directly
@@ -1312,10 +1341,6 @@ int journal_file_verify(
         mmap_cache_free_fd(f->mmap, cache_entry_fd);
         mmap_cache_free_fd(f->mmap, cache_entry_array_fd);
 
-        safe_close(data_fd);
-        safe_close(entry_fd);
-        safe_close(entry_array_fd);
-
         if (first_contained)
                 *first_contained = le64toh(f->header->head_entry_realtime);
         if (last_validated)
@@ -1335,15 +1360,6 @@ fail:
                   (unsigned long long) f->last_stat.st_size,
                   100 * p / f->last_stat.st_size);
 
-        if (data_fd >= 0)
-                safe_close(data_fd);
-
-        if (entry_fd >= 0)
-                safe_close(entry_fd);
-
-        if (entry_array_fd >= 0)
-                safe_close(entry_array_fd);
-
         if (cache_data_fd)
                 mmap_cache_free_fd(f->mmap, cache_data_fd);