diff --git a/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp b/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp index d70cd922e9..ce808a2e3c 100644 --- a/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp +++ b/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp @@ -1147,7 +1147,7 @@ BlockAllocator::StartChecking(check_control* control) // Lock the volume's journal status_t status = mutex_lock(&fLock); - if (status < B_OK) { + if (status != B_OK) { fVolume->GetJournal(0)->Unlock(NULL, true); return status; } @@ -1185,16 +1185,12 @@ BlockAllocator::StartChecking(check_control* control) fCheckCookie = cookie; // to be able to restore nicely if "chkbfs" exited abnormally -#if !BFS_SHELL // Put removed vnodes to the stack -- they are not reachable by traversing // the file system anymore. - void* inode = NULL; - ino_t nodeID; - while (get_next_removed_vnode(fVolume->FSVolume(), &nodeID, &inode) - == B_OK) { - cookie->stack.Push(fVolume->ToBlockRun(nodeID)); + InodeList::Iterator iterator = fVolume->RemovedInodes().GetIterator(); + while (Inode* inode = iterator.Next()) { + cookie->stack.Push(inode->BlockRun()); } -#endif // TODO: check reserved area in bitmap! diff --git a/src/add-ons/kernel/file_systems/bfs/Debug.cpp b/src/add-ons/kernel/file_systems/bfs/Debug.cpp index e63057f4ff..86049dd674 100644 --- a/src/add-ons/kernel/file_systems/bfs/Debug.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Debug.cpp @@ -127,7 +127,6 @@ dump_inode(const bfs_inode* inode) dump_block_run( " attributes = ", inode->attributes); Print(" type = %u\n", (unsigned)inode->Type()); Print(" inode_size = %u\n", (unsigned)inode->InodeSize()); - Print(" etc = %#08x\n", (int)inode->etc); Print(" short_symlink = %s\n", S_ISLNK(inode->Mode()) && (inode->Flags() & INODE_LONG_SYMLINK) == 0 ? inode->short_symlink : "-"); diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp b/src/add-ons/kernel/file_systems/bfs/Inode.cpp index 1cb1c0a19a..8583598eec 100644 --- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp @@ -335,6 +335,7 @@ Inode::Inode(Volume* volume, ino_t id) NodeGetter node(volume, this); memcpy(&fNode, node.Node(), sizeof(bfs_inode)); + fNode.flags &= HOST_ENDIAN_TO_BFS_INT32(INODE_PERMANENT_FLAGS); char lockName[B_OS_NAME_LENGTH]; snprintf(lockName, sizeof(lockName), "bfs inode %d.%d", @@ -411,6 +412,9 @@ Inode::~Inode() file_map_delete(Map()); delete fTree; + if ((Flags() & INODE_DELETED) != 0) + fVolume->RemovedInodes().Remove(this); + rw_lock_destroy(&fLock); } @@ -424,7 +428,7 @@ Inode::InitCheck(bool checkNode) if (status == B_BUSY) return B_BUSY; - if (status < B_OK) { + if (status != B_OK) { FATAL(("inode at block %Ld corrupt!\n", BlockNumber())); RETURN_ERROR(B_BAD_DATA); } @@ -436,7 +440,7 @@ Inode::InitCheck(bool checkNode) RETURN_ERROR(B_NO_MEMORY); status_t status = fTree->InitCheck(); - if (status < B_OK) { + if (status != B_OK) { FATAL(("inode tree at block %Ld corrupt!\n", BlockNumber())); RETURN_ERROR(B_BAD_DATA); } @@ -551,7 +555,7 @@ Inode::_MakeSpaceForSmallData(Transaction& transaction, bfs_inode* node, Inode* attribute; status_t status = CreateAttribute(transaction, item->Name(), item->Type(), &attribute); - if (status < B_OK) + if (status != B_OK) RETURN_ERROR(status); size_t length = item->DataSize(); @@ -559,7 +563,7 @@ Inode::_MakeSpaceForSmallData(Transaction& transaction, bfs_inode* node, ReleaseAttribute(attribute); - if (status < B_OK) { + if (status != B_OK) { Vnode vnode(fVolume, Attributes()); Inode* attributes; if (vnode.Get(&attributes) < B_OK @@ -947,6 +951,8 @@ Inode::_RemoveAttribute(Transaction& transaction, const char* name, return status; if (attributes->IsEmpty()) { + attributes->WriteLockInTransaction(transaction); + // remove attribute directory (don't fail if that can't be done) if (remove_vnode(fVolume->FSVolume(), attributes->ID()) == B_OK) { // update the inode, so that no one will ever doubt it's deleted :-) @@ -954,8 +960,11 @@ Inode::_RemoveAttribute(Transaction& transaction, const char* name, if (attributes->WriteBack(transaction) == B_OK) { Attributes().SetTo(0, 0, 0); WriteBack(transaction); - } else + } else { unremove_vnode(fVolume->FSVolume(), attributes->ID()); + attributes->Node().flags + &= ~HOST_ENDIAN_TO_BFS_INT32(INODE_DELETED); + } } } diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.h b/src/add-ons/kernel/file_systems/bfs/Inode.h index 3547011ba2..ea3bffb0ff 100644 --- a/src/add-ons/kernel/file_systems/bfs/Inode.h +++ b/src/add-ons/kernel/file_systems/bfs/Inode.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2008, Axel Dörfler, axeld@pinc-software.de. + * Copyright 2001-2009, Axel Dörfler, axeld@pinc-software.de. * This file may be used under the terms of the MIT License. */ #ifndef INODE_H @@ -24,7 +24,9 @@ class NodeGetter; class Transaction; -class Inode : public SinglyLinkedListLinkImpl { +class Inode { + typedef DoublyLinkedListLink Link; + public: Inode(Volume* volume, ino_t id); Inode(Volume* volume, Transaction& transaction, @@ -174,6 +176,11 @@ public: { ASSERT_WRITE_LOCKED_RW_LOCK(&fLock); } #endif + Link* GetDoublyLinkedListLink() + { return (Link*)&fNode.pad[0]; } + const Link* GetDoublyLinkedListLink() const + { return (Link*)&fNode.pad[0]; } + private: Inode(const Inode& other); Inode& operator=(const Inode& other); diff --git a/src/add-ons/kernel/file_systems/bfs/Journal.cpp b/src/add-ons/kernel/file_systems/bfs/Journal.cpp index 6541598127..4882d08500 100644 --- a/src/add-ons/kernel/file_systems/bfs/Journal.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Journal.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2008, Axel Dörfler, axeld@pinc-software.de. + * Copyright 2001-2009, Axel Dörfler, axeld@pinc-software.de. * This file may be used under the terms of the MIT License. */ @@ -1091,18 +1091,21 @@ Transaction::AddInode(Inode* inode) if (fJournal == NULL) panic("Transaction is not running!"); - InodeList::Iterator iterator = fLockedInodes.GetIterator(); - while (iterator.HasNext()) { - if (iterator.Next() == inode) { - //dprintf(" inode %Ld already in transaction\n", inode->ID()); - return; - } - } + // These flags can only change while holding the transaction lock + if ((inode->Flags() & INODE_IN_TRANSACTION) != 0) + return; + + // We share the same list link with the removed list, so we have to remove + // the inode from that list here (and add it back when we no longer need it) + if ((inode->Flags() & INODE_DELETED) != 0) + GetVolume()->RemovedInodes().Remove(inode); if (!GetVolume()->IsInitializing()) acquire_vnode(GetVolume()->FSVolume(), inode->ID()); + rw_lock_write_lock(&inode->fLock); fLockedInodes.Add(inode); + inode->Node().flags |= HOST_ENDIAN_TO_BFS_INT32(INODE_IN_TRANSACTION); } @@ -1112,8 +1115,14 @@ Transaction::RemoveInode(Inode* inode) if (fJournal == NULL) panic("Transaction is not running!"); + inode->Node().flags &= ~HOST_ENDIAN_TO_BFS_INT32(INODE_IN_TRANSACTION); fLockedInodes.Remove(inode); rw_lock_write_unlock(&inode->fLock); + + // See AddInode() why we do this here + if ((inode->Flags() & INODE_DELETED) != 0) + GetVolume()->RemovedInodes().Add(inode); + if (!GetVolume()->IsInitializing()) put_vnode(GetVolume()->FSVolume(), inode->ID()); } @@ -1123,7 +1132,13 @@ void Transaction::_UnlockInodes() { while (Inode* inode = fLockedInodes.RemoveHead()) { + inode->Node().flags &= ~HOST_ENDIAN_TO_BFS_INT32(INODE_IN_TRANSACTION); rw_lock_write_unlock(&inode->fLock); + + // See AddInode() why we do this here + if ((inode->Flags() & INODE_DELETED) != 0) + GetVolume()->RemovedInodes().Add(inode); + if (!GetVolume()->IsInitializing()) put_vnode(GetVolume()->FSVolume(), inode->ID()); } diff --git a/src/add-ons/kernel/file_systems/bfs/Journal.h b/src/add-ons/kernel/file_systems/bfs/Journal.h index b4a2195b09..5c468f3eb3 100644 --- a/src/add-ons/kernel/file_systems/bfs/Journal.h +++ b/src/add-ons/kernel/file_systems/bfs/Journal.h @@ -16,7 +16,6 @@ struct run_array; class Inode; class LogEntry; typedef DoublyLinkedList LogEntryList; -typedef SinglyLinkedList InodeList; class Journal { diff --git a/src/add-ons/kernel/file_systems/bfs/Volume.h b/src/add-ons/kernel/file_systems/bfs/Volume.h index 7a803277b7..1192ecc6db 100644 --- a/src/add-ons/kernel/file_systems/bfs/Volume.h +++ b/src/add-ons/kernel/file_systems/bfs/Volume.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2008, Axel Dörfler, axeld@pinc-software.de. + * Copyright 2001-2009, Axel Dörfler, axeld@pinc-software.de. * This file may be used under the terms of the MIT License. */ #ifndef VOLUME_H @@ -23,6 +23,8 @@ enum volume_initialize_flags { VOLUME_NO_INDICES = 0x0001, }; +typedef DoublyLinkedList InodeList; + class Volume { public: Volume(fs_volume* volume); @@ -86,6 +88,9 @@ public: status_t CreateIndicesRoot(Transaction& transaction); + InodeList& RemovedInodes() { return fRemovedInodes; } + // This list is guarded by the transaction lock + // block bitmap BlockAllocator& Allocator(); status_t AllocateForInode(Transaction& transaction, @@ -127,7 +132,7 @@ public: uint32* _offset = NULL); static status_t Identify(int fd, disk_super_block* superBlock); - protected: +protected: fs_volume* fVolume; int fDevice; disk_super_block fSuperBlock; @@ -155,6 +160,8 @@ public: void* fBlockCache; thread_id fCheckingThread; + + InodeList fRemovedInodes; }; diff --git a/src/add-ons/kernel/file_systems/bfs/bfs.h b/src/add-ons/kernel/file_systems/bfs/bfs.h index a2c1619458..5d56e632be 100644 --- a/src/add-ons/kernel/file_systems/bfs/bfs.h +++ b/src/add-ons/kernel/file_systems/bfs/bfs.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2008, Axel Dörfler, axeld@pinc-software.de. + * Copyright 2001-2009, Axel Dörfler, axeld@pinc-software.de. * Parts of this code is based on work previously done by Marcus Overhagen. * * This file may be used under the terms of the MIT License. @@ -165,7 +165,8 @@ struct small_data { class Volume; -#define SHORT_SYMLINK_NAME_LENGTH 144 // length incl. terminating '\0' +#define SHORT_SYMLINK_NAME_LENGTH 144 + // length incl. terminating '\0' struct bfs_inode { int32 magic1; @@ -181,13 +182,14 @@ struct bfs_inode { uint32 type; // attribute type int32 inode_size; - uint32 etc; // a pointer to the Inode object during construction + uint32 etc; union { data_stream data; char short_symlink[SHORT_SYMLINK_NAME_LENGTH]; }; int32 pad[4]; + // we use this member as a doubly linked list link small_data small_data_start[0]; @@ -222,6 +224,8 @@ enum inode_flags { INODE_PERMANENT_FLAGS = 0x0000ffff, INODE_WAS_WRITTEN = 0x00020000, + INODE_IN_TRANSACTION = 0x00040000, + // The rest is only used by the file system check functionality INODE_DONT_FREE_SPACE = 0x00080000 }; 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 781c1c0e40..3b9f588d71 100644 --- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp @@ -266,7 +266,7 @@ bfs_get_vnode(fs_volume* _volume, ino_t id, fs_vnode* _node, int* _type, } status_t status = node->InitCheck(volume); - if (status < B_OK) { + if (status != B_OK) { if ((node->Flags() & INODE_DELETED) != 0) { INFORM(("inode at %Ld is already deleted!\n", id)); } else { @@ -281,7 +281,7 @@ bfs_get_vnode(fs_volume* _volume, ino_t id, fs_vnode* _node, int* _type, return B_NO_MEMORY; status = inode->InitCheck(false); - if (status < B_OK) + if (status != B_OK) delete inode; if (status == B_OK) { @@ -328,20 +328,20 @@ bfs_remove_vnode(fs_volume* _volume, fs_vnode* _node, bool reenter) Volume* volume = (Volume*)_volume->private_volume; Inode* inode = (Inode*)_node->private_node; - // 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). - if (inode->Flags() & INODE_DONT_FREE_SPACE) { - delete inode; - return B_OK; - } - // 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 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). + if ((inode->Flags() & INODE_DONT_FREE_SPACE) != 0) { + delete inode; + return B_OK; + } + status_t status = inode->Free(transaction); if (status == B_OK) { transaction.Done();