Transactions can now be nested (bfs_release_vnode() actually did that before,

but that was not working correctly):
only the owning transaction (the one that came first) can now end a transaction.
To do: if the owning transaction fails, it should actually not abort the
transaction in case there were sub transactions, but does so right now
(should do no harm, eventually file data is not freed as it should).


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

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

@@ -1,6 +1,6 @@
 /* Journal - transaction and logging
  *
- * Copyright 2001-2004, Axel Dörfler, [email protected].
+ * Copyright 2001-2005, Axel Dörfler, [email protected].
  * This file may be used under the terms of the MIT License.
  */
 
@@ -338,7 +338,7 @@ Journal::FlushLogAndBlocks()
 
 	// write the current log entry to disk
 
-	if (TransactionSize() != 0) {
+	if (fTransactionID != -1 && TransactionSize() != 0) {
 		status = WriteLogEntry();
 		if (status < B_OK)
 			FATAL(("writing current log entry failed: %s\n", strerror(status)));
@@ -353,14 +353,9 @@ Journal::FlushLogAndBlocks()
 status_t
 Journal::Lock(Transaction *owner)
 {
-	if (owner == fOwner) {
-		dprintf("bfs(%ld): journal is already locked by caller\n", find_thread(NULL));
-		return B_OK;
-	}
-
 	status_t status = fLock.Lock();
-	if (status == B_OK)
+	if (status != B_OK)
-		fOwner = owner;
+		return status;
 
 /*	ToDo:
 	// if the last transaction is older than 2 secs, start a new one
@@ -368,6 +363,13 @@ Journal::Lock(Transaction *owner)
 		WriteLogEntry();
 */
 
+	if (fLock.OwnerCount() > 1) {
+		// we'll just use the current transaction again
+		return B_OK;
+	}
+
+	fOwner = owner;
+
 	fTransactionID = cache_start_transaction(fVolume->BlockCache());
 	if (fTransactionID < B_OK) {
 		fLock.Unlock();
@@ -381,41 +383,25 @@ Journal::Lock(Transaction *owner)
 void
 Journal::Unlock(Transaction *owner, bool success)
 {
-	if (owner != fOwner) {
+	if (fLock.OwnerCount() == 1) {
-		dprintf("bfs(%ld): journal is not owned by caller\n", find_thread(NULL));
+		// we only end the transaction if we would really unlock it
-		return;
+		// ToDo: what about failing transactions that do not unlock?
+		TransactionDone(success);
+
+		fTransactionID = -1;
+		fTimestamp = system_time();
+		fOwner = NULL;
 	}
 
-	TransactionDone(success);
-
-	fTransactionID = -1;
-	fTimestamp = system_time();
-	fOwner = NULL;
 	fLock.Unlock();
 }
 
 
-/** If there is a current transaction that the current thread has
- *	started, this function will give you access to it.
- */
-
-Transaction *
-Journal::CurrentTransaction()
-{
-	if (fLock.LockWithTimeout(0) != B_OK)
-		return NULL;
-
-	Transaction *owner = fOwner;
-	fLock.Unlock();
-
-	return owner;
-}
-
-
 status_t
 Journal::TransactionDone(bool success)
 {
 	if (!success) {
+		fArray.MakeEmpty();
 		cache_abort_transaction(fVolume->BlockCache(), fTransactionID);
 		return B_OK;
 	}

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

@@ -1,6 +1,6 @@
 /* Journal - transaction and logging
  *
- * Copyright 2001-2004, Axel Dörfler, [email protected].
+ * Copyright 2001-2005, Axel Dörfler, [email protected].
  * This file may be used under the terms of the MIT License.
  */
 #ifndef JOURNAL_H
@@ -56,7 +56,7 @@ class Journal {
 		status_t WriteLogEntry();
 		status_t LogBlocks(off_t blockNumber, const uint8 *buffer, size_t numBlocks);
 
-		Transaction *CurrentTransaction();
+		Transaction *CurrentTransaction() const { return fOwner; }
 		uint32 TransactionSize() const { return fArray.CountItems() + fArray.BlocksUsed(); }
 
 		status_t FlushLogAndBlocks();
@@ -137,12 +137,22 @@ class Transaction {
 			fJournal = NULL;
 		}
 
+		bool HasParent()
+		{
+			if (fJournal != NULL)
+				return fJournal->CurrentTransaction() == this;
+
+			return false;
+		}
+
 		status_t WriteBlocks(off_t blockNumber, const uint8 *buffer, size_t numBlocks = 1)
 		{
 			if (fJournal == NULL)
 				return B_NO_INIT;
 
 			// ToDo: implement this properly!
+			// Currently only used in BlockAllocator::StopChecking(), 
+			// so chkbfs won't work correctly
 #if 0
 			return fJournal->LogBlocks(blockNumber, buffer, numBlocks);
 #endif

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

@@ -180,6 +180,9 @@ class RecursiveLock {
 			return B_OK;
 		}
 
+		thread_id Owner() const { return fOwner; }
+		int32 OwnerCount() const { return fOwnerCount; }
+
 	private:
 		sem_id	fSemaphore;
 #ifdef USE_BENAPHORE

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

@@ -298,9 +298,13 @@ bfs_release_vnode(void *_ns, void *_node, bool reenter)
 	// we need to take care about their preallocated blocks here
 	if (inode->NeedsTrimming()) {
 		Transaction transaction(volume, inode->BlockNumber());
-		
+
 		if (inode->TrimPreallocation(transaction) == B_OK)
 			transaction.Done();
+		else if (transaction.HasParent()) {
+			// ToDo: for now, we don't let sub-transactions fail
+			transaction.Done();
+		}
 	}
 
 	delete inode;
@@ -332,23 +336,16 @@ bfs_remove_vnode(void *_ns, void *_node, bool reenter)
 	// If the inode isn't in use anymore, we were called before
 	// bfs_unlink() returns - in this case, we can just use the
 	// transaction which has already deleted the inode.
-	Transaction localTransaction, *transaction = NULL;
+	Transaction transaction(volume, volume->ToBlock(inode->Parent()));
 
-	Journal *journal = volume->GetJournal(volume->ToBlock(inode->Parent()));
+	status_t status = inode->Free(transaction);
-	if (journal != NULL)
-		transaction = journal->CurrentTransaction();
-
-	if (transaction == NULL) {
-		transaction = &localTransaction;
-		localTransaction.Start(volume, inode->BlockNumber());
-	}
-
-	status_t status = inode->Free(*transaction);
 	if (status == B_OK) {
-		if (transaction == &localTransaction)
+		transaction.Done();
-			localTransaction.Done();
 
 		delete inode;
+	} else if (transaction.HasParent()) {
+		// ToDo: for now, we don't let sub-transactions fail
+		transaction.Done();
 	}
 
 	return status;
@@ -489,7 +486,7 @@ bfs_lookup(void *_ns, void *_directory, const char *file, vnode_id *_vnodeID, in
 		RETURN_ERROR(B_BAD_VALUE);
 
 	if ((status = tree->Find((uint8 *)file, (uint16)strlen(file), _vnodeID)) < B_OK) {
-		PRINT(("bfs_walk() could not find %Ld:\"%s\": %s\n", directory->BlockNumber(), file, strerror(status)));
+		//PRINT(("bfs_walk() could not find %Ld:\"%s\": %s\n", directory->BlockNumber(), file, strerror(status)));
 		return status;
 	}
 
@@ -1303,7 +1300,7 @@ bfs_free_cookie(void *_ns, void *_node, void *_cookie)
 static status_t
 bfs_access(void *_ns, void *_node, int accessMode)
 {
-	FUNCTION();
+	//FUNCTION();
 	
 	if (_ns == NULL || _node == NULL)
 		return B_BAD_VALUE;