From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Thu, 28 Sep 2023 00:05:53 +0000 (+0900)
Subject: sd-journal: drop unnecessary re-read of object
X-Git-Tag: v255-rc1~344
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=31438511e072b412f973e6c9fcd60149fd2fbac7;p=thirdparty%2Fsystemd.git

sd-journal: drop unnecessary re-read of object

This reverts the following commits.
- a1640191b4ca583ca62a4bd3b91edec3532bd41f
- 231741d355fbbe544f6bf62d714f56a6c857fb6f

These were done by my misunderstanding of the mmap cache behavior.

Also, this updates the comments added by
df04b9ed86a8b45c25cfff0fd800adb66407309a.
---

diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c
index 1b80afe74dc..1d32d5cbc3a 100644
--- a/src/libsystemd/sd-journal/journal-file.c
+++ b/src/libsystemd/sd-journal/journal-file.c
@@ -838,9 +838,9 @@ static int journal_file_move_to(
         assert(f);
         assert(ret);
 
-        /* This function may clear, overwrite, or alter previously cached entries. After this function has
-         * been called, all objects except for one obtained by this function are invalidated and must be
-         * re-read before use. */
+        /* This function may clear, overwrite, or alter previously cached entries with the same type. After
+         * this function has been called, all previously read objects with the same type may be invalidated,
+         * hence must be re-read before use. */
 
         if (size <= 0)
                 return -EINVAL;
@@ -1088,9 +1088,9 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset
 
         assert(f);
 
-        /* Even if this function fails, it may clear, overwrite, or alter previously cached entries. After
-         * this function has been called, all objects except for one obtained by this function are
-         * invalidated and must be re-read before use.. */
+        /* Even if this function fails, it may clear, overwrite, or alter previously cached entries with the
+         * same type. After this function has been called, all previously read objects with the same type may
+         * be invalidated, hence must be re-read before use. */
 
         /* Objects may only be located at multiple of 64 bit */
         if (!VALID64(offset))
@@ -4341,7 +4341,7 @@ int journal_file_copy_entry(
                 r = journal_file_data_payload(from, NULL, q, NULL, 0, 0, &data, &l);
                 if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) {
                         log_debug_errno(r, "Entry item %"PRIu64" data object is bad, skipping over it: %m", i);
-                        goto next;
+                        continue;
                 }
                 if (r < 0)
                         return r;
@@ -4363,13 +4363,6 @@ int journal_file_copy_entry(
                         .object_offset = h,
                         .hash = le64toh(u->data.hash),
                 };
-
-        next:
-                /* The above journal_file_data_payload() may clear or overwrite cached object. Hence, we need
-                 * to re-read the object from the cache. */
-                r = journal_file_move_to_object(from, OBJECT_ENTRY, p, &o);
-                if (r < 0)
-                        return r;
         }
 
         if (m == 0)
diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c
index 3b8ca21725c..c61573f8606 100644
--- a/src/libsystemd/sd-journal/sd-journal.c
+++ b/src/libsystemd/sd-journal/sd-journal.c
@@ -2640,10 +2640,10 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void **
                 p = journal_file_entry_item_object_offset(f, o, i);
                 r = journal_file_data_payload(f, NULL, p, field, field_length, j->data_threshold, &d, &l);
                 if (r == 0)
-                        goto next;
+                        continue;
                 if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) {
                         log_debug_errno(r, "Entry item %"PRIu64" data object is bad, skipping over it: %m", i);
-                        goto next;
+                        continue;
                 }
                 if (r < 0)
                         return r;
@@ -2652,12 +2652,6 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void **
                 *size = l;
 
                 return 0;
-
-        next:
-                /* journal_file_data_payload() may clear or overwrite cached object. */
-                r = journal_file_move_to_object(f, OBJECT_ENTRY, f->current_offset, &o);
-                if (r < 0)
-                        return r;
         }
 
         return -ENOENT;
@@ -2693,7 +2687,7 @@ _public_ int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t
                 r = journal_file_data_payload(f, NULL, p, NULL, 0, j->data_threshold, &d, &l);
                 if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) {
                         log_debug_errno(r, "Entry item %"PRIu64" data object is bad, skipping over it: %m", j->current_field);
-                        goto next;
+                        continue;
                 }
                 if (r < 0)
                         return r;
@@ -2705,12 +2699,6 @@ _public_ int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t
                 j->current_field++;
 
                 return 1;
-
-        next:
-                /* journal_file_data_payload() may clear or overwrite cached object. */
-                r = journal_file_move_to_object(f, OBJECT_ENTRY, f->current_offset, &o);
-                if (r < 0)
-                        return r;
         }
 
         return 0;