From 8e3d775ad2ab1f6708b7d3d11d901354673a52fe Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Mon, 8 Jun 2026 06:11:51 -0500 Subject: [PATCH 1/7] Move `dir_init_segment` to `Directory` --- src/iocore/cache/CacheDir.cc | 16 ++++++++-------- src/iocore/cache/P_CacheDir.h | 7 ++++++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index ee14d521ff3..8acb4563177 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -210,16 +210,16 @@ dir_bucket_loop_check(Dir *start_dir, Dir *seg) // adds all the directory entries // in a segment to the segment freelist void -dir_init_segment(int s, Directory *directory) +Directory::init_segment(int s) { - directory->header->freelist[s] = 0; - Dir *seg = directory->get_segment(s); + this->header->freelist[s] = 0; + Dir *seg = this->get_segment(s); int l, b; - memset(static_cast(seg), 0, SIZEOF_DIR * DIR_DEPTH * directory->buckets); + memset(static_cast(seg), 0, SIZEOF_DIR * DIR_DEPTH * this->buckets); for (l = 1; l < DIR_DEPTH; l++) { - for (b = 0; b < directory->buckets; b++) { + for (b = 0; b < this->buckets; b++) { Dir *bucket = dir_bucket(b, seg); - directory->free_entry(dir_bucket_row(bucket, l), s); + this->free_entry(dir_bucket_row(bucket, l), s); } } } @@ -231,7 +231,7 @@ dir_bucket_loop_fix(Dir *start_dir, int s, Directory *directory) { if (!dir_bucket_loop_check(start_dir, directory->get_segment(s))) { Warning("Dir loop exists, clearing segment %d", s); - dir_init_segment(s, directory); + directory->init_segment(s); return 1; } return 0; @@ -462,7 +462,7 @@ freelist_pop(int s, StripeSM *stripe) stripe->directory.header->freelist[s] = dir_next(e); // if the freelist if bad, punt. if (dir_offset(e)) { - dir_init_segment(s, &stripe->directory); + stripe->directory->init_segment(s); return nullptr; } Dir *h = dir_from_offset(stripe->directory.header->freelist[s], seg); diff --git a/src/iocore/cache/P_CacheDir.h b/src/iocore/cache/P_CacheDir.h index fbd2f0b25bc..c1c4f4fa4a2 100644 --- a/src/iocore/cache/P_CacheDir.h +++ b/src/iocore/cache/P_CacheDir.h @@ -286,7 +286,9 @@ struct StripeHeaderFooter { uint16_t freelist[1]; }; -struct Directory { +class Directory +{ +public: char *raw_dir{nullptr}; Dir *dir{}; StripeHeaderFooter *header{}; @@ -316,6 +318,9 @@ struct Directory { int bucket_length(Dir *b, int s); int freelist_length(int s); void clean_segment(int s, StripeSM *stripe); + +private: + void init_segment(int s); }; inline int From 4db06ca1c0309cde30616276df305db6eae76183 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Mon, 8 Jun 2026 06:13:43 -0500 Subject: [PATCH 2/7] Move `dir_init_segment` to `Directory` --- src/iocore/cache/CacheDir.cc | 2 +- src/iocore/cache/P_CacheDir.h | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index 8acb4563177..d5bde262d75 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -462,7 +462,7 @@ freelist_pop(int s, StripeSM *stripe) stripe->directory.header->freelist[s] = dir_next(e); // if the freelist if bad, punt. if (dir_offset(e)) { - stripe->directory->init_segment(s); + stripe->directory.init_segment(s); return nullptr; } Dir *h = dir_from_offset(stripe->directory.header->freelist[s], seg); diff --git a/src/iocore/cache/P_CacheDir.h b/src/iocore/cache/P_CacheDir.h index c1c4f4fa4a2..063825453b7 100644 --- a/src/iocore/cache/P_CacheDir.h +++ b/src/iocore/cache/P_CacheDir.h @@ -286,9 +286,7 @@ struct StripeHeaderFooter { uint16_t freelist[1]; }; -class Directory -{ -public: +struct Directory { char *raw_dir{nullptr}; Dir *dir{}; StripeHeaderFooter *header{}; @@ -318,9 +316,7 @@ class Directory int bucket_length(Dir *b, int s); int freelist_length(int s); void clean_segment(int s, StripeSM *stripe); - -private: - void init_segment(int s); + void init_segment(int s); }; inline int From 8a5eb137c0901fbcaeee09d1480793db03189416 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Mon, 8 Jun 2026 06:17:54 -0500 Subject: [PATCH 3/7] Move `dir_bucket_loop_fix` to `Directory` --- src/iocore/cache/CacheDir.cc | 20 ++++++++++---------- src/iocore/cache/CacheVC.cc | 5 +---- src/iocore/cache/P_CacheDir.h | 1 + 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index d5bde262d75..0b4ecd76539 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -227,11 +227,11 @@ Directory::init_segment(int s) // break the infinite loop in directory entries // Note : abuse of the token bit in dir entries int -dir_bucket_loop_fix(Dir *start_dir, int s, Directory *directory) +Directory::bucket_loop_fix(Dir *start_dir, int s) { - if (!dir_bucket_loop_check(start_dir, directory->get_segment(s))) { + if (!dir_bucket_loop_check(start_dir, this->get_segment(s))) { Warning("Dir loop exists, clearing segment %d", s); - directory->init_segment(s); + this->init_segment(s); return 1; } return 0; @@ -243,7 +243,7 @@ Directory::freelist_length(int s) int free = 0; Dir *seg = this->get_segment(s); Dir *e = dir_from_offset(this->header->freelist[s], seg); - if (dir_bucket_loop_fix(e, s, this)) { + if (this->bucket_loop_fix(e, s)) { return (DIR_DEPTH - 1) * this->buckets; } while (e) { @@ -260,7 +260,7 @@ Directory::bucket_length(Dir *b, int s) int i = 0; Dir *seg = this->get_segment(s); #ifdef LOOP_CHECK_MODE - if (dir_bucket_loop_fix(b, s, this)) + if (this->bucket_loop_fix(b, s, this)) return 1; #endif while (e) { @@ -357,7 +357,7 @@ dir_clean_bucket(Dir *b, int s, StripeSM *stripe) #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD) { - if (dir_bucket_loop_fix(b, s, vol->directory)) + if (vol->directory.bucket_loop_fix(b, s)) return; } #endif @@ -495,7 +495,7 @@ Directory::probe(const CacheKey *key, StripeSM *stripe, Dir *result, Dir **last_ Dir *e = nullptr, *p = nullptr, *collision = *last_collision; CHECK_DIR(d); #ifdef LOOP_CHECK_MODE - if (dir_bucket_loop_fix(dir_bucket(b, seg), s, this)) + if (this->bucket_loop_fix(dir_bucket(b, seg), s)) return 0; #endif Lagain: @@ -653,7 +653,7 @@ Directory::overwrite(const CacheKey *key, StripeSM *stripe, Dir *dir, Dir *overw #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD && loop_possible) { - if (dir_bucket_loop_fix(b, s, this)) { + if (this->bucket_loop_fix(b, s)) { loop_possible = false; goto Lagain; } @@ -733,7 +733,7 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del) #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD) { - if (dir_bucket_loop_fix(dir_bucket(b, seg), s, this)) + if (this->bucket_loop_fix(dir_bucket(b, seg), s, this)) return 0; } #endif @@ -939,7 +939,7 @@ Directory::entries_used() sfull = 0; for (int b = 0; b < this->buckets; b++) { Dir *e = dir_bucket(b, seg); - if (dir_bucket_loop_fix(e, s, this)) { + if (this->bucket_loop_fix(e, s)) { sfull = 0; break; } diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index 54a7abc7397..821dce534a3 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -111,9 +111,6 @@ next_in_map(Stripe *stripe, char *vol_map, off_t offset) return new_off + start_offset; } -// Function in CacheDir.cc that we need for make_vol_map(). -int dir_bucket_loop_fix(Dir *start_dir, int s, Directory *directory); - // TODO: If we used a bit vector, we could make a smaller map structure. // TODO: If we saved a high water mark we could have a smaller buf, and avoid searching it // when we are asked about the highest interesting offset. @@ -137,7 +134,7 @@ make_vol_map(Stripe *stripe) Dir *seg = stripe->directory.get_segment(s); for (int b = 0; b < stripe->directory.buckets; b++) { Dir *e = dir_bucket(b, seg); - if (dir_bucket_loop_fix(e, s, &stripe->directory)) { + if (stripe->directory.bucket_loop_fix(e, s)) { break; } while (e) { diff --git a/src/iocore/cache/P_CacheDir.h b/src/iocore/cache/P_CacheDir.h index 063825453b7..d1cdbefd84b 100644 --- a/src/iocore/cache/P_CacheDir.h +++ b/src/iocore/cache/P_CacheDir.h @@ -317,6 +317,7 @@ struct Directory { int freelist_length(int s); void clean_segment(int s, StripeSM *stripe); void init_segment(int s); + int bucket_loop_fix(Dir *start_dir, int s); }; inline int From 0873953716a435985d50d777648846fff1535e52 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Mon, 8 Jun 2026 06:25:12 -0500 Subject: [PATCH 4/7] Move `unlink_from_freelist` to `Directory` --- src/iocore/cache/CacheDir.cc | 20 ++------------------ src/iocore/cache/P_CacheDir.h | 23 ++++++++++++++++++++++- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index 0b4ecd76539..eb95a9b4e7a 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -296,22 +296,6 @@ Directory::check() return 1; } -inline void -unlink_from_freelist(Dir *e, int s, Directory *directory) -{ - Dir *seg = directory->get_segment(s); - Dir *p = dir_from_offset(dir_prev(e), seg); - if (p) { - dir_set_next(p, dir_next(e)); - } else { - directory->header->freelist[s] = dir_next(e); - } - Dir *n = dir_from_offset(dir_next(e), seg); - if (n) { - dir_set_prev(n, dir_prev(e)); - } -} - inline Dir * dir_delete_entry(Dir *e, Dir *p, int s, Directory *directory) { @@ -585,7 +569,7 @@ Directory::insert(const CacheKey *key, StripeSM *stripe, Dir *to_part) for (l = 1; l < DIR_DEPTH; l++) { e = dir_bucket_row(b, l); if (dir_is_empty(e)) { - unlink_from_freelist(e, s, this); + this->unlink_from_freelist(e, s); goto Llink; } } @@ -679,7 +663,7 @@ Directory::overwrite(const CacheKey *key, StripeSM *stripe, Dir *dir, Dir *overw for (l = 1; l < DIR_DEPTH; l++) { e = dir_bucket_row(b, l); if (dir_is_empty(e)) { - unlink_from_freelist(e, s, this); + this->unlink_from_freelist(e, s); goto Llink; } } diff --git a/src/iocore/cache/P_CacheDir.h b/src/iocore/cache/P_CacheDir.h index d1cdbefd84b..2174a9f7b62 100644 --- a/src/iocore/cache/P_CacheDir.h +++ b/src/iocore/cache/P_CacheDir.h @@ -286,7 +286,9 @@ struct StripeHeaderFooter { uint16_t freelist[1]; }; -struct Directory { +class Directory +{ +public: char *raw_dir{nullptr}; Dir *dir{}; StripeHeaderFooter *header{}; @@ -318,6 +320,9 @@ struct Directory { void clean_segment(int s, StripeSM *stripe); void init_segment(int s); int bucket_loop_fix(Dir *start_dir, int s); + +private: + void unlink_from_freelist(Dir *e, int s); }; inline int @@ -366,6 +371,22 @@ dir_from_offset(int64_t i, Dir *seg) #endif } +inline void +Directory::unlink_from_freelist(Dir *e, int s) +{ + Dir *seg = this->get_segment(s); + Dir *p = dir_from_offset(dir_prev(e), seg); + if (p) { + dir_set_next(p, dir_next(e)); + } else { + this->header->freelist[s] = dir_next(e); + } + Dir *n = dir_from_offset(dir_next(e), seg); + if (n) { + dir_set_prev(n, dir_prev(e)); + } +} + inline Dir * next_dir(Dir *d, Dir *seg) { From f8f97d9a8f138c7b99b7e4afd92dd960445bea70 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Mon, 8 Jun 2026 06:34:45 -0500 Subject: [PATCH 5/7] Move `dir_delete_entry` to `Directory` --- src/iocore/cache/CacheDir.cc | 43 ++-------------- src/iocore/cache/P_CacheDir.h | 92 ++++++++++++++++++++++++----------- 2 files changed, 69 insertions(+), 66 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index eb95a9b4e7a..f935820aa15 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -296,39 +296,6 @@ Directory::check() return 1; } -inline Dir * -dir_delete_entry(Dir *e, Dir *p, int s, Directory *directory) -{ - Dir *seg = directory->get_segment(s); - int no = dir_next(e); - directory->header->dirty = 1; - if (p) { - unsigned int fo = directory->header->freelist[s]; - unsigned int eo = dir_to_offset(e, seg); - dir_clear(e); - dir_set_next(p, no); - dir_set_next(e, fo); - if (fo) { - dir_set_prev(dir_from_offset(fo, seg), eo); - } - directory->header->freelist[s] = eo; - } else { - Dir *n = next_dir(e, seg); - if (n) { - // "Shuffle" here means that we're copying the second entry's data to the head entry's location, and removing the second entry - // - because the head entry can't be moved. - ATS_PROBE3(cache_dir_shuffle, s, dir_to_offset(e, seg), dir_to_offset(n, seg)); - dir_assign(e, n); - dir_delete_entry(n, e, s, directory); - return e; - } else { - dir_clear(e); - return nullptr; - } - } - return dir_from_offset(no, seg); -} - inline void dir_clean_bucket(Dir *b, int s, StripeSM *stripe) { @@ -356,7 +323,7 @@ dir_clean_bucket(Dir *b, int s, StripeSM *stripe) } // Match cache_dir_remove arguments ATS_PROBE7(cache_dir_remove_clean_bucket, stripe->fd, s, dir_to_offset(e, seg), dir_offset(e), dir_approx_size(e), 0, 0); - e = dir_delete_entry(e, p, s, &stripe->directory); + e = stripe->directory.delete_entry(e, p, s); continue; } p = e; @@ -490,7 +457,7 @@ Directory::probe(const CacheKey *key, StripeSM *stripe, Dir *result, Dir **last_ ink_assert(dir_offset(e)); // Bug: 51680. Need to check collision before checking // dir_valid(). In case of a collision, if !dir_valid(), we - // don't want to call dir_delete_entry. + // don't want to call Directory::delete_entry. if (collision) { if (collision == e) { collision = nullptr; @@ -517,7 +484,7 @@ Directory::probe(const CacheKey *key, StripeSM *stripe, Dir *result, Dir **last_ ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used); ATS_PROBE7(cache_dir_remove_invalid, stripe->fd, s, dir_to_offset(e, seg), dir_offset(e), dir_approx_size(e), key->slice64(0), key->slice64(1)); - e = dir_delete_entry(e, p, s, this); + e = this->delete_entry(e, p, s); continue; } } else { @@ -727,7 +694,7 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del) ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used); ATS_PROBE7(cache_dir_remove, stripe->fd, s, dir_to_offset(e, seg), offset, dir_approx_size(e), key->slice64(0), key->slice64(1)); - dir_delete_entry(e, p, s, this); + this->delete_entry(e, p, s); CHECK_DIR(d); return 1; } @@ -1052,7 +1019,7 @@ CacheSync::mainEvent(int event, Event * /* e ATS_UNUSED */) /* Don't sync the directory to disk if its not dirty. Syncing the clean directory to disk is also the cause of INKqa07151. Increasing the serial number causes the cache to recover more data than necessary. - The dirty bit it set in dir_insert, overwrite and dir_delete_entry + The dirty bit it set in dir_insert, overwrite and Directory::delete_entry */ if (!stripe->directory.header->dirty) { Dbg(dbg_ctl_cache_dir_sync, "Dir %s not dirty", stripe->hash_text.get()); diff --git a/src/iocore/cache/P_CacheDir.h b/src/iocore/cache/P_CacheDir.h index 2174a9f7b62..0a9150f4832 100644 --- a/src/iocore/cache/P_CacheDir.h +++ b/src/iocore/cache/P_CacheDir.h @@ -30,6 +30,8 @@ #include "tscore/Version.h" #include "tscore/hugepages.h" +#include + #include #include @@ -320,23 +322,12 @@ class Directory void clean_segment(int s, StripeSM *stripe); void init_segment(int s); int bucket_loop_fix(Dir *start_dir, int s); + Dir *delete_entry(Dir *e, Dir *p, int s); private: void unlink_from_freelist(Dir *e, int s); }; -inline int -Directory::entries() const -{ - return this->buckets * DIR_DEPTH * this->segments; -} - -inline Dir * -Directory::get_segment(int s) const -{ - return reinterpret_cast((reinterpret_cast(this->dir)) + (s * this->buckets) * DIR_DEPTH * SIZEOF_DIR); -} - // Global Functions int dir_lookaside_probe(const CacheKey *key, StripeSM *stripe, Dir *result, EvacuationBlock **eblock); @@ -371,22 +362,6 @@ dir_from_offset(int64_t i, Dir *seg) #endif } -inline void -Directory::unlink_from_freelist(Dir *e, int s) -{ - Dir *seg = this->get_segment(s); - Dir *p = dir_from_offset(dir_prev(e), seg); - if (p) { - dir_set_next(p, dir_next(e)); - } else { - this->header->freelist[s] = dir_next(e); - } - Dir *n = dir_from_offset(dir_next(e), seg); - if (n) { - dir_set_prev(n, dir_prev(e)); - } -} - inline Dir * next_dir(Dir *d, Dir *seg) { @@ -417,3 +392,64 @@ dir_bucket_row(Dir *b, int64_t i) { return dir_in_seg(b, i); } + +inline int +Directory::entries() const +{ + return this->buckets * DIR_DEPTH * this->segments; +} + +inline Dir * +Directory::get_segment(int s) const +{ + return reinterpret_cast((reinterpret_cast(this->dir)) + (s * this->buckets) * DIR_DEPTH * SIZEOF_DIR); +} + +inline void +Directory::unlink_from_freelist(Dir *e, int s) +{ + Dir *seg = this->get_segment(s); + Dir *p = dir_from_offset(dir_prev(e), seg); + if (p) { + dir_set_next(p, dir_next(e)); + } else { + this->header->freelist[s] = dir_next(e); + } + Dir *n = dir_from_offset(dir_next(e), seg); + if (n) { + dir_set_prev(n, dir_prev(e)); + } +} + +inline Dir * +Directory::delete_entry(Dir *e, Dir *p, int s) +{ + Dir *seg = this->get_segment(s); + int no = dir_next(e); + this->header->dirty = 1; + if (p) { + unsigned int fo = this->header->freelist[s]; + unsigned int eo = dir_to_offset(e, seg); + dir_clear(e); + dir_set_next(p, no); + dir_set_next(e, fo); + if (fo) { + dir_set_prev(dir_from_offset(fo, seg), eo); + } + this->header->freelist[s] = eo; + } else { + Dir *n = next_dir(e, seg); + if (n) { + // "Shuffle" here means that we're copying the second entry's data to the head entry's location, and removing the second entry + // - because the head entry can't be moved. + ATS_PROBE3(cache_dir_shuffle, s, dir_to_offset(e, seg), dir_to_offset(n, seg)); + dir_assign(e, n); + this->delete_entry(n, e, s); + return e; + } else { + dir_clear(e); + return nullptr; + } + } + return dir_from_offset(no, seg); +} From db623da64e19ad364d5fc167a3780bfb113f71b4 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:44:49 -0500 Subject: [PATCH 6/7] Fix comment typo Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/iocore/cache/CacheDir.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index f935820aa15..891eec9cde5 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -1019,7 +1019,7 @@ CacheSync::mainEvent(int event, Event * /* e ATS_UNUSED */) /* Don't sync the directory to disk if its not dirty. Syncing the clean directory to disk is also the cause of INKqa07151. Increasing the serial number causes the cache to recover more data than necessary. - The dirty bit it set in dir_insert, overwrite and Directory::delete_entry + The dirty bit is set in dir_insert, overwrite and Directory::delete_entry */ if (!stripe->directory.header->dirty) { Dbg(dbg_ctl_cache_dir_sync, "Dir %s not dirty", stripe->hash_text.get()); From ef5c46585c24c2fcc168b404c5dbce7b5516c62e Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Mon, 8 Jun 2026 12:47:17 -0500 Subject: [PATCH 7/7] Make changes suggested by Copilot * Fix conditionally compiled `bucket_loop_fix` callsites --- src/iocore/cache/CacheDir.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index 891eec9cde5..7997c501086 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -260,7 +260,7 @@ Directory::bucket_length(Dir *b, int s) int i = 0; Dir *seg = this->get_segment(s); #ifdef LOOP_CHECK_MODE - if (this->bucket_loop_fix(b, s, this)) + if (this->bucket_loop_fix(b, s)) return 1; #endif while (e) { @@ -684,7 +684,7 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del) #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD) { - if (this->bucket_loop_fix(dir_bucket(b, seg), s, this)) + if (this->bucket_loop_fix(dir_bucket(b, seg), s)) return 0; } #endif