From 2a3a8bfa9a0be87bead347e50959f9a67534bc55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Tue, 28 Jul 2009 15:55:48 +0000 Subject: [PATCH] * Journal::_WriteTransactionToLog()'s return value was ignored previously, leading BFS to report success on actually failed transactions. Those transactions were even kept open, leading to a panic when starting the next transaction. * Transaction::Done() now returns a status - currently, this is only handled correctly where this is likely to happen without a disk fault, ie. if the transaction was too large to be written back safely. * Improved "bfs_journal" command output. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@31850 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- .../kernel/file_systems/bfs/Journal.cpp | 37 +++++++++++---- src/add-ons/kernel/file_systems/bfs/Journal.h | 11 +++-- .../file_systems/bfs/kernel_interface.cpp | 47 ++++++++++--------- 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/add-ons/kernel/file_systems/bfs/Journal.cpp b/src/add-ons/kernel/file_systems/bfs/Journal.cpp index bb06909511..dfa9eb9064 100644 --- a/src/add-ons/kernel/file_systems/bfs/Journal.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Journal.cpp @@ -690,10 +690,9 @@ Journal::_WriteTransactionToLog() 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. + // We created a transaction larger than one we can write back to + // disk - the only option we have (besides risking disk corruption + // by writing it back anyway), is to let it fail. dprintf("transaction too large (%d blocks, %d main, log size %d)!\n", (int)_TransactionSize(), (int)cache_blocks_in_main_transaction( fVolume->BlockCache(), fTransactionID), (int)fLogSize); @@ -959,15 +958,18 @@ Journal::Lock(Transaction* owner, bool separateSubTransactions) } -void +status_t Journal::Unlock(Transaction* owner, bool success) { if (fSeparateSubTransactions || recursive_lock_get_recursion(&fLock) == 1) { // we only end the transaction if we would really unlock it // TODO: what about failing transactions that do not unlock? // (they must make the parent fail, too) - if (fOwner != NULL) - _TransactionDone(success); + if (fOwner != NULL) { + status_t status = _TransactionDone(success); + if (status != B_OK) + return status; + } fTimestamp = system_time(); fOwner = NULL; @@ -978,6 +980,7 @@ Journal::Unlock(Transaction* owner, bool success) } recursive_lock_unlock(&fLock); + return B_OK; } @@ -1021,7 +1024,11 @@ Journal::_TransactionDone(bool success) return B_OK; } - return _WriteTransactionToLog(); + status_t status = _WriteTransactionToLog(); + if (status != B_OK) + fUnwrittenTransactions++; + + return status; } @@ -1033,8 +1040,18 @@ Journal::_TransactionDone(bool success) void Journal::Dump() { - kprintf("log start: %ld\n", fVolume->LogStart()); - kprintf("log end: %ld\n", fVolume->LogEnd()); + kprintf("Journal %p\n", this); + kprintf(" log start: %ld\n", fVolume->LogStart()); + kprintf(" log end: %ld\n", fVolume->LogEnd()); + kprintf(" owner: %p\n", fOwner); + kprintf(" log size: %lu\n", fLogSize); + kprintf(" max transaction size: %lu\n", fMaxTransactionSize); + kprintf(" used: %lu\n", fUsed); + kprintf(" unwritten: %ld\n", fUnwrittenTransactions); + kprintf(" timestamp: %lld\n", fTimestamp); + kprintf(" transaction ID: %ld\n", fTransactionID); + kprintf(" has subtransaction: %d\n", fHasSubtransaction); + kprintf(" separate sub-trans.: %d\n", fSeparateSubTransactions); kprintf("entries:\n"); kprintf(" address id start length\n"); diff --git a/src/add-ons/kernel/file_systems/bfs/Journal.h b/src/add-ons/kernel/file_systems/bfs/Journal.h index 5c468f3eb3..32c5a249d1 100644 --- a/src/add-ons/kernel/file_systems/bfs/Journal.h +++ b/src/add-ons/kernel/file_systems/bfs/Journal.h @@ -27,7 +27,7 @@ public: status_t Lock(Transaction* owner, bool separateSubTransactions); - void Unlock(Transaction* owner, bool success); + status_t Unlock(Transaction* owner, bool success); status_t ReplayLog(); @@ -115,13 +115,16 @@ public: status_t Start(Volume* volume, off_t refBlock); bool IsStarted() const { return fJournal != NULL; } - void Done() + status_t Done() { + status_t status = B_OK; if (fJournal != NULL) { _UnlockInodes(); - fJournal->Unlock(this, true); + status = fJournal->Unlock(this, true); + if (status == B_OK) + fJournal = NULL; } - fJournal = NULL; + return status; } bool HasParent() diff --git a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp index d6e8f5d1f0..e4c5948773 100644 --- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp @@ -359,10 +359,10 @@ bfs_remove_vnode(fs_volume* _volume, fs_vnode* _node, bool reenter) // transaction which has already deleted the inode. Transaction transaction(volume, volume->ToBlock(inode->Parent())); - // The "chkbfs" functionality uses this flag to prevent the space used - // up by the inode from being freed - this flag is set only in situations - // where this is a good idea... (the block bitmap will get fixed anyway - // in this case). + // The file system check functionality uses this flag to prevent the space + // used up by the inode from being freed - this flag is set only in + // situations where this does not cause any harm as the block bitmap will + // get fixed anyway in this case). if ((inode->Flags() & INODE_DONT_FREE_SPACE) != 0) { delete inode; return B_OK; @@ -370,14 +370,15 @@ bfs_remove_vnode(fs_volume* _volume, fs_vnode* _node, bool reenter) status_t status = inode->Free(transaction); if (status == B_OK) { - transaction.Done(); - - delete inode; + status = transaction.Done(); } else if (transaction.HasParent()) { // TODO: for now, we don't let sub-transactions fail - transaction.Done(); + status = transaction.Done(); } + // TODO: the VFS currently does not allow this to fail + delete inode; + return status; } @@ -777,7 +778,7 @@ bfs_write_stat(fs_volume* _volume, fs_vnode* _node, const struct stat* stat, off_t oldSize = inode->Size(); status = inode->SetFileSize(transaction, stat->st_size); - if (status < B_OK) + if (status != B_OK) return status; // fill the new blocks (if any) with zeros @@ -843,9 +844,9 @@ bfs_write_stat(fs_volume* _volume, fs_vnode* _node, const struct stat* stat, status = inode->WriteBack(transaction); if (status == B_OK) - transaction.Done(); - - notify_stat_changed(volume->ID(), inode->ID(), mask); + status = transaction.Done(); + if (status == B_OK) + notify_stat_changed(volume->ID(), inode->ID(), mask); return status; } @@ -891,10 +892,11 @@ bfs_create(fs_volume* _volume, fs_vnode* _directory, const char* name, status = file_cache_disable(inode->FileCache()); } - if (status >= B_OK) { - entry_cache_add(volume->ID(), directory->ID(), name, *_vnodeID); + if (status == B_OK) + status = transaction.Done(); - transaction.Done(); + if (status == B_OK) { + entry_cache_add(volume->ID(), directory->ID(), name, *_vnodeID); // register the cookie *_cookie = cookie; @@ -1017,7 +1019,8 @@ status_t bfs_rename(fs_volume* _volume, fs_vnode* _oldDir, const char* oldName, fs_vnode* _newDir, const char* newName) { - FUNCTION_START(("oldDir = %p, oldName = \"%s\", newDir = %p, newName = \"%s\"\n", _oldDir, oldName, _newDir, newName)); + FUNCTION_START(("oldDir = %p, oldName = \"%s\", newDir = %p, newName = " + "\"%s\"\n", _oldDir, oldName, _newDir, newName)); // there might be some more tests needed?! if (!strcmp(oldName, ".") || !strcmp(oldName, "..") @@ -1236,14 +1239,12 @@ bfs_open(fs_volume* _volume, fs_vnode* _node, int openMode, void** _cookie) inode->WriteLockInTransaction(transaction); status_t status = inode->SetFileSize(transaction, 0); - if (status < B_OK) + if (status == B_OK) + status = inode->WriteBack(transaction); + if (status == B_OK) + status = transaction.Done(); + if (status != B_OK) return status; - - status = inode->WriteBack(transaction); - if (status < B_OK) - return status; - - transaction.Done(); } fileCacheEnabler.Detach();