* _TransactionSize() did not take the block array into account.

* Therefore, the log was not flushed often enough:
  _WriteTransactionToLog() did not check again, and would eventually
  overwrite incomplete transactions (leading to a corrupted log on
  reboot)!
* _TransactionDone() now only flushes the log if the new transaction
  will be joined; _WriteTransactionToLog() now checks itself if there
  is enough space left to actually write the log entry.
* Added a TODO about the logging code not being endian-aware.
* Renamed _BlockNotify() to _TransactionNotify() as only complete
  transactions are notified, it's not working on block level anymore.


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

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

@@ -526,39 +526,38 @@ Journal::ReplayLog()
 
 
 /*!	This is a callback function that is called by the cache, whenever
-	a block is flushed to disk that was updated as part of a transaction.
+	all blocks of a transaction have been flushed to disk.
-	This is necessary to keep track of completed transactions, to be
+	This lets us keep track of completed transactions, and update
-	able to update the log start pointer.
+	the log start pointer as needed. Note, the transactions may not be
+	completed in the order they were written.
 */
 void
-Journal::_BlockNotify(int32 transactionID, int32 event, void *arg)
+Journal::_TransactionNotify(int32 transactionID, int32 event, void *_logEntry)
 {
-	LogEntry *logEntry = (LogEntry *)arg;
+	LogEntry *logEntry = (LogEntry *)_logEntry;
 
 	if (event != TRANSACTION_WRITTEN)
 		return;
 
-	PRINT(("Log entry %p has been finished, transaction ID = %ld\n", logEntry, transactionID));
+	PRINT(("Log entry %p has been finished, transaction ID = %ld\n", logEntry,
+		transactionID));
 
 	Journal *journal = logEntry->GetJournal();
 	disk_super_block &superBlock = journal->fVolume->SuperBlock();
 	bool update = false;
 
 	// Set log_start pointer if possible...
+	// TODO: this is not endian safe!
 
 	journal->fEntriesLock.Lock();
 
 	if (logEntry == journal->fEntries.First()) {
 		LogEntry *next = journal->fEntries.GetNext(logEntry);
-		if (next != NULL) {
+		if (next != NULL)
-			int32 length = next->Start() - logEntry->Start();
+			superBlock.log_start = next->Start() % journal->fLogSize;
-				// log entries inbetween could have been already released, so
+		else
-				// we can't just use LogEntry::Length() here
-			superBlock.log_start = superBlock.log_start + length;
-		} else
 			superBlock.log_start = journal->fVolume->LogEnd();
 
-		superBlock.log_start %= journal->fLogSize;
 		update = true;
 	}
 
@@ -654,6 +653,11 @@ Journal::_WriteTransactionToLog()
 		return B_OK;
 	}
 
+	// If necessary, flush the log, so that we have enough space for this
+	// transaction
+	if (runArrays.LogEntryLength() > FreeLogBlocks())
+		cache_sync_transaction(fVolume->BlockCache(), fTransactionID);
+
 	// Write log entries to disk
 
 	int32 maxVecs = runArrays.MaxArrayLength() + 1;
@@ -759,11 +763,11 @@ Journal::_WriteTransactionToLog()
 
 	if (detached) {
 		fTransactionID = cache_detach_sub_transaction(fVolume->BlockCache(),
-			fTransactionID, _BlockNotify, logEntry);
+			fTransactionID, _TransactionNotify, logEntry);
 		fUnwrittenTransactions = 1;
 	} else {
 		cache_end_transaction(fVolume->BlockCache(), fTransactionID,
-			_BlockNotify, logEntry);
+			_TransactionNotify, logEntry);
 		fUnwrittenTransactions = 0;
 	}
 
@@ -866,7 +870,10 @@ Journal::_TransactionSize() const
 	if (count < 0)
 		return 0;
 
-	return count;
+	// take the number of array blocks in this transaction into account
+	uint32 maxRuns = run_array::MaxRuns(fVolume->BlockSize());
+	uint32 arrayBlocks = (count + maxRuns - 1) / maxRuns;
+	return count + arrayBlocks;
 }
 
 
@@ -882,14 +889,15 @@ 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) {
+	uint32 size = _TransactionSize();
+	if (size < fMaxTransactionSize) {
+		// Flush the log from time to time, so that we have enough space
+		// for this transaction
+		if (size > FreeLogBlocks())
+			cache_sync_transaction(fVolume->BlockCache(), fTransactionID);
+
 		fUnwrittenTransactions++;
 		return B_OK;
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2001-2007, Axel Dörfler, axeld@pinc-software.de.
+ * Copyright 2001-2008, Axel Dörfler, axeld@pinc-software.de.
  * This file may be used under the terms of the MIT License.
  */
 #ifndef JOURNAL_H
@@ -57,7 +57,8 @@ class Journal {
 		status_t _CheckRunArray(const run_array *array);
 		status_t _ReplayRunArray(int32 *start);
 		status_t _TransactionDone(bool success);
-		static void _BlockNotify(int32 transactionID, int32 event, void *arg);
+		static void _TransactionNotify(int32 transactionID, int32 event,
+			void *_logEntry);
 
 		Volume			*fVolume;
 		RecursiveLock	fLock;
@@ -75,8 +76,8 @@ class Journal {
 inline uint32
 Journal::FreeLogBlocks() const
 {
-	return fVolume->LogStart() <= fVolume->LogEnd() ?
+	return fVolume->LogStart() <= fVolume->LogEnd()
-		fLogSize - fVolume->LogEnd() + fVolume->LogStart()
+		? fLogSize - fVolume->LogEnd() + fVolume->LogStart()
 		: fVolume->LogStart() - fVolume->LogEnd();
 }