From 0c3564298b32242b22197ff3e732af24b368013e Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 15 May 2026 15:33:05 -0500 Subject: [PATCH 01/38] Convert ink_queue implementation to std::atomic --- include/tscore/ink_queue.h | 113 +++++++--- src/proxy/logging/LogObject.cc | 6 +- src/tscore/CMakeLists.txt | 10 +- src/tscore/ink_queue.cc | 279 +++++++++++++++--------- src/tscore/unit_tests/test_ink_queue.cc | 243 +++++++++++++++++++++ 5 files changed, 506 insertions(+), 145 deletions(-) create mode 100644 src/tscore/unit_tests/test_ink_queue.cc diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index a0492213df3..52e905dfbbe 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -37,6 +37,10 @@ #include "tscore/ink_defs.h" #include "tscore/ink_apidefs.h" +#include +#include +#include + /* For information on the structure of the x86_64 memory map: @@ -71,30 +75,62 @@ void ink_queue_load_64(void *dst, void *src); /* * Generic Free List Manager */ -// Warning: head_p is read and written in multiple threads without a -// lock, use INK_QUEUE_LD to read safely. -union head_p { - head_p() : data(){}; - #if (defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4) - typedef int32_t version_type; - typedef int64_t data_type; +typedef int32_t head_p_version_type; +typedef int64_t head_p_data_type; #elif TS_HAS_128BIT_CAS - typedef int64_t version_type; - typedef __int128_t data_type; +typedef int64_t head_p_version_type; +typedef __int128_t head_p_data_type; #else - using version_type = int64_t; - using data_type = int64_t; +using head_p_version_type = int64_t; +using head_p_data_type = int64_t; #endif - struct { - void *pointer; - version_type version; - } s; +// Warning: head_p is read and written in multiple threads without a +// lock, use INK_QUEUE_LD to read safely. +using head_p = head_p_data_type; + +#if ((defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4)) || TS_HAS_128BIT_CAS - data_type data; +struct head_p_view { + void *pointer; + head_p_version_type version; }; +#elif TS_HAS_128BIT_CAS + +struct head_p_view { + void *pointer; + head_p_version_type version; +}; + +#elif defined(__x86_64__) || defined(__ia64__) || defined(__powerpc64__) || defined(__mips64) + +struct head_p_view { + int vaddr : 48; + int version : 15; + int vaddr_mode : 1; +}; +#endif + +inline head_p_view +load_head(head_p const &src) +{ + head_p_view result; + static_assert(sizeof(result) == sizeof(src)); + std::memcpy(&result, &src, sizeof(result)); + return result; +} + +#include + +inline void +store_head(head_p &dest, head_p_view const src) +{ + static_assert(sizeof(dest) == sizeof(src)); + std::memcpy(&dest, &src, sizeof(dest)); +} + /* * Why is version required? One scenario is described below * Think of a list like this -> A -> C -> D @@ -119,17 +155,13 @@ union head_p { #endif #if (defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4) -#define FREELIST_POINTER(_x) (_x).s.pointer -#define FREELIST_VERSION(_x) (_x).s.version -#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) \ - (_x).s.pointer = _p; \ - (_x).s.version = _v +#define FREELIST_POINTER(_x) load_head((_x)).pointer +#define FREELIST_VERSION(_x) load_head((_x)).version +#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) store_head((_x), head_p_view{(_p), (_v)}) #elif TS_HAS_128BIT_CAS -#define FREELIST_POINTER(_x) (_x).s.pointer -#define FREELIST_VERSION(_x) (_x).s.version -#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) \ - (_x).s.pointer = _p; \ - (_x).s.version = _v +#define FREELIST_POINTER(_x) load_head((_x)).pointer +#define FREELIST_VERSION(_x) load_head((_x)).version +#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) store_head((_x), head_p_view{(_p), (_v)}) #elif defined(__x86_64__) || defined(__ia64__) || defined(__powerpc64__) || defined(__mips64) /* Layout of FREELIST_POINTER * @@ -178,13 +210,21 @@ union head_p { #endif struct _InkFreeList { - head_p head; - const char *name; - uint32_t type_size, chunk_size, used, allocated, alignment; - uint32_t allocated_base, used_base; - uint32_t hugepages_failure; - bool use_hugepages; - int advice; + std::mutex m; + std::atomic head; + const char *name; + std::atomic used; + std::atomic allocated; + + // These fields must be initialized once and not modified after + // initialization. + uint32_t type_size, chunk_size, alignment; + + std::atomic allocated_base; + std::atomic used_base; + std::atomic hugepages_failure; + bool use_hugepages; + int advice; }; using InkFreeListOps = struct ink_freelist_ops; @@ -213,9 +253,10 @@ void ink_freelists_snap_baseline(); struct InkAtomicList { InkAtomicList() {} - head_p head{}; - const char *name = nullptr; - uint32_t offset = 0; + std::mutex m; + std::atomic head{}; + const char *name = nullptr; + uint32_t offset = 0; }; #if !defined(INK_QUEUE_NT) diff --git a/src/proxy/logging/LogObject.cc b/src/proxy/logging/LogObject.cc index 03e1316a17c..336379d4e6a 100644 --- a/src/proxy/logging/LogObject.cc +++ b/src/proxy/logging/LogObject.cc @@ -324,18 +324,18 @@ increment_pointer_version(head_p *dst) do { INK_QUEUE_LD(h, *dst); SET_FREELIST_POINTER_VERSION(new_h, FREELIST_POINTER(h), FREELIST_VERSION(h) + 1); - } while (ink_atomic_cas(&dst->data, h.data, new_h.data) == false); + } while (ink_atomic_cas(dst, h, new_h) == false); return h; } static bool -write_pointer_version(head_p *dst, head_p old_h, void *ptr, head_p::version_type vers) +write_pointer_version(head_p *dst, head_p old_h, void *ptr, head_p_version_type vers) { head_p tmp_h; SET_FREELIST_POINTER_VERSION(tmp_h, ptr, vers); - return ink_atomic_cas(&dst->data, old_h.data, tmp_h.data); + return ink_atomic_cas(dst, old_h, tmp_h); } LogBuffer * diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index 3d70052b199..842bb14ad3f 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -107,7 +107,14 @@ else() endif() target_link_libraries( - tscore PUBLIC OpenSSL::Crypto libswoc::libswoc yaml-cpp::yaml-cpp systemtap::systemtap resolv::resolv ts::tsutil + tscore + PUBLIC atomic + OpenSSL::Crypto + libswoc::libswoc + yaml-cpp::yaml-cpp + systemtap::systemtap + resolv::resolv + ts::tsutil ) if(TS_USE_POSIX_CAP) @@ -161,6 +168,7 @@ if(BUILD_TESTING) unit_tests/test_ink_base64.cc unit_tests/test_ink_inet.cc unit_tests/test_ink_memory.cc + unit_tests/test_ink_queue.cc unit_tests/test_ink_string.cc unit_tests/test_ink_sys_control.cc unit_tests/test_layout.cc diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 86cab240010..bfb3b33703f 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -38,9 +38,13 @@ #include "tscore/ink_config.h" +#include #include -#include +#include #include +#include +#include +#include #include #include #include @@ -57,6 +61,8 @@ #include "tscore/Diags.h" #include "tscore/JeMiAllocator.h" +#include + struct ink_freelist_ops { void *(*fl_new)(InkFreeList *); void (*fl_free)(InkFreeList *, void *); @@ -92,6 +98,9 @@ void *malloc_new(InkFreeList *f); void malloc_free(InkFreeList *f, void *item); void malloc_bulkfree(InkFreeList *f, void *head, void *tail, size_t num_item); +[[maybe_unused]] static bool is_next_ptr_aligned(InkFreeList const *f, head_p const &item); +[[maybe_unused]] static bool is_addr_aligned(void const *item, std::size_t alignment); + const ink_freelist_ops malloc_ops = {malloc_new, malloc_free, malloc_bulkfree}; const ink_freelist_ops freelist_ops = {freelist_new, freelist_free, freelist_bulkfree}; const ink_freelist_ops *default_ops = &freelist_ops; @@ -137,6 +146,15 @@ void ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32_t chunk_size, uint32_t alignment, bool use_hugepages) { + // The alignment is used as a boundary for INK_ALIGN, + // which requires a power of 2 boundary. + ink_release_assert(alignment % 2 == 0); + + // Freelist nodes are head_p objects whenever they are free, which means + // our allocation has to meet alignment requirements for both head_p objects + // and the allocator type. + alignment = std::lcm(alignment, alignof(head_p)); + InkFreeList *f; ink_freelist_list *fll; @@ -152,7 +170,7 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 f->name = name; /* quick test for power of 2 */ - ink_assert(!(alignment & (alignment - 1))); + ink_release_assert(!(alignment & (alignment - 1))); // It is never useful to have alignment requirement looser than a page size // so clip it. This makes the item alignment checks in the actual allocator simpler. f->alignment = alignment; @@ -178,7 +196,10 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 f->chunk_size = INK_ALIGN(chunk_size * f->type_size, ats_pagesize()) / f->type_size; } Dbg(dbg_ctl_freelist_init, "<%s> Chunk Size request/actual (%" PRIu32 "/%" PRIu32 ")", name, chunk_size, f->chunk_size); - SET_FREELIST_POINTER_VERSION(f->head, FROM_PTR(0), 0); + head_p empty_head; + SET_FREELIST_POINTER_VERSION(empty_head, FROM_PTR(0), 0); + ink_assert(is_next_ptr_aligned(f, empty_head)); + f->head.store(empty_head); *fl = f; } @@ -200,7 +221,13 @@ ink_freelist_create(const char *name, uint32_t type_size, uint32_t chunk_size, u return f; } -#define ADDRESS_OF_NEXT(x, offset) ((void **)((char *)x + offset)) +static head_p * +to_head_p(void *x, std::uint64_t offset) +{ + unsigned char *addr{reinterpret_cast(x) + offset}; + // ink_assert(is_addr_aligned(x, alignof(head_p))); + return reinterpret_cast(addr); +} void * ink_freelist_new(InkFreeList *f) @@ -208,7 +235,7 @@ ink_freelist_new(InkFreeList *f) void *ptr; if (likely(ptr = freelist_global_ops->fl_new(f))) { - ink_atomic_increment(reinterpret_cast(&f->used), 1); + f->used.fetch_add(1, std::memory_order_acq_rel); } return ptr; @@ -222,10 +249,13 @@ freelist_new(InkFreeList *f) { head_p item; head_p next; - int result = 0; + bool result = false; + + std::lock_guard guard{f->m}; + + item = f->head.load(); do { - INK_QUEUE_LD(item, f->head); if (TO_PTR(FREELIST_POINTER(item)) == nullptr) { uint32_t i; void *newp = nullptr; @@ -248,13 +278,14 @@ freelist_new(InkFreeList *f) if (f->advice) { ats_madvise(static_cast(newp), INK_ALIGN(alloc_size, alignment), f->advice); } - SET_FREELIST_POINTER_VERSION(item, newp, 0); + SET_FREELIST_POINTER_VERSION(item, FROM_PTR(newp), 0); + ink_assert(is_next_ptr_aligned(f, item)); - ink_atomic_increment(reinterpret_cast(&f->allocated), f->chunk_size); + f->allocated.fetch_add(f->chunk_size, std::memory_order_relaxed); /* free each of the new elements */ for (i = 0; i < f->chunk_size; i++) { - char *a = (static_cast(FREELIST_POINTER(item))) + i * f->type_size; + char *a = (static_cast(newp)) + i * f->type_size; #ifdef DEADBEEF const char str[4] = {static_cast(0xde), static_cast(0xad), static_cast(0xbe), static_cast(0xef)}; for (int j = 0; j < static_cast(f->type_size); j++) { @@ -263,10 +294,11 @@ freelist_new(InkFreeList *f) #endif freelist_free(f, a); } - } else { - SET_FREELIST_POINTER_VERSION(next, *ADDRESS_OF_NEXT(TO_PTR(FREELIST_POINTER(item)), 0), FREELIST_VERSION(item) + 1); - result = ink_atomic_cas(&f->head.data, item.data, next.data); + head_p *next_ptr = reinterpret_cast(TO_PTR(FREELIST_POINTER(item))); + + SET_FREELIST_POINTER_VERSION(next, FREELIST_POINTER(*next_ptr), FREELIST_VERSION(item) + 1); + result = f->head.compare_exchange_weak(item, next, std::memory_order_acquire, std::memory_order_acquire); #ifdef SANITY if (result) { @@ -282,9 +314,10 @@ freelist_new(InkFreeList *f) } #endif /* SANITY */ } - } while (result == 0); - ink_assert(!((uintptr_t)TO_PTR(FREELIST_POINTER(item)) & (((uintptr_t)f->alignment) - 1))); + } while (result == false); + ink_assert(is_next_ptr_aligned(f, item)); + ink_assert(is_next_ptr_aligned(f, next)); return TO_PTR(FREELIST_POINTER(item)); } @@ -308,9 +341,9 @@ void ink_freelist_free(InkFreeList *f, void *item) { if (likely(item != nullptr)) { - ink_assert(f->used != 0); freelist_global_ops->fl_free(f, item); - ink_atomic_decrement(reinterpret_cast(&f->used), 1); + [[maybe_unused]] std::uint32_t old_used = f->used.fetch_sub(1, std::memory_order_acq_rel); + ink_assert(old_used != 0 && "Mismatched freelist block ref count (too many frees)"); } } @@ -320,12 +353,13 @@ namespace void freelist_free(InkFreeList *f, void *item) { - void **adr_of_next = ADDRESS_OF_NEXT(item, 0); - head_p h; - head_p item_pair; - int result = 0; + // pointer to next pointer + head_p h; + head_p item_pair; + head_p *recovered_item; + bool result = false; - // ink_assert(!((long)item&(f->alignment-1))); XXX - why is this no longer working? -bcall + ink_release_assert(is_addr_aligned(item, f->alignment)); #ifdef DEADBEEF { @@ -338,8 +372,9 @@ freelist_free(InkFreeList *f, void *item) } #endif /* DEADBEEF */ + recovered_item = new (item) head_p{}; + h = f->head.load(); while (!result) { - INK_QUEUE_LD(h, f->head); #ifdef SANITY if (TO_PTR(FREELIST_POINTER(h)) == item) { ink_abort("ink_freelist_free: trying to free item twice"); @@ -351,11 +386,16 @@ freelist_free(InkFreeList *f, void *item) dummy_forced_read(TO_PTR(FREELIST_POINTER(h))); } #endif /* SANITY */ - *adr_of_next = FREELIST_POINTER(h); + + ink_assert(is_next_ptr_aligned(f, h)); + + SET_FREELIST_POINTER_VERSION(*recovered_item, FREELIST_POINTER(h), 0); SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(item), FREELIST_VERSION(h)); - INK_MEMORY_BARRIER; - result = ink_atomic_cas(&f->head.data, h.data, item_pair.data); + result = f->head.compare_exchange_weak(h, item_pair, std::memory_order_release, std::memory_order_relaxed); } + + ink_assert(is_next_ptr_aligned(f, h)); + ink_assert(is_next_ptr_aligned(f, *recovered_item)); } void @@ -373,10 +413,9 @@ malloc_free(InkFreeList *f, void *item) void ink_freelist_free_bulk(InkFreeList *f, void *head, void *tail, size_t num_item) { - ink_assert(f->used >= num_item); - freelist_global_ops->fl_bulkfree(f, head, tail, num_item); - ink_atomic_decrement(reinterpret_cast(&f->used), num_item); + [[maybe_unused]] std::uint32_t const old_used = f->used.fetch_sub(num_item, std::memory_order_acq_rel); + ink_assert(old_used >= num_item && "Mismatched freelist block ref count (too many frees)"); } namespace @@ -385,31 +424,32 @@ namespace void freelist_bulkfree(InkFreeList *f, void *head, void *tail, [[maybe_unused]] size_t num_item) { - void **adr_of_next = ADDRESS_OF_NEXT(tail, 0); - head_p h; - head_p item_pair; - int result = 0; + head_p h; + head_p item_pair; + head_p *recovered_tail; + bool result = false; - // ink_assert(!((long)item&(f->alignment-1))); XXX - why is this no longer working? -bcall + ink_release_assert(is_addr_aligned(head, f->alignment)); + ink_release_assert(is_addr_aligned(tail, f->alignment)); #ifdef DEADBEEF { static const char str[4] = {static_cast(0xde), static_cast(0xad), static_cast(0xbe), static_cast(0xef)}; // set the entire item to DEADBEEF; - void *temp = head; + head_p *temp = reinterpret_cast(head); for (size_t i = 0; i < num_item; i++) { for (int j = sizeof(void *); j < static_cast(f->type_size); j++) { - (static_cast(temp))[j] = str[j % 4]; + (reinterpret_cast(temp))[j] = str[j % 4]; } - *ADDRESS_OF_NEXT(temp, 0) = FROM_PTR(*ADDRESS_OF_NEXT(temp, 0)); - temp = TO_PTR(*ADDRESS_OF_NEXT(temp, 0)); + SET_FREELIST_POINTER_VERSION(*temp, FROM_PTR(FREELIST_POINTER(*temp)), FREELIST_VERSION(*temp)); + temp = to_head_p(TO_PTR(FREELIST_POINTER(*temp)), 0); } } #endif /* DEADBEEF */ + h = f->head.load(); while (!result) { - INK_QUEUE_LD(h, f->head); #ifdef SANITY if (TO_PTR(FREELIST_POINTER(h)) == head) { ink_abort("ink_freelist_free: trying to free item twice"); @@ -421,10 +461,10 @@ freelist_bulkfree(InkFreeList *f, void *head, void *tail, [[maybe_unused]] size_ dummy_forced_read(TO_PTR(FREELIST_POINTER(h))); } #endif /* SANITY */ - *adr_of_next = FREELIST_POINTER(h); + recovered_tail = new (tail) head_p{}; + SET_FREELIST_POINTER_VERSION(*recovered_tail, FREELIST_POINTER(h), 0); SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(head), FREELIST_VERSION(h)); - INK_MEMORY_BARRIER; - result = ink_atomic_cas(&f->head.data, h.data, item_pair.data); + result = f->head.compare_exchange_weak(h, item_pair, std::memory_order_release, std::memory_order_relaxed); } } @@ -444,6 +484,18 @@ malloc_bulkfree(InkFreeList *f, void *head, void *tail, size_t num_item) } } +bool +is_next_ptr_aligned(InkFreeList const *f, head_p const &item) +{ + return is_addr_aligned(TO_PTR(FREELIST_POINTER(item)), f->alignment); +} + +bool +is_addr_aligned(void const *item, std::size_t alignment) +{ + return !(reinterpret_cast(item) & (alignment - 1u)); +} + } // end anonymous namespace void @@ -452,9 +504,9 @@ ink_freelists_snap_baseline() ink_freelist_list *fll; fll = freelists; while (fll) { - fll->fl->allocated_base = fll->fl->allocated; - fll->fl->used_base = fll->fl->used; - fll = fll->next; + fll->fl->allocated_base.store(fll->fl->allocated.load(std::memory_order_relaxed), std::memory_order_relaxed); + fll->fl->used_base.store(fll->fl->used.load(std::memory_order_relaxed), std::memory_order_relaxed); + fll = fll->next; } } @@ -472,12 +524,16 @@ ink_freelists_dump_baselinerel(FILE *f) fll = freelists; while (fll) { - int a = fll->fl->allocated - fll->fl->allocated_base; + std::uint32_t const allocated = fll->fl->allocated.load(std::memory_order_relaxed); + std::uint32_t const allocated_base = fll->fl->allocated_base.load(std::memory_order_relaxed); + int const a = allocated - allocated_base; if (a != 0) { + std::uint32_t const used = fll->fl->used.load(std::memory_order_relaxed); + std::uint32_t const used_base = fll->fl->used_base.load(std::memory_order_relaxed); fprintf(f, " %18" PRIu64 " | %18" PRIu64 " | %7u | %10u | memory/%s\n", - static_cast(fll->fl->allocated - fll->fl->allocated_base) * static_cast(fll->fl->type_size), - static_cast(fll->fl->used - fll->fl->used_base) * static_cast(fll->fl->type_size), - fll->fl->used - fll->fl->used_base, fll->fl->type_size, fll->fl->name ? fll->fl->name : ""); + static_cast(allocated - allocated_base) * static_cast(fll->fl->type_size), + static_cast(used - used_base) * static_cast(fll->fl->type_size), used - used_base, + fll->fl->type_size, fll->fl->name ? fll->fl->name : ""); } fll = fll->next; } @@ -501,13 +557,14 @@ ink_freelists_dump(FILE *f) uint64_t total_used = 0; fll = freelists; while (fll) { + std::uint64_t const allocated = fll->fl->allocated.load(std::memory_order_relaxed); + std::uint64_t const used = fll->fl->used.load(std::memory_order_relaxed); fprintf(f, " %18" PRIu64 " | %18" PRIu64 " | %18" PRIu64 " | %18" PRIu64 " | %10u | %10u | %10u | memory/%s\n", - static_cast(fll->fl->allocated) * static_cast(fll->fl->type_size), - static_cast(fll->fl->allocated), - static_cast(fll->fl->used) * static_cast(fll->fl->type_size), static_cast(fll->fl->used), - fll->fl->type_size, fll->fl->chunk_size, fll->fl->hugepages_failure, fll->fl->name ? fll->fl->name : ""); - total_allocated += static_cast(fll->fl->allocated) * static_cast(fll->fl->type_size); - total_used += static_cast(fll->fl->used) * static_cast(fll->fl->type_size); + allocated * static_cast(fll->fl->type_size), allocated, used * static_cast(fll->fl->type_size), + used, fll->fl->type_size, fll->fl->chunk_size, fll->fl->hugepages_failure.load(), + fll->fl->name ? fll->fl->name : ""); + total_allocated += allocated * static_cast(fll->fl->type_size); + total_used += used * static_cast(fll->fl->type_size); fll = fll->next; } fprintf(f, " %18" PRIu64 " | %18" PRIu64 " | | TOTAL\n", total_allocated, total_used); @@ -519,7 +576,9 @@ ink_atomiclist_init(InkAtomicList *l, const char *name, uint32_t offset_to_next) { l->name = name; l->offset = offset_to_next; - SET_FREELIST_POINTER_VERSION(l->head, FROM_PTR(0), 0); + head_p empty_head; + SET_FREELIST_POINTER_VERSION(empty_head, FROM_PTR(nullptr), 0); + l->head.store(empty_head); } void * @@ -527,20 +586,23 @@ ink_atomiclist_pop(InkAtomicList *l) { head_p item; head_p next; - int result = 0; + bool result = 0; + + std::lock_guard guard{l->m}; + + item = l->head.load(); do { - INK_QUEUE_LD(item, l->head); if (TO_PTR(FREELIST_POINTER(item)) == nullptr) { return nullptr; } - SET_FREELIST_POINTER_VERSION(next, *ADDRESS_OF_NEXT(TO_PTR(FREELIST_POINTER(item)), l->offset), FREELIST_VERSION(item) + 1); - result = ink_atomic_cas(&l->head.data, item.data, next.data); - } while (result == 0); - { - void *ret = TO_PTR(FREELIST_POINTER(item)); - *ADDRESS_OF_NEXT(ret, l->offset) = nullptr; - return ret; - } + head_p *next_ptr = reinterpret_cast(reinterpret_cast(TO_PTR(FREELIST_POINTER(item))) + l->offset); + SET_FREELIST_POINTER_VERSION(next, FREELIST_POINTER(*next_ptr), FREELIST_VERSION(item) + 1); + result = l->head.compare_exchange_weak(item, next); + } while (result == false); + + head_p *ret = reinterpret_cast(reinterpret_cast(TO_PTR(FREELIST_POINTER(item))) + l->offset); + SET_FREELIST_POINTER_VERSION(*ret, nullptr, 0); + return ret; } void * @@ -548,45 +610,53 @@ ink_atomiclist_popall(InkAtomicList *l) { head_p item; head_p next; - int result = 0; + bool result = false; + + item = l->head.load(); do { - INK_QUEUE_LD(item, l->head); if (TO_PTR(FREELIST_POINTER(item)) == nullptr) { return nullptr; } SET_FREELIST_POINTER_VERSION(next, FROM_PTR(nullptr), FREELIST_VERSION(item) + 1); - result = ink_atomic_cas(&l->head.data, item.data, next.data); - } while (result == 0); - { - void *ret = TO_PTR(FREELIST_POINTER(item)); - void *e = ret; - /* fixup forward pointers */ - while (e) { - void *n = TO_PTR(*ADDRESS_OF_NEXT(e, l->offset)); - *ADDRESS_OF_NEXT(e, l->offset) = n; - e = n; - } - return ret; + result = l->head.compare_exchange_weak(item, next); + } while (result == false); + + void *ret = TO_PTR(FREELIST_POINTER(item)); + void *e = ret; + /* fixup forward pointers */ + while (e) { + head_p *e_ = to_head_p(e, l->offset); + void *n = TO_PTR(FREELIST_POINTER(*e_)); + SET_FREELIST_POINTER_VERSION(*e_, n, FREELIST_VERSION(*e_)); + e = n; } + + // ink_assert(is_addr_aligned(reinterpret_cast(ret) + l->offset, alignof(head_p))); + + return ret; } void * ink_atomiclist_push(InkAtomicList *l, void *item) { - void **adr_of_next = ADDRESS_OF_NEXT(item, l->offset); - head_p head; - head_p item_pair; - int result = 0; - void *h = nullptr; + // ink_release_assert(is_addr_aligned(reinterpret_cast(item) + l->offset, alignof(head_p))); + + head_p head; + head_p item_pair; + head_p *recovered_item; + bool result = false; + void *h = nullptr; + + head = l->head.load(); do { - INK_QUEUE_LD(head, l->head); - h = FREELIST_POINTER(head); - *adr_of_next = h; + h = FREELIST_POINTER(head); ink_assert(item != TO_PTR(h)); + + recovered_item = new (reinterpret_cast(item) + l->offset) head_p{}; + SET_FREELIST_POINTER_VERSION(*recovered_item, FREELIST_POINTER(head), 0); SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(item), FREELIST_VERSION(head)); - INK_MEMORY_BARRIER; - result = ink_atomic_cas(&l->head.data, head.data, item_pair.data); - } while (result == 0); + result = l->head.compare_exchange_weak(head, item_pair); + } while (result == false); return TO_PTR(h); } @@ -594,26 +664,25 @@ ink_atomiclist_push(InkAtomicList *l, void *item) void * ink_atomiclist_remove(InkAtomicList *l, void *item) { - head_p head; - void *prev = nullptr; - void **addr_next = ADDRESS_OF_NEXT(item, l->offset); - void *item_next = *addr_next; - int result = 0; + head_p head; + void *prev = nullptr; + head_p *addr_next = to_head_p(item, l->offset); + void *item_next = FREELIST_POINTER(*addr_next); + bool result = 0; /* * first, try to pop it if it is first */ - INK_QUEUE_LD(head, l->head); + head = l->head.load(); while (TO_PTR(FREELIST_POINTER(head)) == item) { head_p next; SET_FREELIST_POINTER_VERSION(next, item_next, FREELIST_VERSION(head) + 1); - result = ink_atomic_cas(&l->head.data, head.data, next.data); + result = l->head.compare_exchange_weak(head, next); if (result) { - *addr_next = nullptr; + SET_FREELIST_POINTER_VERSION(*addr_next, nullptr, 0); return item; } - INK_QUEUE_LD(head, l->head); } /* @@ -621,13 +690,13 @@ ink_atomiclist_remove(InkAtomicList *l, void *item) */ prev = TO_PTR(FREELIST_POINTER(head)); while (prev) { - void **prev_adr_of_next = ADDRESS_OF_NEXT(prev, l->offset); - void *prev_prev = prev; - prev = TO_PTR(*prev_adr_of_next); + head_p *prev_adr_of_next = to_head_p(prev, l->offset); + void *prev_prev = prev; + prev = TO_PTR(FREELIST_POINTER(*prev_adr_of_next)); if (prev == item) { ink_assert(prev_prev != item_next); - *prev_adr_of_next = item_next; - *addr_next = nullptr; + SET_FREELIST_POINTER_VERSION(*prev_adr_of_next, item_next, 0); + SET_FREELIST_POINTER_VERSION(*addr_next, nullptr, 0); return item; } } diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc new file mode 100644 index 00000000000..3da79f2512f --- /dev/null +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -0,0 +1,243 @@ +/* + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +#define CATCH_CONFIG_ENABLE_BENCHMARKING +#include + +#include + +#include +#include +#include + +TEST_CASE("Freelist", "[freelist]") +{ + // There is no error reporting for this routine. The allocation + // is assumed to never fail. + InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 1, alignof(std::int32_t))}; + + SECTION("Freelist allocates aligned pointers") + { + InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 1, alignof(std::int32_t))}; + void *addr{ink_freelist_new(f)}; + + CHECK(!(reinterpret_cast(addr) & (alignof(std::int32_t) - 1))); + + ink_freelist_free(f, addr); + } + + SECTION("Two new freelist allocations") + { + std::int32_t *a{new (ink_freelist_new(f)) std::int32_t{1}}; + std::int32_t *b{new (ink_freelist_new(f)) std::int32_t{2}}; + + CHECK(((*a == 1) && (*b == 2))); + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + } + + SECTION("Freelist stack behavior") + { + void *a{ink_freelist_new(f)}; + void *b{ink_freelist_new(f)}; + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + + void *x{ink_freelist_new(f)}; + void *y{ink_freelist_new(f)}; + + CHECK((a == y && b == x)); + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + } +} + +TEST_CASE("Empty atomic list", "[atomiclist]") +{ + InkAtomicList l; + ink_atomiclist_init(&l, "test#1", 0); + + CHECK(nullptr == ink_atomiclist_pop(&l)); +} + +TEST_CASE("Popall from empty atomic list", "[atomiclist]") +{ + InkAtomicList l; + ink_atomiclist_init(&l, "test#1", 0); + + CHECK(nullptr == ink_atomiclist_popall(&l)); +} + +TEST_CASE("Atomic list", "[atomiclist]") +{ + InkAtomicList l; + ink_atomiclist_init(&l, "test#1", 0); + + // We allocate memory this way to ensure proper alignment for atomiclist. + InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 1, alignof(std::int32_t))}; + + SECTION("Atomic list becomes empty after push and pop") + { + ink_atomiclist_push(&l, ink_freelist_new(f)); + void *a{ink_atomiclist_pop(&l)}; + + CHECK(nullptr == ink_atomiclist_pop(&l)); + + ink_freelist_free(f, a); + } + + SECTION("Atomic list stack behavior") + { + void *a{ink_freelist_new(f)}; + void *b{ink_freelist_new(f)}; + + ink_atomiclist_push(&l, a); + ink_atomiclist_push(&l, b); + + void *x{ink_atomiclist_pop(&l)}; + void *y{ink_atomiclist_pop(&l)}; + + CHECK((a == y && b == x)); + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + } + + SECTION("Atomic list remove head") + { + void *a{ink_freelist_new(f)}; + void *b{ink_freelist_new(f)}; + + ink_atomiclist_push(&l, a); + ink_atomiclist_push(&l, b); + + CHECK(b == ink_atomiclist_remove(&l, b)); + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + } + + SECTION("Atomic list remove tail") + { + void *a{ink_freelist_new(f)}; + void *b{ink_freelist_new(f)}; + + ink_atomiclist_push(&l, a); + ink_atomiclist_push(&l, b); + + CHECK(a == ink_atomiclist_remove(&l, a)); + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + } + + SECTION("Atomic list becomes empty after popall") + { + void *a{ink_freelist_new(f)}; + void *b{ink_freelist_new(f)}; + + ink_atomiclist_push(&l, a); + ink_atomiclist_push(&l, b); + + ink_atomiclist_popall(&l); + CHECK(nullptr == ink_atomiclist_popall(&l)); + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + } + + SECTION("Atomic list popall behavior") + { + void *a{ink_freelist_new(f)}; + void *b{ink_freelist_new(f)}; + + ink_atomiclist_push(&l, a); + ink_atomiclist_push(&l, b); + + head_p *head{reinterpret_cast(ink_atomiclist_popall(&l))}; + head_p *tail{reinterpret_cast(FREELIST_POINTER(*head))}; + + CHECK(((head == b) && (tail == a))); + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + } +} + +TEST_CASE("Freelist benchmarks", "[freelist][bench]") +{ + BENCHMARK_ADVANCED("Single threaded alloc")(Catch::Benchmark::Chronometer meter) + { + InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 4, alignof(std::int32_t))}; + + ink_freelist_new(f); + + meter.measure([&]() { return ink_freelist_new(f); }); + }; + + BENCHMARK_ADVANCED("Single threaded free")(Catch::Benchmark::Chronometer meter) + { + InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 4, alignof(std::int32_t))}; + + std::vector ptrs; + ptrs.resize(meter.runs()); + for (auto &x : ptrs) { + x = ink_freelist_new(f); + } + + meter.measure([&](int i) { return ink_freelist_free(f, ptrs[i]); }); + }; + + /* + BENCHMARK_ADVANCED("yay")(Catch::Benchmark::Chronometer meter) + { + InkFreeList *f{ink_freelist_create("yay", 4, 8, 16)}; + + std::atomic done(false); + + auto const use_memory{[&]() { + void *storage{ink_freelist_new(f)}; + std::int32_t *n{new (storage) std::int32_t{}}; + *n = 10; + ink_freelist_free(f, n); + }}; + + auto const distract{[&]() { + while (!done) { + use_memory(); + } + }}; + + std::jthread contender{distract}; + + meter.measure([&]() { + for (int i{0}; i < 10; ++i) { + use_memory(); + } + }); + + done = true; + }; + */ +} From 581723b788b290b775602aee7d79e41a3edff424 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 06:04:32 -0500 Subject: [PATCH 02/38] Make changes suggested by Copilot * Fix macro definitions for all platforms The definitions still used the `data` member of the old `head_p` struct. * Clean up `head_p_view` definitions * Remove unused `` include * Make `freelist_init` alignment check consistent * Test atomiclist with offset * Add @file description to test_ink_queue.cc * Add missing `` include to unit tests * Construct `InkFreeList` with placement new --- include/tscore/ink_queue.h | 39 ++++-------- src/tscore/ink_queue.cc | 18 +++--- src/tscore/unit_tests/test_ink_queue.cc | 80 +++++++++---------------- 3 files changed, 51 insertions(+), 86 deletions(-) diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index 52e905dfbbe..34fda9ee20e 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -92,25 +92,14 @@ using head_p = head_p_data_type; #if ((defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4)) || TS_HAS_128BIT_CAS +// This struct maps to head_p on the above platforms. On other platforms, +// bitshifting is used to read head_p. +// See FREELIST_POINTER for the layout on those platforms. struct head_p_view { void *pointer; head_p_version_type version; }; -#elif TS_HAS_128BIT_CAS - -struct head_p_view { - void *pointer; - head_p_version_type version; -}; - -#elif defined(__x86_64__) || defined(__ia64__) || defined(__powerpc64__) || defined(__mips64) - -struct head_p_view { - int vaddr : 48; - int version : 15; - int vaddr_mode : 1; -}; #endif inline head_p_view @@ -122,8 +111,6 @@ load_head(head_p const &src) return result; } -#include - inline void store_head(head_p &dest, head_p_view const src) { @@ -176,16 +163,14 @@ store_head(head_p &dest, head_p_view const src) */ #if ((~0 >> 1) < 0) /* the shift is `arithmetic' */ -#define FREELIST_POINTER(_x) \ - ((void *)((((intptr_t)(_x).data) & 0x0000FFFFFFFFFFFFLL) | ((((intptr_t)(_x).data) >> 63) << 48))) // sign extend +#define FREELIST_POINTER(_x) ((void *)((((intptr_t)(_x)) & 0x0000FFFFFFFFFFFFLL) | ((((intptr_t)(_x)) >> 63) << 48))) // sign extend #else /* the shift is `logical' */ -#define FREELIST_POINTER(_x) \ - ((void *)((((intptr_t)(_x).data) & 0x0000FFFFFFFFFFFFLL) | ((~((((intptr_t)(_x).data) >> 63) - 1)) << 48))) +#define FREELIST_POINTER(_x) ((void *)((((intptr_t)(_x)) & 0x0000FFFFFFFFFFFFLL) | ((~((((intptr_t)(_x)) >> 63) - 1)) << 48))) #endif -#define FREELIST_VERSION(_x) ((((intptr_t)(_x).data) & 0x7FFF000000000000LL) >> 48) -#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) (_x).data = ((((intptr_t)(_p)) & 0x8000FFFFFFFFFFFFLL) | (((_v) & 0x7FFFLL) << 48)) +#define FREELIST_VERSION(_x) ((((intptr_t)(_x)) & 0x7FFF000000000000LL) >> 48) +#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) (_x) = ((((intptr_t)(_p)) & 0x8000FFFFFFFFFFFFLL) | (((_v) & 0x7FFFLL) << 48)) #elif defined(__aarch64__) /* Layout of FREELIST_POINTER * @@ -195,16 +180,14 @@ store_head(head_p &dest, head_p_view const src) */ #if ((~0 >> 1) < 0) /* the shift is `arithmetic' */ -#define FREELIST_POINTER(_x) \ - ((void *)((((intptr_t)(_x).data) & 0x000FFFFFFFFFFFFFLL) | ((((intptr_t)(_x).data) >> 63) << 52))) // sign extend +#define FREELIST_POINTER(_x) ((void *)((((intptr_t)(_x)) & 0x000FFFFFFFFFFFFFLL) | ((((intptr_t)(_x)) >> 63) << 52))) // sign extend #else /* the shift is `logical' */ -#define FREELIST_POINTER(_x) \ - ((void *)((((intptr_t)(_x).data) & 0x000FFFFFFFFFFFFFLL) | ((~((((intptr_t)(_x).data) >> 63) - 1)) << 52))) +#define FREELIST_POINTER(_x) ((void *)((((intptr_t)(_x)) & 0x000FFFFFFFFFFFFFLL) | ((~((((intptr_t)(_x)) >> 63) - 1)) << 52))) #endif -#define FREELIST_VERSION(_x) ((((intptr_t)(_x).data) & 0x7FF0000000000000LL) >> 52) -#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) (_x).data = ((((intptr_t)(_p)) & 0x800FFFFFFFFFFFFFLL) | (((_v) & 0x7FFLL) << 52)) +#define FREELIST_VERSION(_x) ((((intptr_t)(_x)) & 0x7FF0000000000000LL) >> 52) +#define SET_FREELIST_POINTER_VERSION(_x, _p, _v) (_x) = ((((intptr_t)(_p)) & 0x800FFFFFFFFFFFFFLL) | (((_v) & 0x7FFLL) << 52)) #else #error "unsupported processor" #endif diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index bfb3b33703f..e5c1cb34099 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -61,8 +61,6 @@ #include "tscore/Diags.h" #include "tscore/JeMiAllocator.h" -#include - struct ink_freelist_ops { void *(*fl_new)(InkFreeList *); void (*fl_free)(InkFreeList *, void *); @@ -148,7 +146,7 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 { // The alignment is used as a boundary for INK_ALIGN, // which requires a power of 2 boundary. - ink_release_assert(alignment % 2 == 0); + ink_release_assert(alignment != 0 && (alignment & (alignment - 1u)) == 0); // Freelist nodes are head_p objects whenever they are free, which means // our allocation has to meet alignment requirements for both head_p objects @@ -160,8 +158,13 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 /* its safe to add to this global list because ink_freelist_init() is only called from single-threaded initialization code. */ - f = static_cast(ats_memalign(alignment, sizeof(InkFreeList))); - ink_zero(*f); + f = new (ats_memalign(alignment, sizeof(InkFreeList))) InkFreeList; + f->used = 0; + f->allocated = 0; + f->allocated_base = 0; + f->used_base = 0; + f->hugepages_failure = 0; + f->advice = 0; fll = static_cast(ats_malloc(sizeof(ink_freelist_list))); fll->fl = f; @@ -600,8 +603,9 @@ ink_atomiclist_pop(InkAtomicList *l) result = l->head.compare_exchange_weak(item, next); } while (result == false); - head_p *ret = reinterpret_cast(reinterpret_cast(TO_PTR(FREELIST_POINTER(item))) + l->offset); - SET_FREELIST_POINTER_VERSION(*ret, nullptr, 0); + void *ret = TO_PTR(FREELIST_POINTER(item)); + head_p *ret_ = reinterpret_cast(reinterpret_cast(ret) + l->offset); + SET_FREELIST_POINTER_VERSION(*ret_, nullptr, 0); return ret; } diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index 3da79f2512f..6910ced35e4 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -1,22 +1,24 @@ -/* +/** @file - @section license License + Freelist and Atomiclist tests - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at + @section license License - http://www.apache.org/licenses/LICENSE-2.0 + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. */ #define CATCH_CONFIG_ENABLE_BENCHMARKING @@ -25,8 +27,10 @@ #include #include +#include #include #include +#include TEST_CASE("Freelist", "[freelist]") { @@ -91,11 +95,16 @@ TEST_CASE("Popall from empty atomic list", "[atomiclist]") TEST_CASE("Atomic list", "[atomiclist]") { + struct test_type { + std::int32_t i; + head_p next; + }; + InkAtomicList l; - ink_atomiclist_init(&l, "test#1", 0); + ink_atomiclist_init(&l, "test#1", offsetof(test_type, next)); // We allocate memory this way to ensure proper alignment for atomiclist. - InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 1, alignof(std::int32_t))}; + InkFreeList *f{ink_freelist_create("test#1", sizeof(test_type), 1, alignof(test_type))}; SECTION("Atomic list becomes empty after push and pop") { @@ -175,8 +184,9 @@ TEST_CASE("Atomic list", "[atomiclist]") ink_atomiclist_push(&l, a); ink_atomiclist_push(&l, b); - head_p *head{reinterpret_cast(ink_atomiclist_popall(&l))}; - head_p *tail{reinterpret_cast(FREELIST_POINTER(*head))}; + void *head{ink_atomiclist_popall(&l)}; + head_p *head_{reinterpret_cast(reinterpret_cast(head) + l.offset)}; + void *tail{FREELIST_POINTER(*head_)}; CHECK(((head == b) && (tail == a))); @@ -208,36 +218,4 @@ TEST_CASE("Freelist benchmarks", "[freelist][bench]") meter.measure([&](int i) { return ink_freelist_free(f, ptrs[i]); }); }; - - /* - BENCHMARK_ADVANCED("yay")(Catch::Benchmark::Chronometer meter) - { - InkFreeList *f{ink_freelist_create("yay", 4, 8, 16)}; - - std::atomic done(false); - - auto const use_memory{[&]() { - void *storage{ink_freelist_new(f)}; - std::int32_t *n{new (storage) std::int32_t{}}; - *n = 10; - ink_freelist_free(f, n); - }}; - - auto const distract{[&]() { - while (!done) { - use_memory(); - } - }}; - - std::jthread contender{distract}; - - meter.measure([&]() { - for (int i{0}; i < 10; ++i) { - use_memory(); - } - }); - - done = true; - }; - */ } From e059484e61cb968d4d22b88140f9d69b5a5f0128 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 07:22:53 -0500 Subject: [PATCH 03/38] Make changes suggested by Copilot * Only link libatomic when using 128bit atomics and lib exists --- src/tscore/CMakeLists.txt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index 842bb14ad3f..f68877d582c 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -106,15 +106,15 @@ else() target_sources(tscore PRIVATE HKDF_openssl.cc) endif() +if(TS_HAS_128BIT_CAS) + find_library(atomic QUIET) + if(atomic_FOUND) + target_link_libraries(tscore PUBLIC atomic) + endif() +endif() + target_link_libraries( - tscore - PUBLIC atomic - OpenSSL::Crypto - libswoc::libswoc - yaml-cpp::yaml-cpp - systemtap::systemtap - resolv::resolv - ts::tsutil + tscore PUBLIC OpenSSL::Crypto libswoc::libswoc yaml-cpp::yaml-cpp systemtap::systemtap resolv::resolv ts::tsutil ) if(TS_USE_POSIX_CAP) From f00b13e91233ea3e8a9576b7d989eb28f5b518cf Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 07:50:50 -0500 Subject: [PATCH 04/38] Fix race condition with assertion --- src/tscore/ink_queue.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index e5c1cb34099..4aac9e2d71c 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -390,15 +390,16 @@ freelist_free(InkFreeList *f, void *item) } #endif /* SANITY */ - ink_assert(is_next_ptr_aligned(f, h)); - SET_FREELIST_POINTER_VERSION(*recovered_item, FREELIST_POINTER(h), 0); SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(item), FREELIST_VERSION(h)); + + // This assertion has to happen-before the node is in the list and + // can be handed out to an allocator. + ink_assert(is_next_ptr_aligned(f, *recovered_item)); result = f->head.compare_exchange_weak(h, item_pair, std::memory_order_release, std::memory_order_relaxed); } ink_assert(is_next_ptr_aligned(f, h)); - ink_assert(is_next_ptr_aligned(f, *recovered_item)); } void From c3d81797ab3dca62250d6ec55b6a6d718d7b71f7 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 07:54:55 -0500 Subject: [PATCH 05/38] Link libatomic on all platforms but Apple --- src/tscore/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index f68877d582c..b3b676fdd6e 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -108,7 +108,7 @@ endif() if(TS_HAS_128BIT_CAS) find_library(atomic QUIET) - if(atomic_FOUND) + if(APPLE) target_link_libraries(tscore PUBLIC atomic) endif() endif() From 2e2a8469ca518f085d7ae7a30379f390c5cad2d1 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 08:10:10 -0500 Subject: [PATCH 06/38] Fix incorrect check for non-Apple platforms --- src/tscore/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index b3b676fdd6e..e0b7910490a 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -107,8 +107,7 @@ else() endif() if(TS_HAS_128BIT_CAS) - find_library(atomic QUIET) - if(APPLE) + if(NOT APPLE) target_link_libraries(tscore PUBLIC atomic) endif() endif() From fa61ef3532b34acf6ee59a4b3cef6cbced77566a Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 08:38:35 -0500 Subject: [PATCH 07/38] Implement complete cross-platform libatomic check --- src/tscore/CMakeLists.txt | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index e0b7910490a..db4399e1c8e 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -107,7 +107,27 @@ else() endif() if(TS_HAS_128BIT_CAS) - if(NOT APPLE) + set(ATOMICLIB_TEST_PROGRAM + " + #include + + std::atomic<__int128> g_atomic; + + int main() { } + " + ) + + check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" NEED_LIBATOMIC) + + if(NEED_LIBATOMIC) + set(CMAKE_REQUIRED_LIBRARIES atomic) + check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" HAVE_LIBATOMIC) + unset(CMAKE_REQUIRED_LIBRARIES) + + if(NOT HAVE_LIBATOMIC) + message(FATAL_ERROR "Cannot build 128 bit atomics") + endif() + target_link_libraries(tscore PUBLIC atomic) endif() endif() From 4eeea4efc90a0bc4f831d52d8d389f5f10fd611d Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 09:07:36 -0500 Subject: [PATCH 08/38] Disable std::atomic<__int128> on lacking platforms --- src/tscore/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index db4399e1c8e..87658e25d6f 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -125,7 +125,8 @@ if(TS_HAS_128BIT_CAS) unset(CMAKE_REQUIRED_LIBRARIES) if(NOT HAVE_LIBATOMIC) - message(FATAL_ERROR "Cannot build 128 bit atomics") + message(WARNING "Cannot build 128 bit atomics") + set(TS_HAS_128BIT_CAS FALSE) endif() target_link_libraries(tscore PUBLIC atomic) From c96a8628d3f83bd79fcca264a92dee4086cd2d66 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 09:27:10 -0500 Subject: [PATCH 09/38] Do not link libatomic when not available --- src/tscore/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index 87658e25d6f..efdafd9d32f 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -127,9 +127,10 @@ if(TS_HAS_128BIT_CAS) if(NOT HAVE_LIBATOMIC) message(WARNING "Cannot build 128 bit atomics") set(TS_HAS_128BIT_CAS FALSE) + else() + target_link_libraries(tscore PUBLIC atomic) endif() - target_link_libraries(tscore PUBLIC atomic) endif() endif() From de95aa741c7e16b3febf2af960c9ff5b8d9fe37e Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 19 May 2026 11:14:28 -0500 Subject: [PATCH 10/38] Base `TS_HAS_128BIT_CAS` on `__atomic` hardware --- cmake/Check128BitCas.cmake | 10 ++++++---- include/tscore/ink_queue.h | 4 ++-- src/tscore/CMakeLists.txt | 28 ---------------------------- 3 files changed, 8 insertions(+), 34 deletions(-) diff --git a/cmake/Check128BitCas.cmake b/cmake/Check128BitCas.cmake index 850bc398fe3..7c39d35dae1 100644 --- a/cmake/Check128BitCas.cmake +++ b/cmake/Check128BitCas.cmake @@ -25,16 +25,18 @@ set(CHECK_PROGRAM " - int main(void) + #include + + int main() { - __int128_t x = 0; - return __sync_bool_compare_and_swap(&x,0,10); + std::atomic<__int128> x{0}; + return x.compare_exchange_strong(10, 0); } " ) include(CheckCSourceCompiles) -check_c_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_CAS) +check_cxx_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_CAS) if(NOT TS_HAS_128BIT_CAS) unset(TS_HAS_128BIT_CAS CACHE) diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index 34fda9ee20e..dda25d5245f 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -100,8 +100,6 @@ struct head_p_view { head_p_version_type version; }; -#endif - inline head_p_view load_head(head_p const &src) { @@ -118,6 +116,8 @@ store_head(head_p &dest, head_p_view const src) std::memcpy(&dest, &src, sizeof(dest)); } +#endif + /* * Why is version required? One scenario is described below * Think of a list like this -> A -> C -> D diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index efdafd9d32f..9297bea403c 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -106,34 +106,6 @@ else() target_sources(tscore PRIVATE HKDF_openssl.cc) endif() -if(TS_HAS_128BIT_CAS) - set(ATOMICLIB_TEST_PROGRAM - " - #include - - std::atomic<__int128> g_atomic; - - int main() { } - " - ) - - check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" NEED_LIBATOMIC) - - if(NEED_LIBATOMIC) - set(CMAKE_REQUIRED_LIBRARIES atomic) - check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" HAVE_LIBATOMIC) - unset(CMAKE_REQUIRED_LIBRARIES) - - if(NOT HAVE_LIBATOMIC) - message(WARNING "Cannot build 128 bit atomics") - set(TS_HAS_128BIT_CAS FALSE) - else() - target_link_libraries(tscore PUBLIC atomic) - endif() - - endif() -endif() - target_link_libraries( tscore PUBLIC OpenSSL::Crypto libswoc::libswoc yaml-cpp::yaml-cpp systemtap::systemtap resolv::resolv ts::tsutil ) From aa231666f95a5c2e800dfcb333b1aecf113170bc Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 06:05:27 -0500 Subject: [PATCH 11/38] Suppress freelist_new leaks in CI --- ci/asan_leak_suppression/unit_tests.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/asan_leak_suppression/unit_tests.txt b/ci/asan_leak_suppression/unit_tests.txt index a553604f1bf..abc41e494bb 100644 --- a/ci/asan_leak_suppression/unit_tests.txt +++ b/ci/asan_leak_suppression/unit_tests.txt @@ -2,3 +2,8 @@ # not destroyed because it holds a reference to a stack-allocated # TestRefCountObj whose free() override calls exit(1). leak:test_http_hdr_print_and_copy_aux + +# on 64-bit architectures, freelist pointers are stored in a special bitwise +# format, so LSan cannot find link pointers. It will erroneously determine +# that the list tail is not reachable. +leak:freelist_new From 47739793d0270ad9267180194e786f248c46b59f Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 10:23:09 -0500 Subject: [PATCH 12/38] Assert proper atomiclist alignment --- src/tscore/ink_queue.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 4aac9e2d71c..d1719d05926 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -578,6 +578,12 @@ ink_freelists_dump(FILE *f) void ink_atomiclist_init(InkAtomicList *l, const char *name, uint32_t offset_to_next) { + // The pointers we push onto the atomiclist will also need to be aligned. If + // the offset is not aligned, then it is not possible for the caller to + // determine a consistent, safe alignment for the object the head_p objects + // are subobjects of. + ink_release_assert(offset_to_next % alignof(head_p) == 0); + l->name = name; l->offset = offset_to_next; head_p empty_head; @@ -636,7 +642,7 @@ ink_atomiclist_popall(InkAtomicList *l) e = n; } - // ink_assert(is_addr_aligned(reinterpret_cast(ret) + l->offset, alignof(head_p))); + ink_assert(is_addr_aligned(ret, alignof(head_p))); return ret; } @@ -644,7 +650,7 @@ ink_atomiclist_popall(InkAtomicList *l) void * ink_atomiclist_push(InkAtomicList *l, void *item) { - // ink_release_assert(is_addr_aligned(reinterpret_cast(item) + l->offset, alignof(head_p))); + ink_release_assert(is_addr_aligned(item, alignof(head_p))); head_p head; head_p item_pair; From c1bf02b8c80ae0a7bcd7d9efd243e7cfa8519e9d Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 14:49:02 -0500 Subject: [PATCH 13/38] Make changes suggested by Copilot * Store freelist nodes as `void*`, not `head_p` * Remove redundant freelist allocation in tests --- src/tscore/ink_queue.cc | 109 ++++++++++++------------ src/tscore/unit_tests/test_ink_queue.cc | 11 ++- 2 files changed, 59 insertions(+), 61 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index d1719d05926..1b3e7485b6a 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -148,10 +148,9 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 // which requires a power of 2 boundary. ink_release_assert(alignment != 0 && (alignment & (alignment - 1u)) == 0); - // Freelist nodes are head_p objects whenever they are free, which means - // our allocation has to meet alignment requirements for both head_p objects - // and the allocator type. - alignment = std::lcm(alignment, alignof(head_p)); + // Freelist nodes have to be at least void* aligned since they store void*s when + // not allocated out. + alignment = std::lcm(alignment, alignof(void *)); InkFreeList *f; ink_freelist_list *fll; @@ -224,12 +223,12 @@ ink_freelist_create(const char *name, uint32_t type_size, uint32_t chunk_size, u return f; } -static head_p * -to_head_p(void *x, std::uint64_t offset) +static void ** +to_voidp_p(void *x, std::uint64_t offset) { unsigned char *addr{reinterpret_cast(x) + offset}; - // ink_assert(is_addr_aligned(x, alignof(head_p))); - return reinterpret_cast(addr); + ink_assert(is_addr_aligned(x, alignof(void **))); + return reinterpret_cast(addr); } void * @@ -298,9 +297,9 @@ freelist_new(InkFreeList *f) freelist_free(f, a); } } else { - head_p *next_ptr = reinterpret_cast(TO_PTR(FREELIST_POINTER(item))); + void **next_ptr = reinterpret_cast(TO_PTR(FREELIST_POINTER(item))); - SET_FREELIST_POINTER_VERSION(next, FREELIST_POINTER(*next_ptr), FREELIST_VERSION(item) + 1); + SET_FREELIST_POINTER_VERSION(next, *next_ptr, FREELIST_VERSION(item) + 1); result = f->head.compare_exchange_weak(item, next, std::memory_order_acquire, std::memory_order_acquire); #ifdef SANITY @@ -357,10 +356,10 @@ void freelist_free(InkFreeList *f, void *item) { // pointer to next pointer - head_p h; - head_p item_pair; - head_p *recovered_item; - bool result = false; + head_p h; + head_p item_pair; + void **recovered_item; + bool result = false; ink_release_assert(is_addr_aligned(item, f->alignment)); @@ -375,7 +374,7 @@ freelist_free(InkFreeList *f, void *item) } #endif /* DEADBEEF */ - recovered_item = new (item) head_p{}; + recovered_item = new (item) void *{}; h = f->head.load(); while (!result) { #ifdef SANITY @@ -390,12 +389,12 @@ freelist_free(InkFreeList *f, void *item) } #endif /* SANITY */ - SET_FREELIST_POINTER_VERSION(*recovered_item, FREELIST_POINTER(h), 0); - SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(item), FREELIST_VERSION(h)); + *recovered_item = FREELIST_POINTER(h); + SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(recovered_item), FREELIST_VERSION(h)); // This assertion has to happen-before the node is in the list and // can be handed out to an allocator. - ink_assert(is_next_ptr_aligned(f, *recovered_item)); + ink_assert(is_addr_aligned(TO_PTR(*recovered_item), f->alignment)); result = f->head.compare_exchange_weak(h, item_pair, std::memory_order_release, std::memory_order_relaxed); } @@ -428,10 +427,10 @@ namespace void freelist_bulkfree(InkFreeList *f, void *head, void *tail, [[maybe_unused]] size_t num_item) { - head_p h; - head_p item_pair; - head_p *recovered_tail; - bool result = false; + head_p h; + head_p item_pair; + void **recovered_tail; + bool result = false; ink_release_assert(is_addr_aligned(head, f->alignment)); ink_release_assert(is_addr_aligned(tail, f->alignment)); @@ -441,13 +440,13 @@ freelist_bulkfree(InkFreeList *f, void *head, void *tail, [[maybe_unused]] size_ static const char str[4] = {static_cast(0xde), static_cast(0xad), static_cast(0xbe), static_cast(0xef)}; // set the entire item to DEADBEEF; - head_p *temp = reinterpret_cast(head); + void **temp = reinterpret_cast(head); for (size_t i = 0; i < num_item; i++) { for (int j = sizeof(void *); j < static_cast(f->type_size); j++) { (reinterpret_cast(temp))[j] = str[j % 4]; } - SET_FREELIST_POINTER_VERSION(*temp, FROM_PTR(FREELIST_POINTER(*temp)), FREELIST_VERSION(*temp)); - temp = to_head_p(TO_PTR(FREELIST_POINTER(*temp)), 0); + *temp = FROM_PTR(*temp); + temp = to_voidp_p(TO_PTR(*temp), 0); } } #endif /* DEADBEEF */ @@ -465,8 +464,8 @@ freelist_bulkfree(InkFreeList *f, void *head, void *tail, [[maybe_unused]] size_ dummy_forced_read(TO_PTR(FREELIST_POINTER(h))); } #endif /* SANITY */ - recovered_tail = new (tail) head_p{}; - SET_FREELIST_POINTER_VERSION(*recovered_tail, FREELIST_POINTER(h), 0); + recovered_tail = new (tail) void *{}; + *recovered_tail = FREELIST_POINTER(h); SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(head), FREELIST_VERSION(h)); result = f->head.compare_exchange_weak(h, item_pair, std::memory_order_release, std::memory_order_relaxed); } @@ -605,14 +604,14 @@ ink_atomiclist_pop(InkAtomicList *l) if (TO_PTR(FREELIST_POINTER(item)) == nullptr) { return nullptr; } - head_p *next_ptr = reinterpret_cast(reinterpret_cast(TO_PTR(FREELIST_POINTER(item))) + l->offset); - SET_FREELIST_POINTER_VERSION(next, FREELIST_POINTER(*next_ptr), FREELIST_VERSION(item) + 1); + void **next_ptr = reinterpret_cast(reinterpret_cast(TO_PTR(FREELIST_POINTER(item))) + l->offset); + SET_FREELIST_POINTER_VERSION(next, *next_ptr, FREELIST_VERSION(item) + 1); result = l->head.compare_exchange_weak(item, next); } while (result == false); - void *ret = TO_PTR(FREELIST_POINTER(item)); - head_p *ret_ = reinterpret_cast(reinterpret_cast(ret) + l->offset); - SET_FREELIST_POINTER_VERSION(*ret_, nullptr, 0); + void *ret = TO_PTR(FREELIST_POINTER(item)); + void **ret_ = reinterpret_cast(reinterpret_cast(ret) + l->offset); + *ret_ = nullptr; return ret; } @@ -636,10 +635,10 @@ ink_atomiclist_popall(InkAtomicList *l) void *e = ret; /* fixup forward pointers */ while (e) { - head_p *e_ = to_head_p(e, l->offset); - void *n = TO_PTR(FREELIST_POINTER(*e_)); - SET_FREELIST_POINTER_VERSION(*e_, n, FREELIST_VERSION(*e_)); - e = n; + void **e_ = to_voidp_p(e, l->offset); + void *n = TO_PTR(FREELIST_POINTER(*e_)); + *e_ = n; + e = n; } ink_assert(is_addr_aligned(ret, alignof(head_p))); @@ -650,21 +649,21 @@ ink_atomiclist_popall(InkAtomicList *l) void * ink_atomiclist_push(InkAtomicList *l, void *item) { - ink_release_assert(is_addr_aligned(item, alignof(head_p))); + ink_release_assert(is_addr_aligned(item, alignof(void *))); - head_p head; - head_p item_pair; - head_p *recovered_item; - bool result = false; - void *h = nullptr; + head_p head; + head_p item_pair; + void **recovered_item; + bool result = false; + void *h = nullptr; head = l->head.load(); do { h = FREELIST_POINTER(head); ink_assert(item != TO_PTR(h)); - recovered_item = new (reinterpret_cast(item) + l->offset) head_p{}; - SET_FREELIST_POINTER_VERSION(*recovered_item, FREELIST_POINTER(head), 0); + recovered_item = new (reinterpret_cast(item) + l->offset) void *{}; + *recovered_item = FREELIST_POINTER(head); SET_FREELIST_POINTER_VERSION(item_pair, FROM_PTR(item), FREELIST_VERSION(head)); result = l->head.compare_exchange_weak(head, item_pair); } while (result == false); @@ -675,11 +674,11 @@ ink_atomiclist_push(InkAtomicList *l, void *item) void * ink_atomiclist_remove(InkAtomicList *l, void *item) { - head_p head; - void *prev = nullptr; - head_p *addr_next = to_head_p(item, l->offset); - void *item_next = FREELIST_POINTER(*addr_next); - bool result = 0; + head_p head; + void *prev = nullptr; + void **addr_next = to_voidp_p(item, l->offset); + void *item_next = *addr_next; + bool result = 0; /* * first, try to pop it if it is first @@ -691,7 +690,7 @@ ink_atomiclist_remove(InkAtomicList *l, void *item) result = l->head.compare_exchange_weak(head, next); if (result) { - SET_FREELIST_POINTER_VERSION(*addr_next, nullptr, 0); + *addr_next = nullptr; return item; } } @@ -701,13 +700,13 @@ ink_atomiclist_remove(InkAtomicList *l, void *item) */ prev = TO_PTR(FREELIST_POINTER(head)); while (prev) { - head_p *prev_adr_of_next = to_head_p(prev, l->offset); - void *prev_prev = prev; - prev = TO_PTR(FREELIST_POINTER(*prev_adr_of_next)); + void **prev_adr_of_next = to_voidp_p(prev, l->offset); + void *prev_prev = prev; + prev = TO_PTR(FREELIST_POINTER(*prev_adr_of_next)); if (prev == item) { ink_assert(prev_prev != item_next); - SET_FREELIST_POINTER_VERSION(*prev_adr_of_next, item_next, 0); - SET_FREELIST_POINTER_VERSION(*addr_next, nullptr, 0); + *prev_adr_of_next = item_next; + *addr_next = nullptr; return item; } } diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index 6910ced35e4..98e43f5427a 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -40,8 +40,7 @@ TEST_CASE("Freelist", "[freelist]") SECTION("Freelist allocates aligned pointers") { - InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 1, alignof(std::int32_t))}; - void *addr{ink_freelist_new(f)}; + void *addr{ink_freelist_new(f)}; CHECK(!(reinterpret_cast(addr) & (alignof(std::int32_t) - 1))); @@ -97,7 +96,7 @@ TEST_CASE("Atomic list", "[atomiclist]") { struct test_type { std::int32_t i; - head_p next; + void *next; }; InkAtomicList l; @@ -184,9 +183,9 @@ TEST_CASE("Atomic list", "[atomiclist]") ink_atomiclist_push(&l, a); ink_atomiclist_push(&l, b); - void *head{ink_atomiclist_popall(&l)}; - head_p *head_{reinterpret_cast(reinterpret_cast(head) + l.offset)}; - void *tail{FREELIST_POINTER(*head_)}; + void *head{ink_atomiclist_popall(&l)}; + void **head_{reinterpret_cast(reinterpret_cast(head) + l.offset)}; + void *tail{FREELIST_POINTER(*head_)}; CHECK(((head == b) && (tail == a))); From 6a6c19af13b33d66b567a502b691e33b58638266 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 14:59:25 -0500 Subject: [PATCH 14/38] Make changes suggested by Copilot * Fix 128bit CAS test program --- cmake/Check128BitCas.cmake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmake/Check128BitCas.cmake b/cmake/Check128BitCas.cmake index 7c39d35dae1..8c84b4cf10e 100644 --- a/cmake/Check128BitCas.cmake +++ b/cmake/Check128BitCas.cmake @@ -30,7 +30,8 @@ set(CHECK_PROGRAM int main() { std::atomic<__int128> x{0}; - return x.compare_exchange_strong(10, 0); + __int128 expected{x.load()}; + return x.compare_exchange_strong(expected, 10); } " ) From 11df3e0420a524187cb0a5dbf707e38860a5c5ab Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 15:16:17 -0500 Subject: [PATCH 15/38] Do not use `FREELIST_POINTER` to read `void*` --- src/tscore/ink_queue.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 1b3e7485b6a..a7cf5e8bdf9 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -636,7 +636,7 @@ ink_atomiclist_popall(InkAtomicList *l) /* fixup forward pointers */ while (e) { void **e_ = to_voidp_p(e, l->offset); - void *n = TO_PTR(FREELIST_POINTER(*e_)); + void *n = TO_PTR(*e_); *e_ = n; e = n; } @@ -702,7 +702,7 @@ ink_atomiclist_remove(InkAtomicList *l, void *item) while (prev) { void **prev_adr_of_next = to_voidp_p(prev, l->offset); void *prev_prev = prev; - prev = TO_PTR(FREELIST_POINTER(*prev_adr_of_next)); + prev = TO_PTR(*prev_adr_of_next); if (prev == item) { ink_assert(prev_prev != item_next); *prev_adr_of_next = item_next; From ea92af6d0e4ed9ccf9e204a3071601f60143a2e6 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 15:27:43 -0500 Subject: [PATCH 16/38] Do not use `FREELIST_POINTER` for `void*` in tests --- src/tscore/unit_tests/test_ink_queue.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index 98e43f5427a..e36a5cb38b1 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -185,7 +185,7 @@ TEST_CASE("Atomic list", "[atomiclist]") void *head{ink_atomiclist_popall(&l)}; void **head_{reinterpret_cast(reinterpret_cast(head) + l.offset)}; - void *tail{FREELIST_POINTER(*head_)}; + void *tail{(*head_}; CHECK(((head == b) && (tail == a))); From c0f63c006062d5e39d4bd3e2ded214c87dd54485 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 15:31:42 -0500 Subject: [PATCH 17/38] Make changes suggested by Copilot * Change remaining `alignof(head_p)` to `alignof(void*)` --- src/tscore/ink_queue.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index a7cf5e8bdf9..f924a63c963 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -227,7 +227,7 @@ static void ** to_voidp_p(void *x, std::uint64_t offset) { unsigned char *addr{reinterpret_cast(x) + offset}; - ink_assert(is_addr_aligned(x, alignof(void **))); + ink_assert(is_addr_aligned(addr, alignof(void **))); return reinterpret_cast(addr); } @@ -579,9 +579,9 @@ ink_atomiclist_init(InkAtomicList *l, const char *name, uint32_t offset_to_next) { // The pointers we push onto the atomiclist will also need to be aligned. If // the offset is not aligned, then it is not possible for the caller to - // determine a consistent, safe alignment for the object the head_p objects + // determine a consistent, safe alignment for the object the void* objects // are subobjects of. - ink_release_assert(offset_to_next % alignof(head_p) == 0); + ink_release_assert(offset_to_next % alignof(void *) == 0); l->name = name; l->offset = offset_to_next; @@ -641,7 +641,7 @@ ink_atomiclist_popall(InkAtomicList *l) e = n; } - ink_assert(is_addr_aligned(ret, alignof(head_p))); + ink_assert(is_addr_aligned(ret, alignof(void *))); return ret; } From 71099a33fa25a3559af03278631f8b18725a106c Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Wed, 20 May 2026 15:38:08 -0500 Subject: [PATCH 18/38] Fix syntax error --- src/tscore/unit_tests/test_ink_queue.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index e36a5cb38b1..593c3fef172 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -185,7 +185,7 @@ TEST_CASE("Atomic list", "[atomiclist]") void *head{ink_atomiclist_popall(&l)}; void **head_{reinterpret_cast(reinterpret_cast(head) + l.offset)}; - void *tail{(*head_}; + void *tail{*head_}; CHECK(((head == b) && (tail == a))); From d103e37abcea5ae6bc7d902627f476e2eaa45c55 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 08:01:15 -0500 Subject: [PATCH 19/38] Remove benchmarks in favor of existing ones --- src/tscore/unit_tests/test_ink_queue.cc | 28 +------------------------ 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index 593c3fef172..d154192febf 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -21,8 +21,7 @@ limitations under the License. */ -#define CATCH_CONFIG_ENABLE_BENCHMARKING -#include +#include #include @@ -193,28 +192,3 @@ TEST_CASE("Atomic list", "[atomiclist]") ink_freelist_free(f, b); } } - -TEST_CASE("Freelist benchmarks", "[freelist][bench]") -{ - BENCHMARK_ADVANCED("Single threaded alloc")(Catch::Benchmark::Chronometer meter) - { - InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 4, alignof(std::int32_t))}; - - ink_freelist_new(f); - - meter.measure([&]() { return ink_freelist_new(f); }); - }; - - BENCHMARK_ADVANCED("Single threaded free")(Catch::Benchmark::Chronometer meter) - { - InkFreeList *f{ink_freelist_create("test#1", sizeof(std::int32_t), 4, alignof(std::int32_t))}; - - std::vector ptrs; - ptrs.resize(meter.runs()); - for (auto &x : ptrs) { - x = ink_freelist_new(f); - } - - meter.measure([&](int i) { return ink_freelist_free(f, ptrs[i]); }); - }; -} From e19cafd201c297d50911097342a133e6b0ef3ff5 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 08:07:34 -0500 Subject: [PATCH 20/38] Revert 128bit CAS test changes --- cmake/Check128BitCas.cmake | 11 ++++------- include/tscore/ink_queue.h | 4 ++-- src/tscore/CMakeLists.txt | 28 ++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/cmake/Check128BitCas.cmake b/cmake/Check128BitCas.cmake index 8c84b4cf10e..850bc398fe3 100644 --- a/cmake/Check128BitCas.cmake +++ b/cmake/Check128BitCas.cmake @@ -25,19 +25,16 @@ set(CHECK_PROGRAM " - #include - - int main() + int main(void) { - std::atomic<__int128> x{0}; - __int128 expected{x.load()}; - return x.compare_exchange_strong(expected, 10); + __int128_t x = 0; + return __sync_bool_compare_and_swap(&x,0,10); } " ) include(CheckCSourceCompiles) -check_cxx_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_CAS) +check_c_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_CAS) if(NOT TS_HAS_128BIT_CAS) unset(TS_HAS_128BIT_CAS CACHE) diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index dda25d5245f..34fda9ee20e 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -100,6 +100,8 @@ struct head_p_view { head_p_version_type version; }; +#endif + inline head_p_view load_head(head_p const &src) { @@ -116,8 +118,6 @@ store_head(head_p &dest, head_p_view const src) std::memcpy(&dest, &src, sizeof(dest)); } -#endif - /* * Why is version required? One scenario is described below * Think of a list like this -> A -> C -> D diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index 9297bea403c..efdafd9d32f 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -106,6 +106,34 @@ else() target_sources(tscore PRIVATE HKDF_openssl.cc) endif() +if(TS_HAS_128BIT_CAS) + set(ATOMICLIB_TEST_PROGRAM + " + #include + + std::atomic<__int128> g_atomic; + + int main() { } + " + ) + + check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" NEED_LIBATOMIC) + + if(NEED_LIBATOMIC) + set(CMAKE_REQUIRED_LIBRARIES atomic) + check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" HAVE_LIBATOMIC) + unset(CMAKE_REQUIRED_LIBRARIES) + + if(NOT HAVE_LIBATOMIC) + message(WARNING "Cannot build 128 bit atomics") + set(TS_HAS_128BIT_CAS FALSE) + else() + target_link_libraries(tscore PUBLIC atomic) + endif() + + endif() +endif() + target_link_libraries( tscore PUBLIC OpenSSL::Crypto libswoc::libswoc yaml-cpp::yaml-cpp systemtap::systemtap resolv::resolv ts::tsutil ) From 7f8e7b7639b4bd0d4158aea9fae580f39332f0cc Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 08:14:13 -0500 Subject: [PATCH 21/38] Add 128bit atomic test in CMake for ink queue --- CMakeLists.txt | 1 + cmake/Check128BitAtomic.cmake | 58 ++++++++++++++++++++++++++++ include/tscore/ink_config.h.cmake.in | 1 + include/tscore/ink_queue.h | 10 ++--- 4 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 cmake/Check128BitAtomic.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index fcd8b2eef13..5e157c976a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -416,6 +416,7 @@ if(EXTERNAL_LIBSWOC) endif() include(Check128BitCas) +include(Check128BitAtomic) include(ConfigureTransparentProxy) # Find ccache diff --git a/cmake/Check128BitAtomic.cmake b/cmake/Check128BitAtomic.cmake new file mode 100644 index 00000000000..130764ceae3 --- /dev/null +++ b/cmake/Check128BitAtomic.cmake @@ -0,0 +1,58 @@ +####################### +# +# Licensed to the Apache Software Foundation (ASF) under one or more contributor license +# agreements. See the NOTICE file distributed with this work for additional information regarding +# copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# +####################### + +# Check128BitAtomic.cmake +# +# This will define the following variables +# +# TS_HAS_128BIT_ATOMIC +# TS_NEEDS_MCX16_FOR_128BIT_ATOMIC +# + +set(CHECK_PROGRAM + " + #include + + int main() + { + std::atomic<__int128> x{0}; + __int128 expected{x.load()}; + return x.compare_exchange_strong(expected, 10); + } + " +) + +include(CheckCSourceCompiles) +check_cxx_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_ATOMIC) + +if(NOT TS_HAS_128BIT_ATOMIC) + unset(TS_HAS_128BIT_ATOMIC CACHE) + set(CMAKE_REQUIRED_FLAGS "-Werror -mcx16") + check_c_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_ATOMIC) + set(NEED_MCX16 ${TS_HAS_128BIT_ATOMIC}) + unset(CMAKE_REQUIRED_FLAGS) +endif() + +set(TS_NEEDS_MCX16_FOR_128BIT_ATOMIC + ${NEED_MCX16} + CACHE BOOL "Whether -mcx16 is needed to compile 128bit atomics" +) + +unset(CHECK_PROGRAM) +unset(NEEDS_MCX16) + +mark_as_advanced(TS_HAS_128BIT_ATOMIC TS_NEEDS_MCX16_FOR_128BIT_ATOMIC) diff --git a/include/tscore/ink_config.h.cmake.in b/include/tscore/ink_config.h.cmake.in index 73c8b860fb9..fcdb2a29f44 100644 --- a/include/tscore/ink_config.h.cmake.in +++ b/include/tscore/ink_config.h.cmake.in @@ -139,6 +139,7 @@ const int DEFAULT_STACKSIZE = @DEFAULT_STACK_SIZE@; /* Feature Flags */ #cmakedefine01 TS_HAS_128BIT_CAS +#cmakedefine01 TS_HAS_128BIT_ATOMIC #cmakedefine01 TS_HAS_BACKTRACE #cmakedefine01 TS_HAS_IN6_IS_ADDR_UNSPECIFIED #cmakedefine01 TS_HAS_IP_TOS diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index 34fda9ee20e..69631ab2336 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -78,7 +78,7 @@ void ink_queue_load_64(void *dst, void *src); #if (defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4) typedef int32_t head_p_version_type; typedef int64_t head_p_data_type; -#elif TS_HAS_128BIT_CAS +#elif TS_HAS_128BIT_ATOMIC typedef int64_t head_p_version_type; typedef __int128_t head_p_data_type; #else @@ -90,7 +90,7 @@ using head_p_data_type = int64_t; // lock, use INK_QUEUE_LD to read safely. using head_p = head_p_data_type; -#if ((defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4)) || TS_HAS_128BIT_CAS +#if ((defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4)) || TS_HAS_128BIT_ATOMIC // This struct maps to head_p on the above platforms. On other platforms, // bitshifting is used to read head_p. @@ -100,8 +100,6 @@ struct head_p_view { head_p_version_type version; }; -#endif - inline head_p_view load_head(head_p const &src) { @@ -111,6 +109,8 @@ load_head(head_p const &src) return result; } +#endif + inline void store_head(head_p &dest, head_p_view const src) { @@ -145,7 +145,7 @@ store_head(head_p &dest, head_p_view const src) #define FREELIST_POINTER(_x) load_head((_x)).pointer #define FREELIST_VERSION(_x) load_head((_x)).version #define SET_FREELIST_POINTER_VERSION(_x, _p, _v) store_head((_x), head_p_view{(_p), (_v)}) -#elif TS_HAS_128BIT_CAS +#elif TS_HAS_128BIT_ATOMIC #define FREELIST_POINTER(_x) load_head((_x)).pointer #define FREELIST_VERSION(_x) load_head((_x)).version #define SET_FREELIST_POINTER_VERSION(_x, _p, _v) store_head((_x), head_p_view{(_p), (_v)}) From 160a583946248335a2fba03fb3936f2e7c51bc25 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 08:20:56 -0500 Subject: [PATCH 22/38] Make changes suggested by Copilot * Fix access of head in List.h This is the only access that needs to be fixed. --- include/tscore/List.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/tscore/List.h b/include/tscore/List.h index f652ddc88e3..8c08e6ee416 100644 --- a/include/tscore/List.h +++ b/include/tscore/List.h @@ -957,7 +957,7 @@ template struct AtomicSLL { C * head() { - return (C *)TO_PTR(FREELIST_POINTER(al.head)); + return (C *)TO_PTR(FREELIST_POINTER(al.head.load())); } C * next(C *c) From 7ba52bfee5b80b5a94d90898d8d6900531972a2e Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 08:28:24 -0500 Subject: [PATCH 23/38] Build `store_head` conditionally --- include/tscore/ink_queue.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index 69631ab2336..d4a8c787fad 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -109,8 +109,6 @@ load_head(head_p const &src) return result; } -#endif - inline void store_head(head_p &dest, head_p_view const src) { @@ -118,6 +116,8 @@ store_head(head_p &dest, head_p_view const src) std::memcpy(&dest, &src, sizeof(dest)); } +#endif + /* * Why is version required? One scenario is described below * Think of a list like this -> A -> C -> D From 895d1b4dd142a0f6bd3038df14b28fe049c9ab24 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 08:58:48 -0500 Subject: [PATCH 24/38] Require both `__sync` and `__swap` operations --- include/tscore/ink_queue.h | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index d4a8c787fad..c7b24118a0a 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -55,6 +55,12 @@ void ink_queue_load_64(void *dst, void *src); +#if TS_HAS_128BIT_CAS && TS_HAS_128BIT_ATOMIC +#define TS_128BIT_QUEUE 1 +#else +#define TS_128BIT_QUEUE 0 +#endif + #ifdef __x86_64__ #define INK_QUEUE_LD64(dst, src) *((uint64_t *)&(dst)) = *((uint64_t *)&(src)) #else @@ -62,7 +68,7 @@ void ink_queue_load_64(void *dst, void *src); #endif // passing a const volatile value of 0 works around a gcc bug -#if TS_HAS_128BIT_CAS +#if TS_128BIT_QUEUE #define INK_QUEUE_LD(dst, src) \ do { \ const volatile __int128_t iqld0 = 0; \ @@ -78,7 +84,7 @@ void ink_queue_load_64(void *dst, void *src); #if (defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4) typedef int32_t head_p_version_type; typedef int64_t head_p_data_type; -#elif TS_HAS_128BIT_ATOMIC +#elif TS_128BIT_QUEUE typedef int64_t head_p_version_type; typedef __int128_t head_p_data_type; #else @@ -90,7 +96,7 @@ using head_p_data_type = int64_t; // lock, use INK_QUEUE_LD to read safely. using head_p = head_p_data_type; -#if ((defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4)) || TS_HAS_128BIT_ATOMIC +#if ((defined(__i386__) || defined(__arm__) || defined(__mips__)) && (SIZEOF_VOIDP == 4)) || TS_128BIT_QUEUE // This struct maps to head_p on the above platforms. On other platforms, // bitshifting is used to read head_p. @@ -145,7 +151,7 @@ store_head(head_p &dest, head_p_view const src) #define FREELIST_POINTER(_x) load_head((_x)).pointer #define FREELIST_VERSION(_x) load_head((_x)).version #define SET_FREELIST_POINTER_VERSION(_x, _p, _v) store_head((_x), head_p_view{(_p), (_v)}) -#elif TS_HAS_128BIT_ATOMIC +#elif TS_128BIT_QUEUE #define FREELIST_POINTER(_x) load_head((_x)).pointer #define FREELIST_VERSION(_x) load_head((_x)).version #define SET_FREELIST_POINTER_VERSION(_x, _p, _v) store_head((_x), head_p_view{(_p), (_v)}) From 0623f858d7adc7ff0b70e8b6f6462d2c60e8ca54 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 09:39:58 -0500 Subject: [PATCH 25/38] Add `-mcx16` flag when needed for atomics --- src/tscore/CMakeLists.txt | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/src/tscore/CMakeLists.txt b/src/tscore/CMakeLists.txt index efdafd9d32f..04a2d606d86 100644 --- a/src/tscore/CMakeLists.txt +++ b/src/tscore/CMakeLists.txt @@ -106,34 +106,6 @@ else() target_sources(tscore PRIVATE HKDF_openssl.cc) endif() -if(TS_HAS_128BIT_CAS) - set(ATOMICLIB_TEST_PROGRAM - " - #include - - std::atomic<__int128> g_atomic; - - int main() { } - " - ) - - check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" NEED_LIBATOMIC) - - if(NEED_LIBATOMIC) - set(CMAKE_REQUIRED_LIBRARIES atomic) - check_cxx_source_compiles("${ATOMICLIB_TEST_PROGRAM}" HAVE_LIBATOMIC) - unset(CMAKE_REQUIRED_LIBRARIES) - - if(NOT HAVE_LIBATOMIC) - message(WARNING "Cannot build 128 bit atomics") - set(TS_HAS_128BIT_CAS FALSE) - else() - target_link_libraries(tscore PUBLIC atomic) - endif() - - endif() -endif() - target_link_libraries( tscore PUBLIC OpenSSL::Crypto libswoc::libswoc yaml-cpp::yaml-cpp systemtap::systemtap resolv::resolv ts::tsutil ) @@ -151,7 +123,10 @@ endif() add_dependencies(tscore ParseRules) target_include_directories(tscore PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) -if(TS_HAS_128BIT_CAS AND TS_NEEDS_MCX16_FOR_CAS) +if((TS_HAS_128BIT_CAS + AND TS_HAS_128BIT_ATOMIC + AND (TS_NEEDS_MCX16_FOR_CAS OR TS_NEEDS_MCX16_FOR_128BIT_ATOMIC)) +) target_compile_options(tscore PUBLIC "-mcx16") endif() From 43ab51fd8c18ee46de35ee1ab4942b2c69ab14b7 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Tue, 2 Jun 2026 09:52:58 -0500 Subject: [PATCH 26/38] Make changes suggested by Copilot * Explicitly load atomic in empty list check * Consistently use cxx CMake checks for atomics * Align `InkFreeList` correctly --- cmake/Check128BitAtomic.cmake | 4 ++-- include/tscore/ink_queue.h | 4 ++-- src/tscore/ink_queue.cc | 12 +++++++++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cmake/Check128BitAtomic.cmake b/cmake/Check128BitAtomic.cmake index 130764ceae3..2aa018b730c 100644 --- a/cmake/Check128BitAtomic.cmake +++ b/cmake/Check128BitAtomic.cmake @@ -36,13 +36,13 @@ set(CHECK_PROGRAM " ) -include(CheckCSourceCompiles) +include(CheckCXXSourceCompiles) check_cxx_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_ATOMIC) if(NOT TS_HAS_128BIT_ATOMIC) unset(TS_HAS_128BIT_ATOMIC CACHE) set(CMAKE_REQUIRED_FLAGS "-Werror -mcx16") - check_c_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_ATOMIC) + check_cxx_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_ATOMIC) set(NEED_MCX16 ${TS_HAS_128BIT_ATOMIC}) unset(CMAKE_REQUIRED_FLAGS) endif() diff --git a/include/tscore/ink_queue.h b/include/tscore/ink_queue.h index c7b24118a0a..0a962a514a6 100644 --- a/include/tscore/ink_queue.h +++ b/include/tscore/ink_queue.h @@ -249,10 +249,10 @@ struct InkAtomicList { }; #if !defined(INK_QUEUE_NT) -#define INK_ATOMICLIST_EMPTY(_x) (!(TO_PTR(FREELIST_POINTER((_x.head))))) +#define INK_ATOMICLIST_EMPTY(_x) (!(TO_PTR(FREELIST_POINTER((_x.head.load()))))) #else /* ink_queue_nt.c doesn't do the FROM/TO pointer swizzling */ -#define INK_ATOMICLIST_EMPTY(_x) (!((FREELIST_POINTER((_x.head))))) +#define INK_ATOMICLIST_EMPTY(_x) (!((FREELIST_POINTER((_x.head.load()))))) #endif // WARNING: the "name" string is not copied, it has to be a statically-stored constant string. diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index f924a63c963..69f518843a3 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -148,9 +148,15 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 // which requires a power of 2 boundary. ink_release_assert(alignment != 0 && (alignment & (alignment - 1u)) == 0); - // Freelist nodes have to be at least void* aligned since they store void*s when - // not allocated out. - alignment = std::lcm(alignment, alignof(void *)); + // The same alignment is used for both the InkFreeList object and to + // calculate the alignment member that determines alignment for + // chunks allocated in ink_freelist_new. I don't know why. + // + // The alignment may need to be corrected to ensure all internal bookkeeping + // objects are properly aligned, not only the user objects placed into the + // allocated memory. + alignment = std::lcm(alignment, static_cast(alignof(InkFreeList))); + alignment = std::lcm(alignment, static_cast(alignof(void *))); InkFreeList *f; ink_freelist_list *fll; From 18607e41cc2ffdfc50b48447fe4e7e56c2c40fa3 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 4 Jun 2026 07:38:12 -0500 Subject: [PATCH 27/38] Make changes suggested by Copilot * Fix typo of `NEEDS_MCX16` -> `NEED_MCX16` --- cmake/Check128BitAtomic.cmake | 4 +++- cmake/Check128BitCas.cmake | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmake/Check128BitAtomic.cmake b/cmake/Check128BitAtomic.cmake index 2aa018b730c..dabdef5ae7c 100644 --- a/cmake/Check128BitAtomic.cmake +++ b/cmake/Check128BitAtomic.cmake @@ -39,6 +39,8 @@ set(CHECK_PROGRAM include(CheckCXXSourceCompiles) check_cxx_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_ATOMIC) +set(NEED_MCX16 FALSE) + if(NOT TS_HAS_128BIT_ATOMIC) unset(TS_HAS_128BIT_ATOMIC CACHE) set(CMAKE_REQUIRED_FLAGS "-Werror -mcx16") @@ -53,6 +55,6 @@ set(TS_NEEDS_MCX16_FOR_128BIT_ATOMIC ) unset(CHECK_PROGRAM) -unset(NEEDS_MCX16) +unset(NEED_MCX16) mark_as_advanced(TS_HAS_128BIT_ATOMIC TS_NEEDS_MCX16_FOR_128BIT_ATOMIC) diff --git a/cmake/Check128BitCas.cmake b/cmake/Check128BitCas.cmake index 850bc398fe3..f74c753f4e7 100644 --- a/cmake/Check128BitCas.cmake +++ b/cmake/Check128BitCas.cmake @@ -36,6 +36,8 @@ set(CHECK_PROGRAM include(CheckCSourceCompiles) check_c_source_compiles("${CHECK_PROGRAM}" TS_HAS_128BIT_CAS) +set(NEED_MCX16 FALSE) + if(NOT TS_HAS_128BIT_CAS) unset(TS_HAS_128BIT_CAS CACHE) set(CMAKE_REQUIRED_FLAGS "-Werror -mcx16") @@ -50,6 +52,6 @@ set(TS_NEEDS_MCX16_FOR_CAS ) unset(CHECK_PROGRAM) -unset(NEEDS_MCX16) +unset(NEED_MCX16) mark_as_advanced(TS_HAS_128BIT_CAS TS_NEEDS_MCX16_FOR_CAS) From bc028805897b45e9ecf2a4657e60d861c760d5d9 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 4 Jun 2026 08:15:49 -0500 Subject: [PATCH 28/38] Fix typo in LSan suppression file --- ci/asan_leak_suppression/unit_tests.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/asan_leak_suppression/unit_tests.txt b/ci/asan_leak_suppression/unit_tests.txt index abc41e494bb..e7e910dacf4 100644 --- a/ci/asan_leak_suppression/unit_tests.txt +++ b/ci/asan_leak_suppression/unit_tests.txt @@ -6,4 +6,4 @@ leak:test_http_hdr_print_and_copy_aux # on 64-bit architectures, freelist pointers are stored in a special bitwise # format, so LSan cannot find link pointers. It will erroneously determine # that the list tail is not reachable. -leak:freelist_new +leak:ink_freelist_new From fe13bcd7155a1d23e9ee3b3806d25657ff9bcfe6 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 4 Jun 2026 08:19:33 -0500 Subject: [PATCH 29/38] Remove unprofessional phrase in comment --- src/tscore/ink_queue.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 69f518843a3..d6119edc128 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -150,11 +150,11 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 // The same alignment is used for both the InkFreeList object and to // calculate the alignment member that determines alignment for - // chunks allocated in ink_freelist_new. I don't know why. + // chunks allocated in ink_freelist_new. // - // The alignment may need to be corrected to ensure all internal bookkeeping - // objects are properly aligned, not only the user objects placed into the - // allocated memory. + // The alignment is adjusted here to ensure all internal bookkeeping + // objects are properly aligned, as well as user objects placed into + // memory allocated from the freelist. alignment = std::lcm(alignment, static_cast(alignof(InkFreeList))); alignment = std::lcm(alignment, static_cast(alignof(void *))); From 1f8dc73700c79df9f70d82a117ca74922fe9a4a1 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 4 Jun 2026 12:57:23 -0500 Subject: [PATCH 30/38] Include header for placement new --- src/tscore/ink_queue.cc | 1 + src/tscore/unit_tests/test_ink_queue.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index d6119edc128..84a8e6073e7 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index d154192febf..7bd1aa2dd39 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -28,6 +28,7 @@ #include #include #include +#include #include #include From 1f4edc911a10fe1ed136a8156d9480cf2067eb1c Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 5 Jun 2026 06:46:12 -0500 Subject: [PATCH 31/38] Fix leak suppression matching --- ci/asan_leak_suppression/unit_tests.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/asan_leak_suppression/unit_tests.txt b/ci/asan_leak_suppression/unit_tests.txt index e7e910dacf4..b7e5c59905e 100644 --- a/ci/asan_leak_suppression/unit_tests.txt +++ b/ci/asan_leak_suppression/unit_tests.txt @@ -6,4 +6,4 @@ leak:test_http_hdr_print_and_copy_aux # on 64-bit architectures, freelist pointers are stored in a special bitwise # format, so LSan cannot find link pointers. It will erroneously determine # that the list tail is not reachable. -leak:ink_freelist_new +leak:freelist_new* From 5acaee7d949eb51adb9678dab1e9199b176a909b Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 12 Jun 2026 13:14:46 -0500 Subject: [PATCH 32/38] Mark freelist chunks as reachable memory for LSan This change was authored with Claude Opus 4.7 and Claude Sonnet 4.6. --- src/tscore/ink_queue.cc | 16 +++++++++ src/tscore/unit_tests/test_ink_queue.cc | 46 +++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 84a8e6073e7..9761f0e77ec 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -52,6 +52,10 @@ #include "swoc/bwf_ip.h" +#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) +#include +#endif + #include "tscore/ink_atomic.h" #include "tscore/ink_queue.h" #include "tscore/ink_memory.h" @@ -287,6 +291,18 @@ freelist_new(InkFreeList *f) if (f->advice) { ats_madvise(static_cast(newp), INK_ALIGN(alloc_size, alignment), f->advice); } + // LSan root-region registration. Each cell on the free-chain stores its + // successor as a bitwise-tagged pointer (version bits merged into the high + // bits of the word by SET_FREELIST_POINTER_VERSION / FROM_PTR). LSan's + // pointer scanner sees the tagged word and cannot recognize it as a heap + // pointer, so it classifies the cells as direct leaks. Registering the + // entire chunk as a root region tells LSan to scan the chunk's bytes for + // pointers; through that scan every cell on the chain becomes reachable and + // is reclassified as "still reachable." Registration happens once per chunk + // (amortized over chunk_size cells) and is a no-op in non-ASan builds. +#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) + __lsan_register_root_region(newp, INK_ALIGN(alloc_size, alignment)); +#endif SET_FREELIST_POINTER_VERSION(item, FROM_PTR(newp), 0); ink_assert(is_next_ptr_aligned(f, item)); diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index 7bd1aa2dd39..7d5355e4092 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -32,6 +32,52 @@ #include #include +// Chunk-seeded cells on the free-chain at process exit must not appear +// as direct leaks. The one issued cell is reachable via the registered root +// region; the remaining chunk_size-1 seeded cells are also covered because +// the entire chunk is a registered LSan root region. +#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) +TEST_CASE("FreelistChunkSeededCellsNotDirectLeak", "[freelist][lsan]") +{ + InkFreeList *f{ink_freelist_create("lsan_seeded", 8, 8, 8)}; + + // Force the first chunk allocation. Do not return this cell — intentionally + // held to represent an "in-use" cell so LSan sees it reachable via the + // root region scan rather than as a dropped pointer. + (void)ink_freelist_new(f); + + // The remaining chunk_size-1 seeded cells are on the free-chain. At process + // exit LSan must classify them as "still reachable" through the registered + // root region, not as direct leaks. + CHECK(true); +} +#endif + +// Caller-returned cells on the free-chain at process exit must not +// appear as direct leaks. +#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) +TEST_CASE("FreelistReturnedCellsNotDirectLeak", "[freelist][lsan]") +{ + InkFreeList *f{ink_freelist_create("lsan_returned", 8, 8, 8)}; + + // Issue several cells then return them all via single-item free. + void *a{ink_freelist_new(f)}; + void *b{ink_freelist_new(f)}; + void *c{ink_freelist_new(f)}; + void *d{ink_freelist_new(f)}; + + ink_freelist_free(f, a); + ink_freelist_free(f, b); + ink_freelist_free(f, c); + ink_freelist_free(f, d); + + // All four returned cells are now on the free-chain. At process exit LSan + // must classify them as "still reachable" through the registered root + // region, not as direct leaks. + CHECK(true); +} +#endif + TEST_CASE("Freelist", "[freelist]") { // There is no error reporting for this routine. The allocation From 8f242bf3b998bc30a6215afb760e7d128e044ddc Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 12 Jun 2026 13:15:59 -0500 Subject: [PATCH 33/38] Remove `freelist_new` LSan suppression --- ci/asan_leak_suppression/unit_tests.txt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ci/asan_leak_suppression/unit_tests.txt b/ci/asan_leak_suppression/unit_tests.txt index b7e5c59905e..a553604f1bf 100644 --- a/ci/asan_leak_suppression/unit_tests.txt +++ b/ci/asan_leak_suppression/unit_tests.txt @@ -2,8 +2,3 @@ # not destroyed because it holds a reference to a stack-allocated # TestRefCountObj whose free() override calls exit(1). leak:test_http_hdr_print_and_copy_aux - -# on 64-bit architectures, freelist pointers are stored in a special bitwise -# format, so LSan cannot find link pointers. It will erroneously determine -# that the list tail is not reachable. -leak:freelist_new* From c8e05c971a10312c231795e96b3b3ab64456c244 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 12 Jun 2026 13:35:12 -0500 Subject: [PATCH 34/38] Do not rely on operator short-circuiting in macros --- src/tscore/ink_queue.cc | 12 ++++++++++-- src/tscore/unit_tests/test_ink_queue.cc | 14 ++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 9761f0e77ec..bfe726ac10e 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -52,7 +52,15 @@ #include "swoc/bwf_ip.h" -#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) +#ifdef __SANITIZE_ADDRESS__ +#define TS_ASAN_ENABLED +#elif defined(__has_feature) +#if __has_feature(address_sanitizer) +#define TS_ASAN_ENABLED +#endif +#endif + +#ifdef TS_ASAN_ENABLED #include #endif @@ -300,7 +308,7 @@ freelist_new(InkFreeList *f) // pointers; through that scan every cell on the chain becomes reachable and // is reclassified as "still reachable." Registration happens once per chunk // (amortized over chunk_size cells) and is a no-op in non-ASan builds. -#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) +#ifdef TS_ASAN_ENABLED __lsan_register_root_region(newp, INK_ALIGN(alloc_size, alignment)); #endif SET_FREELIST_POINTER_VERSION(item, FROM_PTR(newp), 0); diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index 7d5355e4092..840e1930085 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -36,7 +36,15 @@ // as direct leaks. The one issued cell is reachable via the registered root // region; the remaining chunk_size-1 seeded cells are also covered because // the entire chunk is a registered LSan root region. -#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) +#ifdef __SANITIZE_ADDRESS__ +#define TS_ASAN_ENABLED +#elif defined(__has_feature) +#if __has_feature(address_sanitizer) +#define TS_ASAN_ENABLED +#endif +#endif + +#ifdef TS_ASAN_ENABLED TEST_CASE("FreelistChunkSeededCellsNotDirectLeak", "[freelist][lsan]") { InkFreeList *f{ink_freelist_create("lsan_seeded", 8, 8, 8)}; @@ -51,11 +59,9 @@ TEST_CASE("FreelistChunkSeededCellsNotDirectLeak", "[freelist][lsan]") // root region, not as direct leaks. CHECK(true); } -#endif // Caller-returned cells on the free-chain at process exit must not // appear as direct leaks. -#if defined(__SANITIZE_ADDRESS__) || (defined(__has_feature) && __has_feature(address_sanitizer)) TEST_CASE("FreelistReturnedCellsNotDirectLeak", "[freelist][lsan]") { InkFreeList *f{ink_freelist_create("lsan_returned", 8, 8, 8)}; @@ -76,7 +82,7 @@ TEST_CASE("FreelistReturnedCellsNotDirectLeak", "[freelist][lsan]") // region, not as direct leaks. CHECK(true); } -#endif +#endif // TS_ASAN_ENABLED TEST_CASE("Freelist", "[freelist]") { From b6f0f4befc84eb422669ed9438d9374f5c36ad73 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 12 Jun 2026 13:42:08 -0500 Subject: [PATCH 35/38] Make changes suggested by Copilot * Use `to_voidp_p` consistently --- src/tscore/ink_queue.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index bfe726ac10e..141be20a3c9 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -328,7 +328,7 @@ freelist_new(InkFreeList *f) freelist_free(f, a); } } else { - void **next_ptr = reinterpret_cast(TO_PTR(FREELIST_POINTER(item))); + void **next_ptr = to_voidp_p(TO_PTR(FREELIST_POINTER(item)), 0); SET_FREELIST_POINTER_VERSION(next, *next_ptr, FREELIST_VERSION(item) + 1); result = f->head.compare_exchange_weak(item, next, std::memory_order_acquire, std::memory_order_acquire); @@ -471,7 +471,7 @@ freelist_bulkfree(InkFreeList *f, void *head, void *tail, [[maybe_unused]] size_ static const char str[4] = {static_cast(0xde), static_cast(0xad), static_cast(0xbe), static_cast(0xef)}; // set the entire item to DEADBEEF; - void **temp = reinterpret_cast(head); + void **temp = to_voidp_p(head, 0); for (size_t i = 0; i < num_item; i++) { for (int j = sizeof(void *); j < static_cast(f->type_size); j++) { (reinterpret_cast(temp))[j] = str[j % 4]; @@ -635,13 +635,13 @@ ink_atomiclist_pop(InkAtomicList *l) if (TO_PTR(FREELIST_POINTER(item)) == nullptr) { return nullptr; } - void **next_ptr = reinterpret_cast(reinterpret_cast(TO_PTR(FREELIST_POINTER(item))) + l->offset); + void **next_ptr = to_voidp_p(reinterpret_cast(TO_PTR(FREELIST_POINTER(item))), l->offset); SET_FREELIST_POINTER_VERSION(next, *next_ptr, FREELIST_VERSION(item) + 1); result = l->head.compare_exchange_weak(item, next); } while (result == false); void *ret = TO_PTR(FREELIST_POINTER(item)); - void **ret_ = reinterpret_cast(reinterpret_cast(ret) + l->offset); + void **ret_ = to_voidp_p(reinterpret_cast(ret), l->offset); *ret_ = nullptr; return ret; } From 23fdc0d7b542c166d871e55d9439c3206fb8579d Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Fri, 12 Jun 2026 13:59:16 -0500 Subject: [PATCH 36/38] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/tscore/unit_tests/test_ink_queue.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tscore/unit_tests/test_ink_queue.cc b/src/tscore/unit_tests/test_ink_queue.cc index 840e1930085..98f1ed169e5 100644 --- a/src/tscore/unit_tests/test_ink_queue.cc +++ b/src/tscore/unit_tests/test_ink_queue.cc @@ -25,12 +25,9 @@ #include -#include #include #include #include -#include -#include // Chunk-seeded cells on the free-chain at process exit must not appear // as direct leaks. The one issued cell is reachable via the registered root From 9db8b8126821d0f93018e0fc4a4ce2df998a6342 Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Fri, 12 Jun 2026 14:42:04 -0500 Subject: [PATCH 37/38] Try out supposed lsan_ignore fix --- src/tscore/ink_queue.cc | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 141be20a3c9..ecfc388402c 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -299,17 +299,16 @@ freelist_new(InkFreeList *f) if (f->advice) { ats_madvise(static_cast(newp), INK_ALIGN(alloc_size, alignment), f->advice); } - // LSan root-region registration. Each cell on the free-chain stores its - // successor as a bitwise-tagged pointer (version bits merged into the high - // bits of the word by SET_FREELIST_POINTER_VERSION / FROM_PTR). LSan's - // pointer scanner sees the tagged word and cannot recognize it as a heap - // pointer, so it classifies the cells as direct leaks. Registering the - // entire chunk as a root region tells LSan to scan the chunk's bytes for - // pointers; through that scan every cell on the chain becomes reachable and - // is reclassified as "still reachable." Registration happens once per chunk - // (amortized over chunk_size cells) and is a no-op in non-ASan builds. + // Tell LSan not to report this chunk as a direct leak. Cells on the + // free-chain store their successor as a bitwise-tagged pointer (version + // bits merged into bits 48-62 by SET_FREELIST_POINTER_VERSION), which + // exceeds LSan's CanBeAHeapPointer threshold on x86_64/aarch64. LSan + // therefore cannot trace from any root through the chain to the chunk, + // and reports it as a direct leak. __lsan_ignore_object suppresses the + // report for the chunk itself; one call per chunk allocation, no + // per-cell cost. #ifdef TS_ASAN_ENABLED - __lsan_register_root_region(newp, INK_ALIGN(alloc_size, alignment)); + __lsan_ignore_object(newp); #endif SET_FREELIST_POINTER_VERSION(item, FROM_PTR(newp), 0); ink_assert(is_next_ptr_aligned(f, item)); From 8549d85ad7ab2d6ed85a044ddbb31e8b21ad7dab Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Sat, 13 Jun 2026 08:32:31 -0500 Subject: [PATCH 38/38] Make changes suggested by Copilot * Mutually exclude `ink_atomiclist_popall` and `ink_atomiclist_pop` --- src/tscore/ink_queue.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index ecfc388402c..8d6ada965c7 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -652,6 +652,16 @@ ink_atomiclist_popall(InkAtomicList *l) head_p next; bool result = false; + // Later in this routine, we need to walk the freelist chain to restore + // next pointers. Because `ink_atomiclist_pop` reads one of the pointers + // in the chain after loading the head, we need to mutually exclude + // the walk in this routine (which writes) from the read in + // `ink_atomiclist_pop`. We do not need to acquire the lock this early, + // but freelist benchmarks suggest that serializing pop operations is + // more efficient than CAS loops under contention in these types of + // data structures. We acquire the lock early for that reason. + std::lock_guard guard{l->m}; + item = l->head.load(); do { if (TO_PTR(FREELIST_POINTER(item)) == nullptr) {