From 81bc5709221d7cfd6696e1422b388fd01e5cbff6 Mon Sep 17 00:00:00 2001 From: Hugo Santos Date: Thu, 26 Apr 2007 06:05:08 +0000 Subject: [PATCH] assorted slab fixes. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@20833 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/private/kernel/slab/Base.h | 34 ++++++-- headers/private/kernel/slab/HashStrategy.h | 4 +- headers/private/kernel/slab/MergedStrategy.h | 10 +-- headers/private/kernel/slab/Strategy.h | 6 +- headers/private/kernel/util/OpenHashTable.h | 1 + src/system/kernel/slab/Slab.cpp | 87 +++++++++++++++++--- 6 files changed, 116 insertions(+), 26 deletions(-) diff --git a/headers/private/kernel/slab/Base.h b/headers/private/kernel/slab/Base.h index 497894087d..3026d85009 100644 --- a/headers/private/kernel/slab/Base.h +++ b/headers/private/kernel/slab/Base.h @@ -25,13 +25,16 @@ static const int kMinimumSlabItems = 32; typedef void (*base_cache_constructor)(void *cookie, void *object); typedef void (*base_cache_destructor)(void *cookie, void *object); -/* base Slab implementation, opaque to the backend used. */ +/* base Slab implementation, opaque to the backend used. + * + * NOTE: the caller is responsible for the Cache's locking. */ typedef struct base_cache { char name[32]; size_t object_size; size_t cache_color_cycle; - struct list partial, full; + struct list empty, partial, full; + size_t empty_count; base_cache_constructor constructor; base_cache_destructor destructor; void *cookie; @@ -51,8 +54,12 @@ typedef struct cache_slab { void base_cache_init(base_cache *cache, const char *name, size_t object_size, size_t alignment, base_cache_constructor constructor, base_cache_destructor destructor, void *cookie); +void base_cache_destroy(base_cache *cache, + void (*return_slab)(base_cache *, cache_slab *)); cache_object_link *base_cache_allocate_object(base_cache *cache); +cache_object_link *base_cache_allocate_object_with_new_slab(base_cache *cache, + cache_slab *slab); int base_cache_return_object(base_cache *cache, cache_slab *slab, cache_object_link *link); @@ -82,16 +89,26 @@ public: destructor, cookie); } + ~Cache() + { + base_cache_destroy(this, _ReturnSlab); + } + void *AllocateObject(uint32_t flags) { - if (list_is_empty(&partial)) { + cache_object_link *link = base_cache_allocate_object(this); + + // if the cache is returning NULL it is because it ran out of slabs + if (link == NULL) { cache_slab *newSlab = fStrategy.NewSlab(flags); if (newSlab == NULL) return NULL; - list_add_item(&partial, newSlab); + link = base_cache_allocate_object_with_new_slab(this, newSlab); + if (link == NULL) + panic("cache: failed to allocate with an empty slab"); } - return fStrategy.Object(base_cache_allocate_object(this)); + return fStrategy.Object(link); } void ReturnObject(void *object) @@ -103,9 +120,14 @@ public: } private: + static void _ReturnSlab(base_cache *self, cache_slab *slab) + { + ((Cache *)self)->fStrategy.ReturnSlab(slab); + } + Strategy fStrategy; }; -#endif +#endif /* __cplusplus */ #endif diff --git a/headers/private/kernel/slab/HashStrategy.h b/headers/private/kernel/slab/HashStrategy.h index c9ed6072e9..7c44b00b8d 100644 --- a/headers/private/kernel/slab/HashStrategy.h +++ b/headers/private/kernel/slab/HashStrategy.h @@ -133,8 +133,8 @@ struct HashCacheStrategy : BaseCacheStrategy, BaseHashCacheStrategy { // it's very important that we cast this to BaseHashCacheStrategy // so we get the proper instance offset through void * - return BaseCacheStrategy::_ConstructSlab(slab, pages, 0, - _Linkage, (BaseHashCacheStrategy *)this); + return BaseCacheStrategy::_ConstructSlab(slab, pages, + _SlabSize(), _Linkage, (BaseHashCacheStrategy *)this); } void ReturnSlab(BaseSlab *slab) diff --git a/headers/private/kernel/slab/MergedStrategy.h b/headers/private/kernel/slab/MergedStrategy.h index 221f4f3357..339b29cb9e 100644 --- a/headers/private/kernel/slab/MergedStrategy.h +++ b/headers/private/kernel/slab/MergedStrategy.h @@ -57,9 +57,6 @@ public: void *pages; size_t byteCount = _SlabSize(); - if (byteCount > Backend::kMaximumAlignedLength) - byteCount = Backend::kMaximumAlignedLength; - // in order to save a pointer per object or a hash table to // map objects to slabs we required this set of pages to be // aligned in a (pageCount * PAGE_SIZE) boundary. @@ -70,7 +67,7 @@ public: _SlabInPages(pages)->id = id; return BaseCacheStrategy::_ConstructSlab(_SlabInPages(pages), - pages, sizeof(Slab), _Linkage, this); + pages, _SlabSize() - sizeof(Slab), _Linkage, this); } void ReturnSlab(BaseSlab *slab) @@ -81,7 +78,10 @@ public: private: size_t _SlabSize() const { - return BaseCacheStrategy::SlabSize(sizeof(Slab)); + size_t byteCount = BaseCacheStrategy::SlabSize(sizeof(Slab)); + if (byteCount > Backend::kMaximumAlignedLength) + byteCount = Backend::kMaximumAlignedLength; + return byteCount; } Link *_Linkage(void *object) const diff --git a/headers/private/kernel/slab/Strategy.h b/headers/private/kernel/slab/Strategy.h index d68950c2cb..b0e26262f1 100644 --- a/headers/private/kernel/slab/Strategy.h +++ b/headers/private/kernel/slab/Strategy.h @@ -34,11 +34,11 @@ protected: typename Backend::AllocationID id; }; - BaseSlab *_ConstructSlab(Slab *slab, void *pages, size_t tailSpace, + BaseSlab *_ConstructSlab(Slab *slab, void *pages, size_t byteCount, ObjectLink *(*getLink)(void *parent, void *object), void *parent) { - return base_cache_construct_slab(fParent, slab, pages, - SlabSize(tailSpace) - tailSpace, getLink, parent); + return base_cache_construct_slab(fParent, slab, pages, byteCount, + getLink, parent); } void _DestructSlab(BaseSlab *slab) diff --git a/headers/private/kernel/util/OpenHashTable.h b/headers/private/kernel/util/OpenHashTable.h index ab3cd2c997..684f78858f 100644 --- a/headers/private/kernel/util/OpenHashTable.h +++ b/headers/private/kernel/util/OpenHashTable.h @@ -11,6 +11,7 @@ #define _OPEN_HASH_TABLE_H_ #include +#include // the Definition template must have four methods: `HashKey', `Hash', // `Compare' and `GetLink;. It must also define several types as shown in the diff --git a/src/system/kernel/slab/Slab.cpp b/src/system/kernel/slab/Slab.cpp index b36e0dbf4d..7a404c4fcb 100644 --- a/src/system/kernel/slab/Slab.cpp +++ b/src/system/kernel/slab/Slab.cpp @@ -21,6 +21,15 @@ // same code. We'll have to resolve all of the dependencies // then, for now, it is still not required. +//#define TRACE_SLAB + +#ifdef TRACE_SLAB +#define TRACE_CACHE(cache, format, args...) \ + dprintf("Cache[%p, %s] " format "\n", cache, cache->name , ##args) +#else +#define TRACE_CACHE(cache, format, bananas...) do { } while (0) +#endif + // TODO this value should be dynamically tuned per cache. static const int kMagazineCapacity = 32; @@ -54,14 +63,14 @@ slab_area_backend_allocate(base_cache *cache, area_id *id, void **pages, if (flags & CACHE_ALIGN_TO_TOTAL && byteCount > B_PAGE_SIZE) return NULL; - dprintf("AreaBackend::AllocatePages(%lu, 0x%lx)\n", byteCount, flags); + TRACE_CACHE(cache, "allocate pages (%lu, 0x0%lx)", byteCount, flags); area_id areaId = create_area(cache->name, pages, B_ANY_KERNEL_ADDRESS, byteCount, B_NO_LOCK, B_READ_AREA | B_WRITE_AREA); if (areaId < 0) return areaId; - dprintf(" AreaBackend::AllocatePages() = { %ld, %p }\n", areaId, *pages); + TRACE_CACHE(cache, " ... = { %ld, %p }", areaId, *pages); *id = areaId; return B_OK; @@ -70,7 +79,7 @@ slab_area_backend_allocate(base_cache *cache, area_id *id, void **pages, void slab_area_backend_free(base_cache *cache, area_id area) { - dprintf("AreaBackend::DeletePages(%ld)\n", area); + TRACE_CACHE(cache, "delete pages %ld", area); delete_area(area); } @@ -82,6 +91,8 @@ base_cache_init(base_cache *cache, const char *name, size_t objectSize, { strlcpy(cache->name, name, sizeof(cache->name)); + TRACE_CACHE(cache, "init %lu, %lu", objectSize, alignment); + if (alignment > 0 && (objectSize & (alignment - 1))) cache->object_size = objectSize + alignment - (objectSize & (alignment - 1)); @@ -90,18 +101,53 @@ base_cache_init(base_cache *cache, const char *name, size_t objectSize, cache->cache_color_cycle = 0; + list_init_etc(&cache->empty, offsetof(cache_slab, link)); list_init_etc(&cache->partial, offsetof(cache_slab, link)); list_init_etc(&cache->full, offsetof(cache_slab, link)); + + cache->empty_count = 0; + + cache->constructor = constructor; + cache->destructor = destructor; + cache->cookie = cookie; +} + + +void +base_cache_destroy(base_cache *cache, + void (*return_slab)(base_cache *, cache_slab *)) +{ + if (!list_is_empty(&cache->full)) + panic("cache destroy: still has full slabs"); + + if (!list_is_empty(&cache->partial)) + panic("cache destroy: still has partial slabs"); + + while (!list_is_empty(&cache->empty)) { + cache_slab *slab = (cache_slab *)list_remove_head_item(&cache->empty); + return_slab(cache, slab); + } + + cache->empty_count = 0; } cache_object_link * base_cache_allocate_object(base_cache *cache) { - cache_slab *slab = (cache_slab *)list_get_first_item(&cache->partial); + cache_slab *slab; - dprintf("BaseCache::AllocateObject() from %p, %lu remaining\n", - slab, slab->count); + if (list_is_empty(&cache->partial)) { + if (list_is_empty(&cache->empty)) + return NULL; + + cache->empty_count--; + slab = (cache_slab *)list_remove_head_item(&cache->empty); + list_add_item(&cache->partial, slab); + } else + slab = (cache_slab *)list_get_first_item(&cache->partial); + + TRACE_CACHE(cache, "allocate from %p, %lu remaining.", slab, slab->count); cache_object_link *link = SListPop(slab->free); slab->count--; @@ -115,17 +161,34 @@ base_cache_allocate_object(base_cache *cache) } +cache_object_link * +base_cache_allocate_object_with_new_slab(base_cache *cache, + cache_slab *newSlab) +{ + list_add_item(&cache->partial, newSlab); + return base_cache_allocate_object(cache); +} + + int base_cache_return_object(base_cache *cache, cache_slab *slab, cache_object_link *link) { // We return true if the slab is completely unused. + TRACE_CACHE(cache, "returning %p to %p, %lu used (%lu empty slabs).", + link, slab, slab->size - slab->count, cache->empty_count); + SListPush(slab->free, link); slab->count++; if (slab->count == slab->size) { list_remove_item(&cache->partial, slab); - return 1; + + if (cache->empty_count > 2) + return 1; + + cache->empty_count++; + list_add_item(&cache->empty, slab); } else if (slab->count == 1) { list_remove_item(&cache->full, slab); list_add_item(&cache->partial, slab); @@ -140,8 +203,7 @@ base_cache_construct_slab(base_cache *cache, cache_slab *slab, void *pages, size_t byteCount, cache_object_link *(*getLink)(void *parent, void *object), void *parent) { - dprintf("BaseCache::ConstructSlab(%p, %p, %lu, %p, %p)\n", slab, pages, - byteCount, getLink, parent); + TRACE_CACHE(cache, "construct (%p, %p, %lu)", slab, pages, byteCount); slab->pages = pages; slab->count = slab->size = byteCount / cache->object_size; @@ -155,7 +217,7 @@ base_cache_construct_slab(base_cache *cache, cache_slab *slab, void *pages, else cache->cache_color_cycle += kCacheColorPeriod; - dprintf(" %lu objects, %lu spare bytes, cycle %lu\n", + TRACE_CACHE(cache, " %lu objects, %lu spare bytes, cycle %lu", slab->size, spareBytes, cycle); uint8_t *data = ((uint8_t *)pages) + cycle; @@ -174,9 +236,14 @@ base_cache_construct_slab(base_cache *cache, cache_slab *slab, void *pages, void base_cache_destruct_slab(base_cache *cache, cache_slab *slab) { + TRACE_CACHE(cache, "destruct %p", slab); + if (cache->destructor == NULL) return; + if (slab->count != slab->size) + panic("cache: destroying a slab which isn't empty."); + uint8_t *data = (uint8_t *)slab->pages; for (size_t i = 0; i < slab->size; i++) {