From: Eduard Bagdasaryan Date: Tue, 24 Dec 2019 18:10:49 +0000 (+0000) Subject: Centralized PagePool/PageStack ID generation (#525) X-Git-Tag: SQUID_5_0_1~6 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1fe7f70fb1b9f57e87549c0a64921e19a38079c0;p=thirdparty%2Fsquid.git Centralized PagePool/PageStack ID generation (#525) Easy-to-find-in-cache.log and predictable/stable stack IDs for shared memory pages and/or index slot numbers are very useful when debugging cache metadata corruption issues because they allow to track related (e.g. same-stack) operations across huge SMP logs. --- diff --git a/src/MemStore.cc b/src/MemStore.cc index 93a9807dba..c6081ec469 100644 --- a/src/MemStore.cc +++ b/src/MemStore.cc @@ -30,11 +30,6 @@ static const char *SpaceLabel = "cache_mem_space"; static const char *ExtrasLabel = "cache_mem_ex"; // TODO: sync with Rock::SwapDir::*Path() -// We store free slot IDs (i.e., "space") as Page objects so that we can use -// Ipc::Mem::PageStack. Pages require pool IDs. The value here is not really -// used except for a positivity test. A unique value is handy for debugging. -static const uint32_t SpacePoolId = 510716; - /// Packs to shared memory, allocating new slots/pages as needed. /// Requires an Ipc::StoreMapAnchor locked for writing. class ShmWriter: public Packable @@ -810,7 +805,7 @@ MemStore::noteFreeMapSlice(const Ipc::StoreMapSliceId sliceId) debugs(20, 9, "slice " << sliceId << " freed " << pageId); assert(pageId); Ipc::Mem::PageId slotId; - slotId.pool = SpacePoolId; + slotId.pool = Ipc::Mem::PageStack::IdForMemStoreSpace(); slotId.number = sliceId + 1; if (!waitingFor) { // must zero pageId before we give slice (and pageId extras!) to others @@ -1014,7 +1009,8 @@ MemStoreRr::create() const int64_t entryLimit = MemStore::EntryLimit(); assert(entryLimit > 0); Must(!spaceOwner); - spaceOwner = shm_new(Ipc::Mem::PageStack)(SpaceLabel, SpacePoolId, + spaceOwner = shm_new(Ipc::Mem::PageStack)(SpaceLabel, + Ipc::Mem::PageStack::IdForMemStoreSpace(), entryLimit, 0); Must(!mapOwner); mapOwner = MemStoreMap::Init(MapLabel, entryLimit); diff --git a/src/fs/rock/RockRebuild.cc b/src/fs/rock/RockRebuild.cc index 00e0cb6ccb..b83c367a4a 100644 --- a/src/fs/rock/RockRebuild.cc +++ b/src/fs/rock/RockRebuild.cc @@ -593,7 +593,7 @@ Rock::Rebuild::freeSlot(const SlotId slotId, const bool invalid) } Ipc::Mem::PageId pageId; - pageId.pool = sd->index+1; + pageId.pool = Ipc::Mem::PageStack::IdForSwapDirSpace(sd->index); pageId.number = slotId+1; sd->freeSlots->push(pageId); } diff --git a/src/fs/rock/RockSwapDir.cc b/src/fs/rock/RockSwapDir.cc index 6e77b63f89..b2fa699100 100644 --- a/src/fs/rock/RockSwapDir.cc +++ b/src/fs/rock/RockSwapDir.cc @@ -754,7 +754,7 @@ void Rock::SwapDir::noteFreeMapSlice(const Ipc::StoreMapSliceId sliceId) { Ipc::Mem::PageId pageId; - pageId.pool = index+1; + pageId.pool = Ipc::Mem::PageStack::IdForSwapDirSpace(index); pageId.number = sliceId+1; if (waitingForPage) { *waitingForPage = pageId; @@ -1138,7 +1138,9 @@ void Rock::SwapDirRr::create() // TODO: somehow remove pool id and counters from PageStack? Ipc::Mem::Owner *const freeSlotsOwner = shm_new(Ipc::Mem::PageStack)(sd->freeSlotsPath(), - i+1, capacity, 0); + Ipc::Mem::PageStack::IdForSwapDirSpace(i), + capacity, + 0); freeSlotsOwners.push_back(freeSlotsOwner); // TODO: add method to initialize PageStack with no free pages diff --git a/src/ipc/Makefile.am b/src/ipc/Makefile.am index f4ac289962..9b34027682 100644 --- a/src/ipc/Makefile.am +++ b/src/ipc/Makefile.am @@ -64,7 +64,8 @@ libipc_la_SOURCES = \ mem/PageStack.h \ mem/Pointer.h \ mem/Segment.cc \ - mem/Segment.h + mem/Segment.h \ + mem/forward.h DEFS += -DDEFAULT_STATEDIR=\"$(localstatedir)/run/squid\" diff --git a/src/ipc/mem/Page.h b/src/ipc/mem/Page.h index a9e36db0a5..3c42e2f732 100644 --- a/src/ipc/mem/Page.h +++ b/src/ipc/mem/Page.h @@ -9,6 +9,8 @@ #ifndef SQUID_IPC_MEM_PAGE_H #define SQUID_IPC_MEM_PAGE_H +#include "ipc/mem/forward.h" + #include namespace Ipc @@ -30,7 +32,12 @@ public: typedef const uint32_t PageId::*SaferBool; operator SaferBool() const { return set() ? &PageId::number : NULL; } - uint32_t pool; ///< page pool ID within Squid + /// The ID of a PagePool (and/or PageStack) this page belongs to. + /// Positive values are (ab)used to detect in-use pages. See set(). + /// Eventually, they may identify a PageStack in a multi-segment PagePool. + /// These IDs also distinguish page pools/stacks in debugging logs. + PoolId pool; + // uint32_t segment; ///< memory segment ID within the pool; unused for now uint32_t number; ///< page number within the segment diff --git a/src/ipc/mem/PagePool.cc b/src/ipc/mem/PagePool.cc index 09846c1662..292f2870a9 100644 --- a/src/ipc/mem/PagePool.cc +++ b/src/ipc/mem/PagePool.cc @@ -16,12 +16,9 @@ // Ipc::Mem::PagePool Ipc::Mem::PagePool::Owner * -Ipc::Mem::PagePool::Init(const char *const id, const unsigned int capacity, const size_t pageSize) +Ipc::Mem::PagePool::Init(const char *const shmId, const Ipc::Mem::PoolId stackId, const unsigned int capacity, const size_t pageSize) { - static uint32_t LastPagePoolId = 0; - if (++LastPagePoolId == 0) - ++LastPagePoolId; // skip zero pool id - return shm_new(PageStack)(id, LastPagePoolId, capacity, pageSize); + return shm_new(PageStack)(shmId, stackId, capacity, pageSize); } Ipc::Mem::PagePool::PagePool(const char *const id): diff --git a/src/ipc/mem/PagePool.h b/src/ipc/mem/PagePool.h index 61a433ef12..479c9f4186 100644 --- a/src/ipc/mem/PagePool.h +++ b/src/ipc/mem/PagePool.h @@ -27,7 +27,7 @@ class PagePool public: typedef Ipc::Mem::Owner Owner; - static Owner *Init(const char *const id, const unsigned int capacity, const size_t pageSize); + static Owner *Init(const char *const shmId, const Ipc::Mem::PoolId stackId, const unsigned int capacity, const size_t pageSize); PagePool(const char *const id); diff --git a/src/ipc/mem/PageStack.cc b/src/ipc/mem/PageStack.cc index f3149064c3..1f02e35f38 100644 --- a/src/ipc/mem/PageStack.cc +++ b/src/ipc/mem/PageStack.cc @@ -37,12 +37,14 @@ Ipc::Mem::PageStackStorageSlot::put(const PointerOrMarker expected, const Pointe /* Ipc::Mem::PageStack */ -Ipc::Mem::PageStack::PageStack(const uint32_t aPoolId, const PageCount aCapacity, const size_t aPageSize): +Ipc::Mem::PageStack::PageStack(const PoolId aPoolId, const PageCount aCapacity, const size_t aPageSize): thePoolId(aPoolId), capacity_(aCapacity), thePageSize(aPageSize), size_(0), head_(Slot::NilPtr), slots_(aCapacity) { + assert(thePoolId); + assert(capacity_ < Slot::TakenPage); assert(capacity_ < Slot::NilPtr); @@ -123,7 +125,7 @@ Ipc::Mem::PageStack::sharedMemorySize() const } size_t -Ipc::Mem::PageStack::SharedMemorySize(const uint32_t, const PageCount capacity, const size_t pageSize) +Ipc::Mem::PageStack::SharedMemorySize(const PoolId, const PageCount capacity, const size_t pageSize) { const auto levelsSize = PageId::maxPurpose * sizeof(Levels_t); const size_t pagesDataSize = capacity * pageSize; diff --git a/src/ipc/mem/PageStack.h b/src/ipc/mem/PageStack.h index eb761fb92b..11e2921465 100644 --- a/src/ipc/mem/PageStack.h +++ b/src/ipc/mem/PageStack.h @@ -10,6 +10,7 @@ #define SQUID_IPC_MEM_PAGE_STACK_H #include "ipc/mem/FlexibleArray.h" +#include "ipc/mem/forward.h" #include #include @@ -71,7 +72,7 @@ public: /// the number of (free and/or used) pages in a stack typedef unsigned int PageCount; - PageStack(const uint32_t aPoolId, const PageCount aCapacity, const size_t aPageSize); + PageStack(const PoolId aPoolId, const PageCount aCapacity, const size_t aPageSize); PageCount capacity() const { return capacity_; } size_t pageSize() const { return thePageSize; } @@ -86,7 +87,7 @@ public: bool pageIdIsValid(const PageId &page) const; /// total shared memory size required to share - static size_t SharedMemorySize(const uint32_t aPoolId, const PageCount capacity, const size_t pageSize); + static size_t SharedMemorySize(const PoolId aPoolId, const PageCount capacity, const size_t pageSize); size_t sharedMemorySize() const; /// shared memory size required only by PageStack, excluding @@ -98,13 +99,26 @@ public: static size_t LevelsPaddingSize(const PageCount capacity); size_t levelsPaddingSize() const { return LevelsPaddingSize(capacity_); } + /** + * The following functions return PageStack IDs for the corresponding + * PagePool or a similar PageStack user. The exact values are unimportant, + * but their uniqueness and stability eases debugging. + */ + + /// stack of free cache_mem slot positions + static PoolId IdForMemStoreSpace() { return 10; } + /// multipurpose PagePool of shared memory pages + static PoolId IdForMultipurposePool() { return 200; } // segments could use 2xx + /// stack of free rock cache_dir slot numbers + static PoolId IdForSwapDirSpace(const int dirIdx) { return 900 + dirIdx + 1; } + private: using Slot = PageStackStorageSlot; // XXX: theFoo members look misplaced due to messy separation of PagePool // (which should support multiple Segments but does not) and PageStack // (which should not calculate the Segment size but does) duties. - const uint32_t thePoolId; ///< pool ID + const PoolId thePoolId; ///< pool ID const PageCount capacity_; ///< the maximum number of pages const size_t thePageSize; ///< page size, used to calculate shared memory size /// a lower bound for the number of free pages (for debugging purposes) diff --git a/src/ipc/mem/Pages.cc b/src/ipc/mem/Pages.cc index f8b5f6b3f8..8dfa0957d9 100644 --- a/src/ipc/mem/Pages.cc +++ b/src/ipc/mem/Pages.cc @@ -118,7 +118,9 @@ void SharedMemPagesRr::create() { Must(!owner); - owner = Ipc::Mem::PagePool::Init(PagePoolId, Ipc::Mem::PageLimit(), + owner = Ipc::Mem::PagePool::Init(PagePoolId, + Ipc::Mem::PageStack::IdForMultipurposePool(), + Ipc::Mem::PageLimit(), Ipc::Mem::PageSize()); } diff --git a/src/ipc/mem/forward.h b/src/ipc/mem/forward.h new file mode 100644 index 0000000000..c44f4ce9d6 --- /dev/null +++ b/src/ipc/mem/forward.h @@ -0,0 +1,23 @@ +/* + * Copyright (C) 1996-2019 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#ifndef SQUID_IPC_MEM_FORWARD_H +#define SQUID_IPC_MEM_FORWARD_H + +namespace Ipc +{ + +namespace Mem +{ + typedef uint32_t PoolId; +} // namespace Mem + +} // namespace Ipc + +#endif /* SQUID_IPC_MEM_FORWARD_H */ +