* cache_detach_sub_transaction() didn't really work: it did not put all needed

blocks into the new transaction, but it would set that transaction on all
  blocks of the old transaction, too. Also, it did not correctly update the
  num_blocks/sub_num_blocks fields of the old transaction. Even worse, it did
  return B_OK instead of the ID of the new transaction...
* get_writable_cached_block() did not correctly maintain the number of blocks
  in the sub transaction.
* write_cached_block() did not free the original_data of a block when it wrote
  it back as part of a previous transaction.
* Changed "cookie" for cache_next_block_in_transaction() to "long", so it will
  be 64 bits when needed.
* Improved the API for detaching sub transactions: you can now get the blocks
  of only the main (parent) transaction as well, added new
  cache_block_in_main_transaction() function.
* BFS now flushes the log when there is no space left for the current
  transaction.
* _WriteTransactionToLog() allocated a "vecs" array, but never freed it.
* _WriteTransactionToLog() now also supports detaching the current sub
  transaction if the whole thing is getting too large (it will now also panic
  if that doesn't work out).
* Removed a useless optimization: making the blocks available in the cache
  isn't really needed, as all blocks in a transaction are locked into the
  cache, anyway.
* Implemented Transaction::WriteBlocks().
* Minor cleanup, removed some dead code, fixed warnings in the fs_shell's
  block_cache when compiled with debug output on.


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

headers/os/drivers/fs_cache.h

@@ -39,9 +39,10 @@ extern status_t cache_add_transaction_listener(void *_cache, int32 id,
 extern status_t cache_remove_transaction_listener(void *_cache, int32 id,
 					transaction_notification_hook hook, void *data);
 extern status_t cache_next_block_in_transaction(void *_cache, int32 id,
-					uint32 *_cookie, off_t *_blockNumber, void **_data,
+					bool mainOnly, long *_cookie, off_t *_blockNumber,
-					void **_unchangedData);
+					void **_data, void **_unchangedData);
 extern int32 cache_blocks_in_transaction(void *_cache, int32 id);
+extern int32 cache_blocks_in_main_transaction(void *_cache, int32 id);
 extern int32 cache_blocks_in_sub_transaction(void *_cache, int32 id);
 
 /* block cache */

headers/private/fs_shell/fssh_api_wrapper.h

@@ -805,6 +805,7 @@
 #define cache_remove_transaction_listener fssh_cache_remove_transaction_listener
 #define cache_next_block_in_transaction	fssh_cache_next_block_in_transaction
 #define cache_blocks_in_transaction		fssh_cache_blocks_in_transaction
+#define cache_blocks_in_main_transaction fssh_cache_blocks_in_main_transaction
 #define cache_blocks_in_sub_transaction	fssh_cache_blocks_in_sub_transaction
 
 /* block cache */

headers/private/fs_shell/fssh_fs_cache.h

@@ -45,11 +45,13 @@ extern fssh_status_t	fssh_cache_remove_transaction_listener(void *_cache,
 							int32_t id, fssh_transaction_notification_hook hook,
 							void *data);
 extern fssh_status_t	fssh_cache_next_block_in_transaction(void *_cache,
-							int32_t id, uint32_t *_cookie,
+							int32_t id, bool mainOnly, long *_cookie,
 							fssh_off_t *_blockNumber, void **_data,
 							void **_unchangedData);
 extern int32_t			fssh_cache_blocks_in_transaction(void *_cache,
 							int32_t id);
+extern int32_t			fssh_cache_blocks_in_main_transaction(void *_cache,
+							int32_t id);
 extern int32_t			fssh_cache_blocks_in_sub_transaction(void *_cache,
 							int32_t id);
 

src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp

@@ -925,7 +925,8 @@ BlockAllocator::StartChecking(check_control *control)
 
 	// initialize bitmap
 	memset(fCheckBitmap, 0, size);
-	for (int32 block = fVolume->Log().Start() + fVolume->Log().Length(); block-- > 0;) {
+	for (int32 block = fVolume->Log().Start() + fVolume->Log().Length();
+			block-- > 0;) {
 		_SetCheckBitmapAt(block);
 	}
 
@@ -979,7 +980,8 @@ BlockAllocator::StopChecking(check_control *control)
 			}
 		}
 
-		control->stats.freed = fVolume->UsedBlocks() - usedBlocks + control->stats.missing;
+		control->stats.freed = fVolume->UsedBlocks() - usedBlocks
+			+ control->stats.missing;
 		if (control->stats.freed < 0)
 			control->stats.freed = 0;
 

src/add-ons/kernel/file_systems/bfs/Journal.cpp

@@ -296,13 +296,11 @@ Journal::Journal(Volume *volume)
 	fLock("bfs journal"),
 	fOwner(NULL),
 	fLogSize(volume->Log().Length()),
-	fMaxTransactionSize(fLogSize / 4 - 5),
+	fMaxTransactionSize(fLogSize / 2 - 5),
 	fUsed(0),
 	fUnwrittenTransactions(0),
 	fHasSubtransaction(false)
 {
-	if (fMaxTransactionSize > fLogSize / 2)
-		fMaxTransactionSize = fLogSize / 2 - 5;
 }
 
 
