From 0096914023012a84c1827aa0194067f0a575acd7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 27 Sep 2023 12:02:25 +0900 Subject: [PATCH 1/5] sd-journal: drop unused argument for generic_array_bisect_plus_one() --- src/libsystemd/sd-journal/journal-file.c | 25 +++++++++--------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 45971d3944..175a87def8 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3104,8 +3104,7 @@ static int generic_array_bisect_plus_one( int (*test_object)(JournalFile *f, uint64_t p, uint64_t needle), direction_t direction, Object **ret_object, - uint64_t *ret_offset, - uint64_t *ret_idx) { + uint64_t *ret_offset) { int r; bool step_back = false; @@ -3140,14 +3139,11 @@ static int generic_array_bisect_plus_one( return 0; } - r = generic_array_bisect(f, first, n-1, needle, test_object, direction, ret_object, ret_offset, ret_idx); + r = generic_array_bisect(f, first, n-1, needle, test_object, direction, ret_object, ret_offset, NULL); if (r == 0 && step_back) goto found; - if (r > 0 && ret_idx) - (*ret_idx)++; - return r; found: @@ -3160,9 +3156,6 @@ found: if (ret_offset) *ret_offset = extra; - if (ret_idx) - *ret_idx = 0; - return 1; } @@ -3340,7 +3333,7 @@ int journal_file_move_to_entry_by_monotonic( monotonic, test_object_monotonic, direction, - ret_object, ret_offset, NULL); + ret_object, ret_offset); } void journal_file_reset_location(JournalFile *f) { @@ -3523,7 +3516,7 @@ int journal_file_move_to_entry_by_offset_for_data( p, test_object_offset, direction, - ret, ret_offset, NULL); + ret, ret_offset); } int journal_file_move_to_entry_by_monotonic_for_data( @@ -3560,7 +3553,7 @@ int journal_file_move_to_entry_by_monotonic_for_data( monotonic, test_object_monotonic, direction, - NULL, &z, NULL); + NULL, &z); if (r <= 0) return r; @@ -3581,7 +3574,7 @@ int journal_file_move_to_entry_by_monotonic_for_data( z, test_object_offset, direction, - NULL, &p, NULL); + NULL, &p); if (r <= 0) return r; @@ -3592,7 +3585,7 @@ int journal_file_move_to_entry_by_monotonic_for_data( p, test_object_offset, direction, - NULL, &q, NULL); + NULL, &q); if (r <= 0) return r; @@ -3634,7 +3627,7 @@ int journal_file_move_to_entry_by_seqnum_for_data( seqnum, test_object_seqnum, direction, - ret_object, ret_offset, NULL); + ret_object, ret_offset); } int journal_file_move_to_entry_by_realtime_for_data( @@ -3656,7 +3649,7 @@ int journal_file_move_to_entry_by_realtime_for_data( realtime, test_object_realtime, direction, - ret, ret_offset, NULL); + ret, ret_offset); } void journal_file_dump(JournalFile *f) { From d67078b4a39645a2167259d641abd26340ff8d08 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 27 Sep 2023 12:47:35 +0900 Subject: [PATCH 2/5] sd-journal: rewrite conditions for test result and direction No functional change, just refactoring. Hopefully the condition is more descriptive now. --- src/libsystemd/sd-journal/journal-file.c | 57 +++++++++++++++--------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 175a87def8..af946e6cad 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3107,7 +3107,6 @@ static int generic_array_bisect_plus_one( uint64_t *ret_offset) { int r; - bool step_back = false; assert(f); assert(test_object); @@ -3115,38 +3114,52 @@ static int generic_array_bisect_plus_one( if (n <= 0) return 0; - /* This bisects the array in object 'first', but first checks - * an extra */ + /* This bisects the array in object 'first', but first checks an extra. */ r = test_object(f, extra, needle); if (r < 0) return r; - if (r == TEST_FOUND) - r = direction == DIRECTION_DOWN ? TEST_RIGHT : TEST_LEFT; + if (direction == DIRECTION_DOWN) { + /* If we are going downwards, then we need to return the first object that passes the test. + * When there is no object that passes the test, we need to return the first object that + * test_object() returns TEST_RIGHT for. */ + if (IN_SET(r, + TEST_FOUND, /* The 'extra' object passes the test. Hence, this is the first + * object that passes the test. */ + TEST_RIGHT)) /* The 'extra' object is the first object that test_object() returns + * TEST_RIGHT for, and no object exists even in the chained arrays + * that passes the test. */ + goto use_extra; /* The 'extra' object is exactly the one we are looking for. It is + * not necessary to bisect the chained arrays. */ - /* if we are looking with DIRECTION_UP then we need to first - see if in the actual array there is a matching entry, and - return the last one of that. But if there isn't any we need - to return this one. Hence remember this, and return it - below. */ - if (r == TEST_LEFT) - step_back = direction == DIRECTION_UP; + /* Otherwise, the 'extra' object is not the one we are looking for. Search in the arrays. */ - if (r == TEST_RIGHT) { - if (direction == DIRECTION_DOWN) - goto found; - else - return 0; + } else { + /* If we are going upwards, then we need to return the last object that passes the test. + * When there is no object that passes the test, we need to return the the last object that + * test_object() returns TEST_LEFT for. */ + if (r == TEST_RIGHT) + return 0; /* Not only the 'extra' object, but also all objects in the chained arrays + * will never get TEST_FOUND or TEST_LEFT. The object we are looking for + * does not exist. */ + + /* Even if the 'extra' object passes the test, there may be multiple objects in the arrays + * that also pass the test. Hence, we need to bisect the arrays for finding the last matching + * object. */ } r = generic_array_bisect(f, first, n-1, needle, test_object, direction, ret_object, ret_offset, NULL); + if (r != 0) + return r; /* When > 0, the found object is the first (or last, when DIRECTION_UP) object. + * Hence, return the found object now. */ - if (r == 0 && step_back) - goto found; + /* No matching object found in the chained arrays. + * DIRECTION_DOWN : the 'extra' object neither matches the condition. There is no matching object. + * DIRECTION_UP : the 'extra' object matches the condition. So, return it. */ + if (direction == DIRECTION_DOWN) + return 0; - return r; - -found: +use_extra: if (ret_object) { r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, ret_object); if (r < 0) From 20c45e578488c1d67f44eac45c168c23331fa19f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Sep 2023 02:42:00 +0900 Subject: [PATCH 3/5] sd-journal: drop unnecessary re-read of data object Follow-up for ec50313d4e329de276240883d86d05168a4cf09f. The function generic_array_bisect_plus_one() does not read any new data objects, so the data object is still valid, and not necessary to re-read it. --- src/libsystemd/sd-journal/journal-file.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index af946e6cad..05bdac9ed4 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3541,7 +3541,7 @@ int journal_file_move_to_entry_by_monotonic_for_data( Object **ret_object, uint64_t *ret_offset) { - uint64_t b, z, entry_offset, entry_array_offset, n_entries; + uint64_t z, entry_offset, entry_array_offset, n_entries; Object *o; int r; @@ -3555,7 +3555,7 @@ int journal_file_move_to_entry_by_monotonic_for_data( n_entries = le64toh(READ_NOW(d->data.n_entries)); /* First, seek by time */ - r = find_data_object_by_boot_id(f, boot_id, &o, &b); + r = find_data_object_by_boot_id(f, boot_id, &o, NULL); if (r <= 0) return r; @@ -3572,11 +3572,6 @@ int journal_file_move_to_entry_by_monotonic_for_data( /* And now, continue seeking until we find an entry that * exists in both bisection arrays */ - - r = journal_file_move_to_object(f, OBJECT_DATA, b, &o); - if (r < 0) - return r; - for (;;) { uint64_t p, q; From 80f96c0ce1510d1ad4f8e0699d1fbc8958777812 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Sep 2023 04:15:04 +0900 Subject: [PATCH 4/5] sd-journal: reduce number of calls generic_array_bisect_plus_one() If the first call of generic_array_bisect_plus_one() provides the same offset, then it is not necessary to call the next one, as we already know the entry object is also liked to the input data object. Also, this make the function reuse the object returned by generic_array_bisect_plus_one(). No functional change, just optimization. --- src/libsystemd/sd-journal/journal-file.c | 44 +++++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 05bdac9ed4..750f05d80b 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3542,7 +3542,7 @@ int journal_file_move_to_entry_by_monotonic_for_data( uint64_t *ret_offset) { uint64_t z, entry_offset, entry_array_offset, n_entries; - Object *o; + Object *o, *entry; int r; assert(f); @@ -3570,10 +3570,12 @@ int journal_file_move_to_entry_by_monotonic_for_data( if (r <= 0) return r; - /* And now, continue seeking until we find an entry that - * exists in both bisection arrays */ + /* And now, continue seeking until we find an entry that exists in both bisection arrays. */ for (;;) { - uint64_t p, q; + uint64_t p; + + /* The journal entry found by the above bisect_plus_one() may not have the specified data, + * that is, it may not be linked in the data object. So, we need to check that. */ r = generic_array_bisect_plus_one(f, entry_offset, @@ -3582,9 +3584,15 @@ int journal_file_move_to_entry_by_monotonic_for_data( z, test_object_offset, direction, - NULL, &p); + ret_object ? &entry : NULL, &p); if (r <= 0) return r; + if (p == z) + break; /* The journal entry has the specified data. Yay! */ + + /* If the entry does not have the data, then move to the next (or previous, depends on the + * 'direction') entry linked to the data object. But, the next entry may be in another boot. + * So, we need to check that the entry has the matching boot ID. */ r = generic_array_bisect_plus_one(f, le64toh(o->data.entry_offset), @@ -3593,26 +3601,20 @@ int journal_file_move_to_entry_by_monotonic_for_data( p, test_object_offset, direction, - NULL, &q); - + ret_object ? &entry : NULL, &z); if (r <= 0) return r; + if (p == z) + break; /* The journal entry has the specified boot ID. Yay! */ - if (p == q) { - if (ret_object) { - r = journal_file_move_to_object(f, OBJECT_ENTRY, q, ret_object); - if (r < 0) - return r; - } - - if (ret_offset) - *ret_offset = q; - - return 1; - } - - z = q; + /* If not, let's try to the next entry... */ } + + if (ret_object) + *ret_object = entry; + if (ret_offset) + *ret_offset = z; + return 1; } int journal_file_move_to_entry_by_seqnum_for_data( From 77db35bf10c39861a81a5c35aeff1260342f68d2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Sep 2023 04:55:21 +0900 Subject: [PATCH 5/5] sd-journal: make journal_file_next_entry() always return the nearest entry object Previously, if the input offset 'p' does not point to an entry object, the function returns the next of the nearest entry object on DIRECTION_DOWN, as generic_array_bisect() already returns the nearest entry object. --- src/libsystemd/sd-journal/journal-file.c | 58 +++++++++++++++--------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 750f05d80b..5982938fb9 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3397,7 +3397,8 @@ int journal_file_next_entry( Object **ret_object, uint64_t *ret_offset) { - uint64_t i, n, ofs; + uint64_t i, n, q; + Object *o; int r; assert(f); @@ -3409,38 +3410,53 @@ int journal_file_next_entry( if (n <= 0) return 0; + /* When the input offset 'p' is zero, return the first (or last on DIRECTION_UP) entry. */ if (p == 0) - i = direction == DIRECTION_DOWN ? 0 : n - 1; - else { - r = generic_array_bisect(f, + return generic_array_get(f, le64toh(f->header->entry_array_offset), - le64toh(f->header->n_entries), - p, - test_object_offset, - DIRECTION_DOWN, - NULL, NULL, - &i); - if (r <= 0) - return r; + direction == DIRECTION_DOWN ? 0 : n - 1, + direction, + ret_object, ret_offset); - r = bump_array_index(&i, direction, n); - if (r <= 0) - return r; - } + /* Otherwise, first find the nearest entry object. */ + r = generic_array_bisect(f, + le64toh(f->header->entry_array_offset), + le64toh(f->header->n_entries), + p, + test_object_offset, + direction, + ret_object ? &o : NULL, &q, &i); + if (r <= 0) + return r; + + assert(direction == DIRECTION_DOWN ? p <= q : q <= p); + + /* If the input offset 'p' points to an entry object, generic_array_bisect() should provides + * the same offset, and the index needs to be shifted. Otherwise, use the found object as is, + * as it is the nearest entry object from the input offset 'p'. */ + + if (p != q) + goto found; + + r = bump_array_index(&i, direction, n); + if (r <= 0) + return r; /* And jump to it */ - r = generic_array_get(f, le64toh(f->header->entry_array_offset), i, direction, ret_object, &ofs); + r = generic_array_get(f, le64toh(f->header->entry_array_offset), i, direction, ret_object ? &o : NULL, &q); if (r <= 0) return r; /* Ensure our array is properly ordered. */ - if (p > 0 && !check_properly_ordered(ofs, p, direction)) + if (!check_properly_ordered(q, p, direction)) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), - "%s: entry array not properly ordered at entry %" PRIu64, + "%s: entry array not properly ordered at entry index %" PRIu64, f->path, i); - +found: + if (ret_object) + *ret_object = o; if (ret_offset) - *ret_offset = ofs; + *ret_offset = q; return 1; }