Incorporated hash and block cache changes of r20368 and r20374.

git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@20375 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/file_systems/userlandfs/server/haiku_block_cache.cpp

@@ -105,7 +105,6 @@ transaction_hash(void *_transaction, const void *_id, uint32 range)
 static void
 delete_transaction(block_cache *cache, cache_transaction *transaction)
 {
-	hash_remove(cache->transaction_hash, transaction);
 	if (cache->last_transaction == transaction)
 		cache->last_transaction = NULL;
 
@@ -152,7 +151,8 @@ cached_block::Hash(void *_cacheEntry, const void *_block, uint32 range)
 //	#pragma mark - block_cache
 
 
-block_cache::block_cache(int _fd, off_t numBlocks, size_t blockSize, bool readOnly)
+block_cache::block_cache(int _fd, off_t numBlocks, size_t blockSize,
+	bool readOnly)
 	:
 	hash(NULL),
 	fd(_fd),
@@ -222,8 +222,10 @@ block_cache::FreeBlock(cached_block *block)
 	Free(block->current_data);
 	block->current_data = NULL;
 
-	if (block->original_data != NULL || block->parent_data != NULL)
+	if (block->original_data != NULL || block->parent_data != NULL) {
-		panic("block_cache::FreeBlock(): %p, %p\n", block->original_data, block->parent_data);
+		panic("block_cache::FreeBlock(): %p, %p\n", block->original_data,
+			block->parent_data);
+	}
 
 #ifdef DEBUG_CHANGED
 	Free(block->compare);
@@ -233,6 +235,7 @@ block_cache::FreeBlock(cached_block *block)
 }
 
 