@@ -505,13 +503,37 @@ Journal::_BlockNotify(int32 transactionID, int32 event, void *arg)
 }
 
 
+/*!	Writes the blocks that are part of current transaction into the log,
+	and ends the current transaction.
+	If the current transaction is too large to fit into the log, it will
+	try to detach an existing sub-transaction.
+*/
 status_t
 Journal::_WriteTransactionToLog()
 {
 	// ToDo: in case of a failure, we need a backup plan like writing all
-	//	changed blocks back to disk immediately
+	//	changed blocks back to disk immediately (hello disk corruption!)
+
+	bool detached = false;
+
+	if (_TransactionSize() > fLogSize) {
+		// The current transaction won't fit into the log anymore, try to
+		// detach the current sub-transaction
+		if (_HasSubTransaction() && cache_blocks_in_main_transaction(
+				fVolume->BlockCache(), fTransactionID) < (int32)fLogSize) {
+			detached = true;
+		} else {
+			// TODO: what are our options here?
+			// a) abort the transaction - bad, because all changes are lost
+			// b) carry out the changes, but don't use the log - even worse,
+			//    as it potentially creates a corrupted disk.
+			panic("transaction too large (%ld blocks, %ld main, log size %ld)!\n",
+				(long)_TransactionSize(), cache_blocks_in_main_transaction(
+				fVolume->BlockCache(), fTransactionID), (long)fLogSize);
+			return B_BUFFER_OVERFLOW;
+		}
+	}
 
-	fUnwrittenTransactions = 0;
 	fHasSubtransaction = false;
 
 	int32 blockShift = fVolume->BlockShift();
@@ -524,10 +546,11 @@ Journal::_WriteTransactionToLog()
 
 	RunArrays runArrays(this);
 
-	uint32 cookie = 0;
 	off_t blockNumber;
+	long cookie = 0;
 	while (cache_next_block_in_transaction(fVolume->BlockCache(),
-			fTransactionID, &cookie, &blockNumber, NULL, NULL) == B_OK) {
+			fTransactionID, detached, &cookie, &blockNumber, NULL,
+			NULL) == B_OK) {
 		status = runArrays.Insert(blockNumber);
 		if (status < B_OK) {
 			FATAL(("filling log entry failed!"));
@@ -537,7 +560,15 @@ Journal::_WriteTransactionToLog()
 
 	if (runArrays.Length() == 0) {
 		// nothing has changed during this transaction
-		cache_end_transaction(fVolume->BlockCache(), fTransactionID, NULL, NULL);
+		if (detached) {
+			fTransactionID = cache_detach_sub_transaction(fVolume->BlockCache(),
+				fTransactionID, NULL, NULL);
+			fUnwrittenTransactions = 1;
+		} else {
+			cache_end_transaction(fVolume->BlockCache(), fTransactionID, NULL,
+				NULL);
+			fUnwrittenTransactions = 0;
+		}
 		return B_OK;
 	}
 
@@ -595,17 +626,12 @@ Journal::_WriteTransactionToLog()
 				}
 
 				// make blocks available in the cache
-				const void *data;
+				const void *data = block_cache_get(fVolume->BlockCache(),
-				if (j == 0) {
+					blockNumber + j);
-					data = block_cache_get_etc(fVolume->BlockCache(),
+				if (data == NULL) {
-						blockNumber, blockNumber, run.Length());
+					free(vecs);
-				} else {
-					data = block_cache_get(fVolume->BlockCache(),
-						blockNumber + j);
-				}
-
-				if (data == NULL)
 					return B_IO_ERROR;
+				}
 
 				add_to_iovec(vecs, index, maxVecs, data, fVolume->BlockSize());
 				count++;
@@ -631,6 +657,8 @@ Journal::_WriteTransactionToLog()
 		}
 	}
 
+	free(vecs);
+
 	LogEntry *logEntry = new LogEntry(this, fVolume->LogEnd(),
 		runArrays.Length());
 	if (logEntry == NULL) {
@@ -659,8 +687,15 @@ Journal::_WriteTransactionToLog()
 	fUsed += logEntry->Length();
 	fEntriesLock.Unlock();
 
-	cache_end_transaction(fVolume->BlockCache(), fTransactionID, _BlockNotify,
+	if (detached) {
-		logEntry);
+		fTransactionID = cache_detach_sub_transaction(fVolume->BlockCache(),
+			fTransactionID, _BlockNotify, logEntry);
+		fUnwrittenTransactions = 1;
+	} else {
+		cache_end_transaction(fVolume->BlockCache(), fTransactionID,
+			_BlockNotify, logEntry);
+		fUnwrittenTransactions = 0;
+	}
 
 	// If the log goes to the next round (the log is written as a
 	// circular buffer), all blocks will be flushed out which is
@@ -784,6 +819,11 @@ Journal::_TransactionDone(bool success)
 		return B_OK;
 	}
 
