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

@@ -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<Strategy> *)self)->fStrategy.ReturnSlab(slab);
+	}
+
 	Strategy fStrategy;
 };
 
-#endif
+#endif /* __cplusplus */
 
 #endif

headers/private/kernel/slab/HashStrategy.h

@@ -133,8 +133,8 @@ struct HashCacheStrategy : BaseCacheStrategy<Backend>, BaseHashCacheStrategy {
 
 		// it's very important that we cast this to BaseHashCacheStrategy
 		// so we get the proper instance offset through void *
-		return BaseCacheStrategy<Backend>::_ConstructSlab(slab, pages, 0,
+		return BaseCacheStrategy<Backend>::_ConstructSlab(slab, pages,
-			_Linkage, (BaseHashCacheStrategy *)this);
+			_SlabSize(), _Linkage, (BaseHashCacheStrategy *)this);
 	}
 
 	void ReturnSlab(BaseSlab *slab)

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<Backend>::_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<Backend>::SlabSize(sizeof(Slab));
+		size_t byteCount = BaseCacheStrategy<Backend>::SlabSize(sizeof(Slab));
+		if (byteCount > Backend::kMaximumAlignedLength)
+			byteCount = Backend::kMaximumAlignedLength;
+		return byteCount;
 	}
 
 	Link *_Linkage(void *object) const

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,
+		return base_cache_construct_slab(fParent, slab, pages, byteCount,
-			SlabSize(tailSpace) - tailSpace, getLink, parent);
+			getLink, parent);
 	}
 
 	void _DestructSlab(BaseSlab *slab)

headers/private/kernel/util/OpenHashTable.h

@@ -11,6 +11,7 @@
 #define _OPEN_HASH_TABLE_H_
 
 #include <KernelExport.h>
+#include <util/kernel_cpp.h>
 
 // the Definition template must have four methods: `HashKey', `Hash',
 // `Compare' and `GetLink;. It must also define several types as shown in the

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",
+	if (list_is_empty(&cache->partial)) {
-		slab, slab->count);
+		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,
+	TRACE_CACHE(cache, "construct (%p, %p, %lu)", slab, pages, byteCount);
-		byteCount, getLink, parent);
 
 	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++) {