+/*! Allocates a new block for \a blockNumber, ready for use */
 cached_block *
 block_cache::NewBlock(off_t blockNumber)
 {
@@ -268,7 +271,6 @@ block_cache::NewBlock(off_t blockNumber)
 	block->compare = NULL;
 #endif
 
-	hash_insert(hash, block);
 	allocated_block_count++;
 
 	return block;
@@ -281,7 +283,8 @@ block_cache::RemoveUnusedBlocks(int32 maxAccessed, int32 count)
 	TRACE(("block_cache: remove up to %ld unused blocks\n", count));
 
 	cached_block *next = NULL;
-	for (cached_block *block = unused_blocks.First(); block != NULL; block = next) {
+	for (cached_block *block = unused_blocks.First(); block != NULL;
+			block = next) {
 		next = block->next;
 
 		if (maxAccessed < block->accessed)
@@ -306,14 +309,15 @@ block_cache::RemoveUnusedBlocks(int32 maxAccessed, int32 count)
 }
 
 
-//	#pragma mark -
+//	#pragma mark - private block functions
 
 
 static void
 put_cached_block(block_cache *cache, cached_block *block)
 {
 #ifdef DEBUG_CHANGED
-	if (!block->is_dirty && block->compare != NULL && memcmp(block->current_data, block->compare, cache->block_size)) {
+	if (!block->is_dirty && block->compare != NULL
+		&& memcmp(block->current_data, block->compare, cache->block_size)) {
 		dprintf("new block:\n");
 		dump_block((const char *)block->current_data, 256, "  ");
 		dprintf("unchanged block:\n");
@@ -344,8 +348,10 @@ put_cached_block(block_cache *cache, cached_block *block)
 static void
 put_cached_block(block_cache *cache, off_t blockNumber)
 {
-	if (blockNumber < 0 || blockNumber >= cache->max_blocks)
+	if (blockNumber < 0 || blockNumber >= cache->max_blocks) {
-		panic("put_cached_block: invalid block number %lld (max %lld)", blockNumber, cache->max_blocks - 1);
+		panic("put_cached_block: invalid block number %lld (max %lld)",
+			blockNumber, cache->max_blocks - 1);
+	}
 	
 	cached_block *block = (cached_block *)hash_lookup(cache->hash, &blockNumber);
 	if (block != NULL)
@@ -353,14 +359,29 @@ put_cached_block(block_cache *cache, off_t blockNumber)
 }
 
 
-static cached_block *
+/*!
-get_cached_block(block_cache *cache, off_t blockNumber, bool *allocated, bool readBlock = true)
+	Retrieves the block \a blockNumber from the hash table, if it's already
-{
+	there, or reads it from the disk.
-	if (blockNumber < 0 || blockNumber >= cache->max_blocks)
-		panic("get_cached_block: invalid block number %lld (max %lld)", blockNumber, cache->max_blocks - 1);
 
-	cached_block *block = (cached_block *)hash_lookup(cache->hash, &blockNumber);
+	\param _allocated tells you wether or not a new block has been allocated
-	*allocated = false;
+		to satisfy your request.
+	\param readBlock if \c false, the block will not be read in case it was
+		not already in the cache. The block you retrieve may contain random
+		data.
+*/
+static cached_block *
+get_cached_block(block_cache *cache, off_t blockNumber, bool *_allocated,
+	bool readBlock = true)
+{
+	if (blockNumber < 0 || blockNumber >= cache->max_blocks) {
+		panic("get_cached_block: invalid block number %lld (max %lld)",
+			blockNumber, cache->max_blocks - 1);
+		return NULL;
+	}
+
+	cached_block *block = (cached_block *)hash_lookup(cache->hash,
+		&blockNumber);
+	*_allocated = false;
 
 	if (block == NULL) {
 		// read block into cache
@@ -368,8 +389,10 @@ get_cached_block(block_cache *cache, off_t blockNumber, bool *allocated, bool re
 		if (block == NULL)
 			return NULL;
 
-		*allocated = true;
+		hash_insert(cache->hash, block);
+		*_allocated = true;
 	} else {
+		// TODO: currently, the data is always mapped in
 /*
 		if (block->ref_count == 0 && block->current_data != NULL) {
 			// see if the old block can be resurrected
@@ -382,15 +405,16 @@ get_cached_block(block_cache *cache, off_t blockNumber, bool *allocated, bool re
 			if (block->current_data == NULL)
 				return NULL;
 
-			*allocated = true;
+			*_allocated = true;
 		}
 */
 	}
 
-	if (*allocated && readBlock) {
+	if (*_allocated && readBlock) {
 		int32 blockSize = cache->block_size;
 
-		if (read_pos(cache->fd, blockNumber * blockSize, block->current_data, blockSize) < blockSize) {
+		if (read_pos(cache->fd, blockNumber * blockSize, block->current_data,
+				blockSize) < blockSize) {
 			hash_remove(cache->hash, block);
 			cache->FreeBlock(block);
 			FATAL(("could not read block %Ld\n", blockNumber));
@@ -411,24 +435,29 @@ get_cached_block(block_cache *cache, off_t blockNumber, bool *allocated, bool re
 }
 
 
-/**	Returns the writable block data for the requested blockNumber.
+/*!
- *	If \a cleared is true, the block is not read from disk; an empty block
+	Returns the writable block data for the requested blockNumber.
- *	is returned.
+	If \a cleared is true, the block is not read from disk; an empty block
- *	This is the only method to insert a block into a transaction. It makes
+	is returned.
- *	sure that the previous block contents are preserved in that case.
- */
 
+	This is the only method to insert a block into a transaction. It makes
+	sure that the previous block contents are preserved in that case.
+*/
 static void *
-get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base, off_t length,
+get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base,
-	int32 transactionID, bool cleared)
+	off_t length, int32 transactionID, bool cleared)
 {
-	TRACE(("get_writable_cached_block(blockNumber = %Ld, transaction = %ld)\n", blockNumber, transactionID));
+	TRACE(("get_writable_cached_block(blockNumber = %Ld, transaction = %ld)\n",
+		blockNumber, transactionID));
 
-	if (blockNumber < 0 || blockNumber >= cache->max_blocks)
+	if (blockNumber < 0 || blockNumber >= cache->max_blocks) {
-		panic("get_writable_cached_block: invalid block number %lld (max %lld)", blockNumber, cache->max_blocks - 1);
+		panic("get_writable_cached_block: invalid block number %lld (max %lld)",
+			blockNumber, cache->max_blocks - 1);
+	}
 
 	bool allocated;
-	cached_block *block = get_cached_block(cache, blockNumber, &allocated, !cleared);
+	cached_block *block = get_cached_block(cache, blockNumber, &allocated,
+		!cleared);
 	if (block == NULL)
 		return NULL;
 
@@ -454,7 +483,8 @@ get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base, off
 		// get new transaction
 		cache_transaction *transaction = lookup_transaction(cache, transactionID);
 		if (transaction == NULL) {
-			panic("get_writable_cached_block(): invalid transaction %ld!\n", transactionID);
+			panic("get_writable_cached_block(): invalid transaction %ld!\n",
+				transactionID);
 			put_cached_block(cache, block);
 			return NULL;
 		}
@@ -487,7 +517,7 @@ get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base, off
 		// remember any previous contents for the parent transaction
 		block->parent_data = cache->Allocate();
 		if (block->parent_data == NULL) {
-			// ToDo: maybe we should just continue the current transaction in this case...
+			// TODO: maybe we should just continue the current transaction in this case...
 			FATAL(("could not allocate parent\n"));
 			put_cached_block(cache, block);
 			return NULL;
@@ -507,20 +537,24 @@ get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base, off
 
 
 static status_t
-write_cached_block(block_cache *cache, cached_block *block, bool deleteTransaction)
+write_cached_block(block_cache *cache, cached_block *block,
+	bool deleteTransaction)
 {
 	cache_transaction *previous = block->previous_transaction;
 	int32 blockSize = cache->block_size;
 
-	void *data = previous && block->original_data ? block->original_data : block->current_data;
+	void *data = previous && block->original_data
+		? block->original_data : block->current_data;
 		// we first need to write back changes from previous transactions
 
 	TRACE(("write_cached_block(block %Ld)\n", block->block_number));
 
-	ssize_t written = write_pos(cache->fd, block->block_number * blockSize, data, blockSize);
+	ssize_t written = write_pos(cache->fd, block->block_number * blockSize,
+		data, blockSize);
 
 	if (written < blockSize) {
-		FATAL(("could not write back block %Ld (%s)\n", block->block_number, strerror(errno)));
+		FATAL(("could not write back block %Ld (%s)\n", block->block_number,
+			strerror(errno)));
 		return B_IO_ERROR;
 	}
 
@@ -535,11 +569,15 @@ write_cached_block(block_cache *cache, cached_block *block, bool deleteTransacti
 		if (--previous->num_blocks == 0) {
 			TRACE(("cache transaction %ld finished!\n", previous->id));
 
-			if (previous->notification_hook != NULL)
+			if (previous->notification_hook != NULL) {
-				previous->notification_hook(previous->id, previous->notification_data);
+				previous->notification_hook(previous->id,
+					previous->notification_data);
+			}
 
-			if (deleteTransaction)
+			if (deleteTransaction) {
+				hash_remove(cache->transaction_hash, previous);
 				delete_transaction(cache, previous);
+			}
 		}
 	}
 
@@ -563,8 +601,10 @@ cache_start_transaction(void *_cache)
 	block_cache *cache = (block_cache *)_cache;
 	BenaphoreLocker locker(&cache->lock);
 
-	if (cache->last_transaction && cache->last_transaction->open)
+	if (cache->last_transaction && cache->last_transaction->open) {
-		panic("last transaction (%ld) still open!\n", cache->last_transaction->id);
+		panic("last transaction (%ld) still open!\n",
+			cache->last_transaction->id);
+	}
 
 	cache_transaction *transaction = new(nothrow) cache_transaction;
 	if (transaction == NULL)
@@ -592,17 +632,21 @@ cache_sync_transaction(void *_cache, int32 id)
 	hash_open(cache->transaction_hash, &iterator);
 
 	cache_transaction *transaction;
-	while ((transaction = (cache_transaction *)hash_next(cache->transaction_hash, &iterator)) != NULL) {
+	while ((transaction = (cache_transaction *)hash_next(
-		// ToDo: fix hash interface to make this easier
+			cache->transaction_hash, &iterator)) != NULL) {
+		// close all earlier transactions which haven't been closed yet
 
 		if (transaction->id <= id && !transaction->open) {
+			// write back all of their remaining dirty blocks
 			while (transaction->num_blocks > 0) {
-				status = write_cached_block(cache, transaction->blocks.Head(), false);
+				status = write_cached_block(cache, transaction->blocks.Head(),
+					false);
 				if (status != B_OK)
 					return status;
 			}
+
+			hash_remove_current(cache->transaction_hash, &iterator);
 			delete_transaction(cache, transaction);
-			hash_rewind(cache->transaction_hash, &iterator);
 		}
 	}
 
@@ -612,7 +656,8 @@ cache_sync_transaction(void *_cache, int32 id)
 
 
 status_t
-cache_end_transaction(void *_cache, int32 id, transaction_notification_hook hook, void *data)
+cache_end_transaction(void *_cache, int32 id,
+	transaction_notification_hook hook, void *data)
 {
 	block_cache *cache = (block_cache *)_cache;
 	BenaphoreLocker locker(&cache->lock);
@@ -700,16 +745,17 @@ cache_abort_transaction(void *_cache, int32 id)
 		block->transaction = NULL;
 	}
 
+	hash_remove(cache->transaction_hash, transaction);
 	delete_transaction(cache, transaction);
 	return B_OK;
 }
 
 
-/**	Acknowledges the current parent transaction, and starts a new transaction
+/*!
- *	from its sub transaction.
+	Acknowledges the current parent transaction, and starts a new transaction
- *	The new transaction also gets a new transaction ID.
+	from its sub transaction.
- */
+	The new transaction also gets a new transaction ID.
-
+*/
 int32
 cache_detach_sub_transaction(void *_cache, int32 id,
 	transaction_notification_hook hook, void *data)
@@ -755,7 +801,8 @@ cache_detach_sub_transaction(void *_cache, int32 id,
 			cache->Free(block->original_data);
 			block->original_data = NULL;
 		}
-		if (block->parent_data != NULL && block->parent_data != block->current_data) {
+		if (block->parent_data != NULL
+			&& block->parent_data != block->current_data) {
 			// we need to move this block over to the new transaction
 			block->original_data = block->parent_data;
 			if (last == NULL)
@@ -810,7 +857,8 @@ cache_abort_sub_transaction(void *_cache, int32 id)
 			if (block->original_data != NULL) {
 				// the parent transaction didn't change the block, but the sub
 				// transaction did - we need to revert from the original data
-				memcpy(block->current_data, block->original_data, cache->block_size);
+				memcpy(block->current_data, block->original_data,
+					cache->block_size);
 			}
 		} else if (block->parent_data != block->current_data) {
 			// the block has been changed and must be restored
@@ -871,8 +919,8 @@ cache_start_sub_transaction(void *_cache, int32 id)
 
 
 status_t
-cache_next_block_in_transaction(void *_cache, int32 id, uint32 *_cookie, off_t *_blockNumber,
+cache_next_block_in_transaction(void *_cache, int32 id, uint32 *_cookie,
-	void **_data, void **_unchangedData)
+	off_t *_blockNumber, void **_data, void **_unchangedData)
 {
 	cached_block *block = (cached_block *)*_cookie;
 	block_cache *cache = (block_cache *)_cache;
@@ -949,7 +997,8 @@ block_cache_delete(void *_cache, bool allowWrites)
 
 	uint32 cookie = 0;
 	cached_block *block;
-	while ((block = (cached_block *)hash_remove_first(cache->hash, &cookie)) != NULL) {
+	while ((block = (cached_block *)hash_remove_first(cache->hash,
+			&cookie)) != NULL) {
 		cache->FreeBlock(block);
 	}
 
@@ -957,7 +1006,8 @@ block_cache_delete(void *_cache, bool allowWrites)
 
 	cookie = 0;
 	cache_transaction *transaction;
-	while ((transaction = (cache_transaction *)hash_remove_first(cache->transaction_hash, &cookie)) != NULL) {
+	while ((transaction = (cache_transaction *)hash_remove_first(
+			cache->transaction_hash, &cookie)) != NULL) {
 		delete transaction;
 	}
 
@@ -968,7 +1018,8 @@ block_cache_delete(void *_cache, bool allowWrites)
 void *
 block_cache_create(int fd, off_t numBlocks, size_t blockSize, bool readOnly)
 {
-	block_cache *cache = new(nothrow) block_cache(fd, numBlocks, blockSize, readOnly);
+	block_cache *cache = new(nothrow) block_cache(fd, numBlocks, blockSize,
+		readOnly);
 	if (cache == NULL)
 		return NULL;
 
@@ -1098,12 +1149,20 @@ block_cache_get(void *_cache, off_t blockNumber)
 }
 
 
+/*!
+	Changes the internal status of a writable block to \a dirty. This can be
+	helpful in case you realize you don't need to change that block anymore
+	for whatever reason.
+
+	Note, you must only use this function on blocks that were acquired
+	writable!
+*/
 status_t
-block_cache_set_dirty(void *_cache, off_t blockNumber, bool isDirty, int32 transaction)
+block_cache_set_dirty(void *_cache, off_t blockNumber, bool dirty,
+	int32 transaction)
 {
-	// not yet implemented
+	// TODO: not yet implemented
-	// Note, you must only use this function on blocks that were acquired writable!
+	if (dirty)
-	if (isDirty)
 		panic("block_cache_set_dirty(): not yet implemented that way!\n");
 
 	return B_OK;

src/add-ons/kernel/file_systems/userlandfs/server/haiku_hash.cpp

@@ -11,7 +11,8 @@
 
 #include <Debug.h>
 #include <Errors.h>
-#include <KernelExport.h>
+
+#include "kernel_emu.h"
 
 #undef TRACE
 #define TRACE_HASH 0
@@ -21,13 +22,16 @@
 #	define TRACE(x) ;
 #endif
 
+using UserlandFS::KernelEmu::dprintf;
+using UserlandFS::KernelEmu::panic;
+
 namespace UserlandFS {
 namespace HaikuKernelEmu {
 
 
-// ToDo: the hashtable is not expanded when necessary (no load factor, no nothing)
+// TODO: the hashtable is not expanded when necessary (no load factor, nothing)
-//		Could try to use pools instead of malloc() for the elements - might be
+//		resizing should be optional, though, in case the hash is used at times
-//		faster than the current approach.
+//		that forbid resizing.
 
 struct hash_table {
 	struct hash_element **table;
@@ -130,7 +134,8 @@ hash_remove(struct hash_table *table, void *_element)
 	uint32 hash = table->hash_func(_element, NULL, table->table_size);
 	void *element, *lastElement = NULL;
 
-	for (element = table->table[hash]; element != NULL; lastElement = element, element = NEXT(table, element)) {
+	for (element = table->table[hash]; element != NULL;
+			lastElement = element, element = NEXT(table, element)) {
 		if (element == _element) {
 			if (lastElement != NULL) {
 				// connect the previous entry with the next one
@@ -147,6 +152,40 @@ hash_remove(struct hash_table *table, void *_element)
 }
 
 
+void
+hash_remove_current(struct hash_table *table, struct hash_iterator *iterator)
+{
+	uint32 index = iterator->bucket;
+	void *element;
+
+	if (iterator->current == NULL)
+		panic("hash_remove_current() called too early.");
+
+	for (element = table->table[index]; index < table->table_size; index++) {
+		void *lastElement = NULL;
+
+		while (element != NULL) {
+			if (element == iterator->current) {
+				iterator->current = lastElement;
+
+				if (lastElement != NULL) {
+					// connect the previous entry with the next one
+					PUT_IN_NEXT(table, lastElement, NEXT(table, element));
+				} else {
+					table->table[index] = (struct hash_element *)NEXT(table,
+						element);
+				}
+
+				table->num_elements--;
+				return;
+			}
+
+			element = NEXT(table, element);
+		}
+	}
+}
+
+
 void *
 hash_remove_first(struct hash_table *table, uint32 *_cookie)
 {

src/add-ons/kernel/file_systems/userlandfs/server/haiku_hash.h

@@ -24,6 +24,7 @@ struct hash_table *hash_init(uint32 table_size, int next_ptr_offset,
 int hash_uninit(struct hash_table *table);
 status_t hash_insert(struct hash_table *table, void *_element);
 status_t hash_remove(struct hash_table *table, void *_element);
+void hash_remove_current(struct hash_table *table, struct hash_iterator *iterator);
 void *hash_remove_first(struct hash_table *table, uint32 *_cookie);
 void *hash_find(struct hash_table *table, void *e);
 void *hash_lookup(struct hash_table *table, const void *key);
@@ -32,17 +33,15 @@ void hash_close(struct hash_table *table, struct hash_iterator *i, bool free_ite
 void *hash_next(struct hash_table *table, struct hash_iterator *i);
 void hash_rewind(struct hash_table *table, struct hash_iterator *i);
 
-/* function ptrs must look like this:
+/* function pointers must look like this:
  *
  * uint32 hash_func(void *e, const void *key, uint32 range);
  *		hash function should calculate hash on either e or key,
- *		depending on which one is not NULL
+ *		depending on which one is not NULL - they also need
+ *		to make sure the returned value is within range.
  * int compare_func(void *e, const void *key);
  *		compare function should compare the element with
  *		the key, returning 0 if equal, other if not
- *		NOTE: compare func can be null, in which case the hash
- *		code will compare the key pointer with the target
- * ToDo: check this!
  */
 
 uint32 hash_hash_string(const char *str);