+	// If necessary, flush the log, so that we have enough space for this
+	// transaction
+	if (_TransactionSize() > FreeLogBlocks())
+		cache_sync_transaction(fVolume->BlockCache(), fTransactionID);
+
 	// Up to a maximum size, we will just batch several
 	// transactions together to improve speed
 	if (_TransactionSize() < fMaxTransactionSize) {
@@ -795,51 +835,6 @@ Journal::_TransactionDone(bool success)
 }
 
 
-status_t
-Journal::LogBlocks(off_t blockNumber, const uint8 *buffer, size_t numBlocks)
-{
-	panic("LogBlocks() called!\n");
-#if 0
-	// ToDo: that's for now - we should change the log file size here
-	if (TransactionSize() + numBlocks + 1 > fLogSize)
-		return B_DEVICE_FULL;
-
-	int32 blockSize = fVolume->BlockSize();
-
-	for (;numBlocks-- > 0; blockNumber++, buffer += blockSize) {
-		if (fArray.Find(blockNumber) >= 0) {
-			// The block is already in the log, so just update its data
-			// Note, this is only necessary if this method is called with a buffer
-			// different from the cached block buffer - which is unlikely but
-			// we'll make sure this way (costs one cache lookup, though).
-			// ToDo:
-/*			status_t status = cached_write(fVolume->Device(), blockNumber, buffer, 1, blockSize);
-			if (status < B_OK)
-				return status;
-*/
-			continue;
-		}
-
-		// Insert the block into the transaction's array, and write the changes
-		// back into the locked cache buffer
-		fArray.Insert(blockNumber);
-
-		// ToDo:
-/*		status_t status = cached_write_locked(fVolume->Device(), blockNumber, buffer, 1, blockSize);
-		if (status < B_OK)
-			return status;
-*/	}
-
-	// ToDo:
-	// If necessary, flush the log, so that we have enough space for this transaction
-/*	if (TransactionSize() > FreeLogBlocks())
-		force_cache_flush(fVolume->Device(), true);
-*/
-#endif
-	return B_OK;
-}
-
-
 //	#pragma mark -
 
 

src/add-ons/kernel/file_systems/bfs/Journal.h

@@ -42,8 +42,6 @@ class Journal {
 
 		status_t ReplayLog();
 
-		status_t LogBlocks(off_t blockNumber, const uint8 *buffer, size_t numBlocks);
-
 		Transaction *CurrentTransaction() const { return fOwner; }
 
 		status_t FlushLogAndBlocks();
@@ -136,18 +134,28 @@ class Transaction {
 		}
 
 		status_t
-		WriteBlocks(off_t blockNumber, const uint8 *buffer, size_t numBlocks = 1)
+		WriteBlocks(off_t blockNumber, const uint8 *buffer,
+			size_t numBlocks = 1)
 		{
 			if (fJournal == NULL)
 				return B_NO_INIT;
 
-			// ToDo: implement this properly!
+			void *cache = GetVolume()->BlockCache();
-			// Currently only used in BlockAllocator::StopChecking(), 
+			size_t blockSize = GetVolume()->BlockSize();
-			// so chkbfs won't work correctly
+
-#if 0
+			for (size_t i = 0; i < numBlocks; i++) {
-			return fJournal->LogBlocks(blockNumber, buffer, numBlocks);
+				void *block = block_cache_get_empty(cache, blockNumber + i,
-#endif
+					ID());
-			return B_ERROR;
+				if (block == NULL)
+					return B_ERROR;
+
+				memcpy(block, buffer, blockSize);
+				buffer += blockSize;
+
+				block_cache_put(cache, blockNumber + i);
+			}
+
+			return B_OK;
 		}
 
 		Volume	*GetVolume()

src/system/kernel/cache/block_cache.cpp

@@ -62,6 +62,7 @@ struct cache_transaction {
 	cache_transaction *next;
 	int32			id;
 	int32			num_blocks;
+	int32			main_num_blocks;
 	int32			sub_num_blocks;
 	cached_block	*first_block;
 	block_list		blocks;
@@ -75,9 +76,10 @@ struct cache_transaction {
 #ifdef TRANSACTION_TRACING
 namespace TransactionTracing {
 
-class Start : public AbstractTraceEntry {
+class StartEnd : public AbstractTraceEntry {
 	public:
-		Start(block_cache *cache, cache_transaction *transaction)
+		StartEnd(const char *label, block_cache *cache,
+				cache_transaction *transaction)
 			:
 			fCache(cache),
 			fTransaction(transaction),
@@ -86,17 +88,19 @@ class Start : public AbstractTraceEntry {
 			fNumBlocks(transaction->num_blocks),
 			fSubNumBlocks(transaction->sub_num_blocks)
 		{
+			strlcpy(fLabel, label, sizeof(label));
 			Initialized();
 		}
 
 		virtual void AddDump(char *buffer, size_t size)
 		{
-			snprintf(buffer, size, "cache %p, start transaction %p (id %ld)%s"
+			snprintf(buffer, size, "cache %p, %s transaction %p (id %ld)%s"
-				", %ld/%ld blocks", fCache, fTransaction, fID,
+				", %ld/%ld blocks", fCache, fLabel, fTransaction, fID,
 				fSub ? " sub" : "", fNumBlocks, fSubNumBlocks);
 		}
 
 	private:
+		char				fLabel[12];
 		block_cache			*fCache;
 		cache_transaction	*fTransaction;
 		int32				fID;
@@ -204,6 +208,7 @@ static object_cache *sBlockCache;
 cache_transaction::cache_transaction()
 {
 	num_blocks = 0;
+	main_num_blocks = 0;
 	sub_num_blocks = 0;
 	first_block = NULL;
 	notification_hook = NULL;
@@ -394,8 +399,8 @@ block_cache::FreeBlock(cached_block *block)
 	Free(block->current_data);
 
 	if (block->original_data != NULL || block->parent_data != NULL) {
-		panic("block_cache::FreeBlock(): %p, %p\n", block->original_data,
+		panic("block_cache::FreeBlock(): %Ld, original %p, parent %p\n",
-			block->parent_data);
+			block->block_number, block->original_data, block->parent_data);
 	}
 
 #ifdef DEBUG_CHANGED
@@ -716,16 +721,18 @@ get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base,
 		return block->current_data;
 	}
 
-	if (block->transaction != NULL && block->transaction->id != transactionID) {
+	cache_transaction *transaction = block->transaction;
+
+	if (transaction != NULL && transaction->id != transactionID) {
 		// ToDo: we have to wait here until the other transaction is done.
 		//	Maybe we should even panic, since we can't prevent any deadlocks.
 		panic("get_writable_cached_block(): asked to get busy writable block (transaction %ld)\n", block->transaction->id);
 		put_cached_block(cache, block);
 		return NULL;
 	}
-	if (block->transaction == NULL && transactionID != -1) {
+	if (transaction == NULL && transactionID != -1) {
 		// get new transaction
-		cache_transaction *transaction = lookup_transaction(cache, transactionID);
+		transaction = lookup_transaction(cache, transactionID);
 		if (transaction == NULL) {
 			panic("get_writable_cached_block(): invalid transaction %ld!\n",
 				transactionID);
@@ -746,6 +753,9 @@ get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base,
 		transaction->num_blocks++;
 	}
 
+	bool wasUnchanged = block->original_data == NULL
+		|| block->previous_transaction != NULL;
+
 	if (!(allocated && cleared) && block->original_data == NULL) {
 		// we already have data, so we need to preserve it
 		block->original_data = cache->Allocate();
@@ -768,8 +778,10 @@ get_writable_cached_block(block_cache *cache, off_t blockNumber, off_t base,
 		}
 
 		memcpy(block->parent_data, block->current_data, cache->block_size);
-		block->transaction->sub_num_blocks++;
+		transaction->sub_num_blocks++;
-	}
+	} else if (transaction != NULL && transaction->has_sub_transaction
+		&& block->parent_data == NULL && wasUnchanged)
+		transaction->sub_num_blocks++;
 
 	if (cleared)
 		memset(block->current_data, 0, cache->block_size);
@@ -809,6 +821,13 @@ write_cached_block(block_cache *cache, cached_block *block,
 		previous->blocks.Remove(block);
 		block->previous_transaction = NULL;
 
+		if (block->original_data != NULL && block->transaction == NULL) {
+			// This block is not part of a transaction, so it does not need
+			// its original pointer anymore.
+			cache->Free(block->original_data);
+			block->original_data = NULL;
+		}
+
 		// Has the previous transation been finished with that write?
 		if (--previous->num_blocks == 0) {
 			TRACE(("cache transaction %ld finished!\n", previous->id));
@@ -992,7 +1011,7 @@ cache_start_transaction(void *_cache)
 	cache->last_transaction = transaction;
 
 	TRACE(("cache_start_transaction(): id %ld started\n", transaction->id));
-	T(Start(cache, transaction));
+	T(StartEnd("start", cache, transaction));
 
 	hash_insert(cache->transaction_hash, transaction);
 
@@ -1007,6 +1026,8 @@ cache_sync_transaction(void *_cache, int32 id)
 	BenaphoreLocker locker(&cache->lock);
 	status_t status = B_ENTRY_NOT_FOUND;
 
+	TRACE(("cache_sync_transaction(id %ld)\n", id));
+
 	hash_iterator iterator;
 	hash_open(cache->transaction_hash, &iterator);
 
@@ -1049,6 +1070,8 @@ cache_end_transaction(void *_cache, int32 id,
 		return B_BAD_VALUE;
 	}
 
+	T(StartEnd("end", cache, transaction));
+
 	transaction->notification_hook = hook;
 	transaction->notification_data = data;
 
@@ -1135,8 +1158,7 @@ cache_abort_transaction(void *_cache, int32 id)
 }
 
 
-/*!
+/*!	Acknowledges the current parent transaction, and starts a new transaction
-	Acknowledges the current parent transaction, and starts a new transaction
 	from its sub transaction.
 	The new transaction also gets a new transaction ID.
 */
@@ -1188,8 +1210,8 @@ cache_detach_sub_transaction(void *_cache, int32 id,
 			cache->Free(block->original_data);
 			block->original_data = NULL;
 		}
-		if (block->parent_data != NULL
+		if (block->parent_data == NULL
-			&& block->parent_data != block->current_data) {
+			|| 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)
@@ -1197,24 +1219,32 @@ cache_detach_sub_transaction(void *_cache, int32 id,
 			else
 				last->transaction_next = block;
 
+			block->transaction = newTransaction;
 			last = block;
-		}
+		} else
-		block->parent_data = NULL;
+			block->transaction = NULL;
 
-		// move the block to the previous transaction list
+		if (block->parent_data != NULL) {
-		transaction->blocks.Add(block);
+			// move the block to the previous transaction list
+			transaction->blocks.Add(block);
+			block->parent_data = NULL;
+		}
 
 		block->previous_transaction = transaction;
 		block->transaction_next = NULL;
-		block->transaction = newTransaction;
 	}
 
+	newTransaction->num_blocks = transaction->sub_num_blocks;
+
 	transaction->open = false;
+	transaction->has_sub_transaction = false;
+	transaction->num_blocks = transaction->main_num_blocks;
+	transaction->sub_num_blocks = 0;
 
 	hash_insert(cache->transaction_hash, newTransaction);
 	cache->last_transaction = newTransaction;
 
-	return B_OK;
+	return newTransaction->id;
 }
 
 
@@ -1263,6 +1293,8 @@ cache_abort_sub_transaction(void *_cache, int32 id)
 
 	// all subsequent changes will go into the main transaction
 	transaction->has_sub_transaction = false;
+	transaction->sub_num_blocks = 0;
+
 	return B_OK;
 }
 
@@ -1304,8 +1336,9 @@ cache_start_sub_transaction(void *_cache, int32 id)
 
 	// all subsequent changes will go into the sub transaction
 	transaction->has_sub_transaction = true;
+	transaction->main_num_blocks = transaction->num_blocks;
 	transaction->sub_num_blocks = 0;
-	T(Start(cache, transaction));
+	T(StartEnd("start-sub", cache, transaction));
 
 	return B_OK;
 }
@@ -1368,8 +1401,8 @@ cache_remove_transaction_listener(void *_cache, int32 id,
 
 
 extern "C" status_t
-cache_next_block_in_transaction(void *_cache, int32 id, uint32 *_cookie,
+cache_next_block_in_transaction(void *_cache, int32 id, bool mainOnly,
-	off_t *_blockNumber, void **_data, void **_unchangedData)
+	long *_cookie, off_t *_blockNumber, void **_data, void **_unchangedData)
 {
 	cached_block *block = (cached_block *)*_cookie;
 	block_cache *cache = (block_cache *)_cache;
@@ -1377,7 +1410,7 @@ cache_next_block_in_transaction(void *_cache, int32 id, uint32 *_cookie,
 	BenaphoreLocker locker(&cache->lock);
 
 	cache_transaction *transaction = lookup_transaction(cache, id);
-	if (transaction == NULL)
+	if (transaction == NULL || !transaction->open)
 		return B_BAD_VALUE;
 
 	if (block == NULL)
@@ -1385,17 +1418,23 @@ cache_next_block_in_transaction(void *_cache, int32 id, uint32 *_cookie,
 	else
 		block = block->transaction_next;
 
+	if (mainOnly && transaction->has_sub_transaction) {
+		// find next block that the parent changed
+		while (block != NULL && block->parent_data == NULL)
+			block = block->transaction_next;
+	}
+
 	if (block == NULL)
 		return B_ENTRY_NOT_FOUND;
 
 	if (_blockNumber)
 		*_blockNumber = block->block_number;
 	if (_data)
-		*_data = block->current_data;
+		*_data = mainOnly ? block->parent_data : block->current_data;
 	if (_unchangedData)
 		*_unchangedData = block->original_data;
 
-	*_cookie = (uint32)block;
+	*_cookie = (addr_t)block;
 	return B_OK;	
 }
 
@@ -1414,6 +1453,20 @@ cache_blocks_in_transaction(void *_cache, int32 id)
 }
 
 
+extern "C" int32
+cache_blocks_in_main_transaction(void *_cache, int32 id)
+{
+	block_cache *cache = (block_cache *)_cache;
+	BenaphoreLocker locker(&cache->lock);
+
+	cache_transaction *transaction = lookup_transaction(cache, id);
+	if (transaction == NULL)
+		return B_BAD_VALUE;
+
+	return transaction->main_num_blocks;
+}
+
+
 extern "C" int32
 cache_blocks_in_sub_transaction(void *_cache, int32 id)
 {
@@ -1598,7 +1651,7 @@ block_cache_get_empty(void *_cache, off_t blockNumber, int32 transaction)
 		panic("tried to get empty writable block on a read-only cache!");
 
 	return get_writable_cached_block((block_cache *)_cache, blockNumber,
-				blockNumber, 1, transaction, true);
+		blockNumber, 1, transaction, true);
 }
 
 

src/tools/fs_shell/block_cache.cpp

@@ -58,6 +58,7 @@ struct cache_transaction {
 	cache_transaction *next;
 	int32_t			id;
 	int32_t			num_blocks;
+	int32_t			main_num_blocks;
 	int32_t			sub_num_blocks;
 	cached_block	*first_block;
 	block_list		blocks;
@@ -78,6 +79,7 @@ static fssh_status_t write_cached_block(block_cache *cache, cached_block *block,
 cache_transaction::cache_transaction()
 {
 	num_blocks = 0;
+	main_num_blocks = 0;
 	sub_num_blocks = 0;
 	first_block = NULL;
 	notification_hook = NULL;
@@ -246,11 +248,10 @@ void
 block_cache::FreeBlock(cached_block *block)
 {
 	Free(block->current_data);
-	block->current_data = NULL;
 
 	if (block->original_data != NULL || block->parent_data != NULL) {
-		fssh_panic("block_cache::FreeBlock(): %p, %p\n", block->original_data,
+		fssh_panic("block_cache::FreeBlock(): %Ld, original %p, parent %p\n",
-			block->parent_data);
+			block->block_number, block->original_data, block->parent_data);
 	}
 
 #ifdef DEBUG_CHANGED
@@ -415,23 +416,6 @@ get_cached_block(block_cache *cache, fssh_off_t blockNumber, bool *_allocated,
 
 		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
-			block->current_data = cache->allocator->Acquire(block->current_data);
-		}
-
-		if (block->current_data == NULL) {
-			// there is no block yet, but we need one
-			block->current_data = cache->allocator->Get();
-			if (block->current_data == NULL)
-				return NULL;
-
-			*_allocated = true;
-		}
-*/
 	}
 
 	if (*_allocated && readBlock) {
@@ -471,7 +455,7 @@ static void *
 get_writable_cached_block(block_cache *cache, fssh_off_t blockNumber, fssh_off_t base,
 	fssh_off_t length, int32_t transactionID, bool cleared)
 {
-	TRACE(("get_writable_cached_block(blockNumber = %Ld, transaction = %ld)\n",
+	TRACE(("get_writable_cached_block(blockNumber = %Ld, transaction = %d)\n",
 		blockNumber, transactionID));
 
 	if (blockNumber < 0 || blockNumber >= cache->max_blocks) {
@@ -496,16 +480,18 @@ get_writable_cached_block(block_cache *cache, fssh_off_t blockNumber, fssh_off_t
 		return block->current_data;
 	}
 
-	if (block->transaction != NULL && block->transaction->id != transactionID) {
+	cache_transaction *transaction = block->transaction;
+
+	if (transaction != NULL && transaction->id != transactionID) {
 		// ToDo: we have to wait here until the other transaction is done.
 		//	Maybe we should even panic, since we can't prevent any deadlocks.
-		fssh_panic("get_writable_cached_block(): asked to get busy writable block (transaction %d)\n", (int)block->transaction->id);
+		fssh_panic("get_writable_cached_block(): asked to get busy writable block (transaction %d)\n", (int)transaction->id);
 		put_cached_block(cache, block);
 		return NULL;
 	}
-	if (block->transaction == NULL && transactionID != -1) {
+	if (transaction == NULL && transactionID != -1) {
 		// get new transaction
-		cache_transaction *transaction = lookup_transaction(cache, transactionID);
+		transaction = lookup_transaction(cache, transactionID);
 		if (transaction == NULL) {
 			fssh_panic("get_writable_cached_block(): invalid transaction %d!\n",
 				(int)transactionID);
@@ -526,6 +512,9 @@ get_writable_cached_block(block_cache *cache, fssh_off_t blockNumber, fssh_off_t
 		transaction->num_blocks++;
 	}
 
+	bool wasUnchanged = block->original_data == NULL
+		|| block->previous_transaction != NULL;
+
 	if (!(allocated && cleared) && block->original_data == NULL) {
 		// we already have data, so we need to preserve it
 		block->original_data = cache->Allocate();
@@ -548,8 +537,10 @@ get_writable_cached_block(block_cache *cache, fssh_off_t blockNumber, fssh_off_t
 		}
 
 		fssh_memcpy(block->parent_data, block->current_data, cache->block_size);
-		block->transaction->sub_num_blocks++;
+		transaction->sub_num_blocks++;
-	}
+	} else if (transaction != NULL && transaction->has_sub_transaction
+		&& block->parent_data == NULL && wasUnchanged)
+		transaction->sub_num_blocks++;
 
 	if (cleared)
 		fssh_memset(block->current_data, 0, cache->block_size);
@@ -589,6 +580,13 @@ write_cached_block(block_cache *cache, cached_block *block,
 		previous->blocks.Remove(block);
 		block->previous_transaction = NULL;
 
+		if (block->original_data != NULL && block->transaction == NULL) {
+			// This block is not part of a transaction, so it does not need
+			// its original pointer anymore.
+			cache->Free(block->original_data);
+			block->original_data = NULL;
+		}
+
 		// Has the previous transation been finished with that write?
 		if (--previous->num_blocks == 0) {
 			TRACE(("cache transaction %ld finished!\n", previous->id));
@@ -643,7 +641,7 @@ fssh_cache_start_transaction(void *_cache)
 	transaction->id = fssh_atomic_add(&cache->next_transaction_id, 1);
 	cache->last_transaction = transaction;
 
-	TRACE(("cache_start_transaction(): id %ld started\n", transaction->id));
+	TRACE(("cache_start_transaction(): id %d started\n", transaction->id));
 
 	hash_insert(cache->transaction_hash, transaction);
 
@@ -658,6 +656,8 @@ fssh_cache_sync_transaction(void *_cache, int32_t id)
 	BenaphoreLocker locker(&cache->lock);
 	fssh_status_t status = FSSH_B_ENTRY_NOT_FOUND;
 
+	TRACE(("cache_sync_transaction(id %d)\n", id));
+
 	hash_iterator iterator;
 	hash_open(cache->transaction_hash, &iterator);
 
@@ -692,7 +692,7 @@ fssh_cache_end_transaction(void *_cache, int32_t id,
 	block_cache *cache = (block_cache *)_cache;
 	BenaphoreLocker locker(&cache->lock);
 
-	TRACE(("cache_end_transaction(id = %ld)\n", id));
+	TRACE(("cache_end_transaction(id = %d)\n", id));
 
 	cache_transaction *transaction = lookup_transaction(cache, id);
 	if (transaction == NULL) {
@@ -785,8 +785,7 @@ fssh_cache_abort_transaction(void *_cache, int32_t id)
 }
 
 
-/*!
+/*!	Acknowledges the current parent transaction, and starts a new transaction
-	Acknowledges the current parent transaction, and starts a new transaction
 	from its sub transaction.
 	The new transaction also gets a new transaction ID.
 */
@@ -797,7 +796,7 @@ fssh_cache_detach_sub_transaction(void *_cache, int32_t id,
 	block_cache *cache = (block_cache *)_cache;
 	BenaphoreLocker locker(&cache->lock);
 
-	TRACE(("cache_detach_sub_transaction(id = %ld)\n", id));
+	TRACE(("cache_detach_sub_transaction(id = %d)\n", id));
 
 	cache_transaction *transaction = lookup_transaction(cache, id);
 	if (transaction == NULL) {
@@ -837,8 +836,8 @@ fssh_cache_detach_sub_transaction(void *_cache, int32_t id,
 			cache->Free(block->original_data);
 			block->original_data = NULL;
 		}
-		if (block->parent_data != NULL
+		if (block->parent_data == NULL
-			&& block->parent_data != block->current_data) {
+			|| 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)
@@ -846,24 +845,32 @@ fssh_cache_detach_sub_transaction(void *_cache, int32_t id,
 			else
 				last->transaction_next = block;
 
+			block->transaction = newTransaction;
 			last = block;
-		}
+		} else
-		block->parent_data = NULL;
+			block->transaction = NULL;
 
-		// move the block to the previous transaction list
+		if (block->parent_data != NULL) {
-		transaction->blocks.Add(block);
+			// move the block to the previous transaction list
+			transaction->blocks.Add(block);
+			block->parent_data = NULL;
+		}
 
 		block->previous_transaction = transaction;
 		block->transaction_next = NULL;
-		block->transaction = newTransaction;
 	}
 
+	newTransaction->num_blocks = transaction->sub_num_blocks;
+
 	transaction->open = false;
+	transaction->has_sub_transaction = false;
+	transaction->num_blocks = transaction->main_num_blocks;
+	transaction->sub_num_blocks = 0;
 
 	hash_insert(cache->transaction_hash, newTransaction);
 	cache->last_transaction = newTransaction;
 
-	return FSSH_B_OK;
+	return newTransaction->id;
 }
 
 
@@ -902,7 +909,8 @@ fssh_cache_abort_sub_transaction(void *_cache, int32_t id)
 			// the block has been changed and must be restored
 			TRACE(("cache_abort_sub_transaction(id = %ld): restored contents of block %Ld\n",
 				transaction->id, block->block_number));
-			fssh_memcpy(block->current_data, block->parent_data, cache->block_size);
+			fssh_memcpy(block->current_data, block->parent_data,
+				cache->block_size);
 			cache->Free(block->parent_data);
 		}
 
@@ -911,6 +919,8 @@ fssh_cache_abort_sub_transaction(void *_cache, int32_t id)
 
 	// all subsequent changes will go into the main transaction
 	transaction->has_sub_transaction = false;
+	transaction->sub_num_blocks = 0;
+
 	return FSSH_B_OK;
 }
 
@@ -921,7 +931,7 @@ fssh_cache_start_sub_transaction(void *_cache, int32_t id)
 	block_cache *cache = (block_cache *)_cache;
 	BenaphoreLocker locker(&cache->lock);
 
-	TRACE(("cache_start_sub_transaction(id = %ld)\n", id));
+	TRACE(("cache_start_sub_transaction(id = %d)\n", id));
 
 	cache_transaction *transaction = lookup_transaction(cache, id);
 	if (transaction == NULL) {
@@ -952,6 +962,7 @@ fssh_cache_start_sub_transaction(void *_cache, int32_t id)
 
 	// all subsequent changes will go into the sub transaction
 	transaction->has_sub_transaction = true;
+	transaction->main_num_blocks = transaction->num_blocks;
 	transaction->sub_num_blocks = 0;
 
 	return FSSH_B_OK;
@@ -1015,8 +1026,9 @@ fssh_cache_remove_transaction_listener(void *_cache, int32_t id,
 
 
 fssh_status_t
-fssh_cache_next_block_in_transaction(void *_cache, int32_t id, uint32_t *_cookie,
+fssh_cache_next_block_in_transaction(void *_cache, int32_t id, bool mainOnly,
-	fssh_off_t *_blockNumber, void **_data, void **_unchangedData)
+	long *_cookie, fssh_off_t *_blockNumber, void **_data,
+	void **_unchangedData)
 {
 	cached_block *block = (cached_block *)*_cookie;
 	block_cache *cache = (block_cache *)_cache;
@@ -1024,7 +1036,7 @@ fssh_cache_next_block_in_transaction(void *_cache, int32_t id, uint32_t *_cookie
 	BenaphoreLocker locker(&cache->lock);
 
 	cache_transaction *transaction = lookup_transaction(cache, id);
-	if (transaction == NULL)
+	if (transaction == NULL || !transaction->open)
 		return FSSH_B_BAD_VALUE;
 
 	if (block == NULL)
@@ -1032,13 +1044,19 @@ fssh_cache_next_block_in_transaction(void *_cache, int32_t id, uint32_t *_cookie
 	else
 		block = block->transaction_next;
 
+	if (mainOnly && transaction->has_sub_transaction) {
+		// find next block that the parent changed
+		while (block != NULL && block->parent_data == NULL)
+			block = block->transaction_next;
+	}
+
 	if (block == NULL)
 		return FSSH_B_ENTRY_NOT_FOUND;
 
 	if (_blockNumber)
 		*_blockNumber = block->block_number;
 	if (_data)
-		*_data = block->current_data;
+		*_data = mainOnly ? block->parent_data : block->current_data;
 	if (_unchangedData)
 		*_unchangedData = block->original_data;
 
@@ -1061,6 +1079,20 @@ fssh_cache_blocks_in_transaction(void *_cache, int32_t id)
 }
 
 
+int32_t
+fssh_cache_blocks_in_main_transaction(void *_cache, int32_t id)
+{
+	block_cache *cache = (block_cache *)_cache;
+	BenaphoreLocker locker(&cache->lock);
+
+	cache_transaction *transaction = lookup_transaction(cache, id);
+	if (transaction == NULL)
+		return FSSH_B_BAD_VALUE;
+
+	return transaction->main_num_blocks;
+}
+
+
 int32_t
 fssh_cache_blocks_in_sub_transaction(void *_cache, int32_t id)
 {
@@ -1228,7 +1260,8 @@ fssh_block_cache_get_writable_etc(void *_cache, fssh_off_t blockNumber, fssh_off
 
 
 void *
-fssh_block_cache_get_writable(void *_cache, fssh_off_t blockNumber, int32_t transaction)
+fssh_block_cache_get_writable(void *_cache, fssh_off_t blockNumber,
+	int32_t transaction)
 {
 	return fssh_block_cache_get_writable_etc(_cache, blockNumber,
 		blockNumber, 1, transaction);
@@ -1236,7 +1269,8 @@ fssh_block_cache_get_writable(void *_cache, fssh_off_t blockNumber, int32_t tran
 
 
 void *
-fssh_block_cache_get_empty(void *_cache, fssh_off_t blockNumber, int32_t transaction)
+fssh_block_cache_get_empty(void *_cache, fssh_off_t blockNumber,
+	int32_t transaction)
 {
 	block_cache *cache = (block_cache *)_cache;
 	BenaphoreLocker locker(&cache->lock);
@@ -1247,12 +1281,13 @@ fssh_block_cache_get_empty(void *_cache, fssh_off_t blockNumber, int32_t transac
 		fssh_panic("tried to get empty writable block on a read-only cache!");
 
 	return get_writable_cached_block((block_cache *)_cache, blockNumber,
-				blockNumber, 1, transaction, true);
+		blockNumber, 1, transaction, true);
 }
 
 
 const void *
-fssh_block_cache_get_etc(void *_cache, fssh_off_t blockNumber, fssh_off_t base, fssh_off_t length)
+fssh_block_cache_get_etc(void *_cache, fssh_off_t blockNumber, fssh_off_t base,
+	fssh_off_t length)
 {
 	block_cache *cache = (block_cache *)_cache;
 	BenaphoreLocker locker(&cache->lock);

src/tools/fs_shell/block_cache_priv.h

@@ -65,7 +65,6 @@ struct block_cache {
 	cache_transaction *last_transaction;
 	hash_table	*transaction_hash;
 
-	block_list	unmapped_blocks;
 	block_list	unused_blocks;
 
 	bool		read_only;