Author: bonefish Date: 2010-01-03 00:27:41 +0100 (Sun, 03 Jan 2010) New Revision: 34865 Changeset: http://dev.haiku-os.org/changeset/34865/haiku Added: haiku/trunk/src/system/kernel/fs/Vnode.cpp haiku/trunk/src/system/kernel/fs/Vnode.h Modified: haiku/trunk/src/system/kernel/fs/Jamfile haiku/trunk/src/system/kernel/fs/vfs.cpp Log: * Moved the vnode structure (to by Vnode class at some time in the future) into its own header/source files. * Changed vnode's bit fields to a single, atomically changeable int32 using flags instead. Added respective accessor methods. * Added a per-vnode mutex-like lock, which uses 2 bits of the structure and 32 global "buckets" which are used for waiter lists for the vnode locks. * Reorganized the VFS locking a bit: Renamed sVnodeMutex to sVnodeLock and made it an r/w lock. In most situations it is now only read-locked to reduce its contention. The per-vnode locks guard the fields of the vnode structure and the newly introduced sUnusedVnodesLock has taken over the job to guard the unused vnodes list. The main intent of the changes was to reduce the contention of the sVnodeMutex, which was partially successful. In my standard -j8 Haiku image build test the new sUnusedVnodesLock took over about a fourth of the former sVnodeMutex contention, but the sVnodeLock and the vnode locks have virtually no contention to speak of, now. A lot of contention migrated to the unrelated "pages" mutex (another bottleneck). The overall build time dropped about 10 %. Modified: haiku/trunk/src/system/kernel/fs/Jamfile =================================================================== --- haiku/trunk/src/system/kernel/fs/Jamfile 2010-01-02 22:11:08 UTC (rev 34864) +++ haiku/trunk/src/system/kernel/fs/Jamfile 2010-01-02 23:27:41 UTC (rev 34865) @@ -14,6 +14,7 @@ node_monitor.cpp rootfs.cpp socket.cpp + Vnode.cpp vfs.cpp vfs_boot.cpp vfs_net_boot.cpp Added: haiku/trunk/src/system/kernel/fs/Vnode.cpp =================================================================== --- haiku/trunk/src/system/kernel/fs/Vnode.cpp (rev 0) +++ haiku/trunk/src/system/kernel/fs/Vnode.cpp 2010-01-02 23:27:41 UTC (rev 34865) @@ -0,0 +1,93 @@ +/* + * Copyright 2009, Ingo Weinhold, ingo_weinhold@xxxxxxx + * Distributed under the terms of the MIT License. + */ + + +#include "Vnode.h" + +#include <util/AutoLock.h> + + +vnode::Bucket vnode::sBuckets[kBucketCount]; + + +vnode::Bucket::Bucket() +{ + mutex_init(&lock, "vnode bucket"); +} + + +/*static*/ void +vnode::StaticInit() +{ + for (uint32 i = 0; i < kBucketCount; i++) + new(&sBuckets[i]) Bucket; +} + + +void +vnode::_WaitForLock() +{ + LockWaiter waiter; + waiter.thread = thread_get_current_thread(); + waiter.vnode = this; + + Bucket& bucket = _Bucket(); + MutexLocker bucketLocker(bucket.lock); + + if ((atomic_or(&fFlags, kFlagsWaitingLocker) + & (kFlagsLocked | kFlagsWaitingLocker)) == 0) { + // The lock holder dropped it in the meantime and no-one else was faster + // than us, so it's ours now. Just mark the node locked and clear the + // waiting flag again. + atomic_or(&fFlags, kFlagsLocked); + atomic_and(&fFlags, ~kFlagsWaitingLocker); + return; + } + + // prepare for waiting + bucket.waiters.Add(&waiter); + thread_prepare_to_block(waiter.thread, 0, THREAD_BLOCK_TYPE_OTHER, + "vnode lock"); + + // start waiting + bucketLocker.Unlock(); + thread_block(); +} + + +void +vnode::_WakeUpLocker() +{ + Bucket& bucket = _Bucket(); + MutexLocker bucketLocker(bucket.lock); + + // mark the node locked again + atomic_or(&fFlags, kFlagsLocked); + + // get the first waiter from the list + LockWaiter* waiter = NULL; + bool onlyWaiter = true; + for (LockWaiterList::Iterator it = bucket.waiters.GetIterator(); + LockWaiter* someWaiter = it.Next();) { + if (someWaiter->vnode == this) { + if (waiter != NULL) { + onlyWaiter = false; + break; + } + waiter = someWaiter; + it.Remove(); + } + } + + ASSERT(waiter != NULL); + + // if that's the only waiter, clear the flag + if (onlyWaiter) + atomic_and(&fFlags, ~kFlagsWaitingLocker); + + // and wake it up + InterruptsSpinLocker threadLocker(gThreadSpinlock); + thread_unblock_locked(waiter->thread, B_OK); +} Added: haiku/trunk/src/system/kernel/fs/Vnode.h =================================================================== --- haiku/trunk/src/system/kernel/fs/Vnode.h (rev 0) +++ haiku/trunk/src/system/kernel/fs/Vnode.h 2010-01-02 23:27:41 UTC (rev 34865) @@ -0,0 +1,203 @@ +/* + * Copyright 2009, Ingo Weinhold, ingo_weinhold@xxxxxxx + * Distributed under the terms of the MIT License. + */ +#ifndef VNODE_H +#define VNODE_H + + +#include <fs_interface.h> + +#include <util/DoublyLinkedList.h> +#include <util/list.h> + +#include <lock.h> +#include <thread.h> + + +struct advisory_locking; +struct file_descriptor; +struct fs_mount; +struct VMCache; + +typedef struct vnode Vnode; + + +struct vnode : fs_vnode, DoublyLinkedListLinkImpl<vnode> { + struct vnode* next; + VMCache* cache; + struct fs_mount* mount; + struct vnode* covered_by; + struct advisory_locking* advisory_locking; + struct file_descriptor* mandatory_locked_by; + list_link unused_link; + ino_t id; + dev_t device; + int32 ref_count; + +public: + inline bool IsBusy() const; + inline void SetBusy(bool busy); + + inline bool IsRemoved() const; + inline void SetRemoved(bool removed); + + inline bool IsUnpublished() const; + inline void SetUnpublished(bool unpublished); + + inline uint32 Type() const; + inline void SetType(uint32 type); + + inline bool Lock(); + inline void Unlock(); + + static void StaticInit(); + +private: + static const uint32 kFlagsLocked = 0x00000001; + static const uint32 kFlagsWaitingLocker = 0x00000002; + static const uint32 kFlagsBusy = 0x00000004; + static const uint32 kFlagsRemoved = 0x00000008; + static const uint32 kFlagsUnpublished = 0x00000010; + static const uint32 kFlagsType = 0xfffff000; + + static const uint32 kBucketCount = 32; + + struct LockWaiter : DoublyLinkedListLinkImpl<LockWaiter> { + LockWaiter* next; + struct thread* thread; + struct vnode* vnode; + }; + + typedef DoublyLinkedList<LockWaiter> LockWaiterList; + + struct Bucket { + mutex lock; + LockWaiterList waiters; + + Bucket(); + }; + +private: + inline Bucket& _Bucket() const; + + void _WaitForLock(); + void _WakeUpLocker(); + +private: + vint32 fFlags; + + static Bucket sBuckets[kBucketCount]; +}; + + +bool +vnode::IsBusy() const +{ + return (fFlags & kFlagsBusy) != 0; +} + + +void +vnode::SetBusy(bool busy) +{ + if (busy) + atomic_or(&fFlags, kFlagsBusy); + else + atomic_and(&fFlags, ~kFlagsBusy); +} + + +bool +vnode::IsRemoved() const +{ + return (fFlags & kFlagsRemoved) != 0; +} + + +void +vnode::SetRemoved(bool removed) +{ + if (removed) + atomic_or(&fFlags, kFlagsRemoved); + else + atomic_and(&fFlags, ~kFlagsRemoved); +} + + +bool +vnode::IsUnpublished() const +{ + return (fFlags & kFlagsUnpublished) != 0; +} + + +void +vnode::SetUnpublished(bool unpublished) +{ + if (unpublished) + atomic_or(&fFlags, kFlagsUnpublished); + else + atomic_and(&fFlags, ~kFlagsUnpublished); +} + + +uint32 +vnode::Type() const +{ + return (uint32)fFlags & kFlagsType; +} + + +void +vnode::SetType(uint32 type) +{ + atomic_and(&fFlags, ~kFlagsType); + atomic_or(&fFlags, type & kFlagsType); +} + + +/*! Locks the vnode. + The caller must hold sVnodeLock (at least read locked) and must continue to + hold it until calling Unlock(). After acquiring the lock the caller is + allowed to write access the vnode's mutable fields, if it hasn't been marked + busy by someone else. + Due to the condition of holding sVnodeLock at least read locked, write + locking it grants the same write access permission to *any* vnode. + + The vnode's lock should be held only for a short time. It can be held over + sUnusedVnodesLock. + + \return Always \c true. +*/ +bool +vnode::Lock() +{ + if ((atomic_or(&fFlags, kFlagsLocked) + & (kFlagsLocked | kFlagsWaitingLocker)) != 0) { + _WaitForLock(); + } + + return true; +} + +void +vnode::Unlock() +{ + if ((atomic_and(&fFlags, ~kFlagsLocked) & kFlagsWaitingLocker) != 0) + _WakeUpLocker(); +} + + +vnode::Bucket& +vnode::_Bucket() const +{ + return sBuckets[((addr_t)this / 64) % kBucketCount]; + // The vnode structure is somewhat larger than 64 bytes (on 32 bit + // archs), so subsequently allocated vnodes fall into different + // buckets. How exactly the vnodes are distributed depends on the + // allocator -- a dedicated slab would be perfect. +} + + +#endif // VNODE_H Modified: haiku/trunk/src/system/kernel/fs/vfs.cpp =================================================================== --- haiku/trunk/src/system/kernel/fs/vfs.cpp 2010-01-02 22:11:08 UTC (rev 34864) +++ haiku/trunk/src/system/kernel/fs/vfs.cpp 2010-01-02 23:27:41 UTC (rev 34865) @@ -56,6 +56,7 @@ #include "EntryCache.h" #include "fifo.h" #include "IORequest.h" +#include "Vnode.h" #include "../cache/vnode_store.h" @@ -115,36 +116,6 @@ // on PATH_MAX -struct vnode : fs_vnode, DoublyLinkedListLinkImpl<vnode> { - struct vnode* next; - VMCache* cache; - dev_t device; - list_link unused_link; - ino_t id; - struct fs_mount* mount; - struct vnode* covered_by; - int32 ref_count; -private: - uint32 fType : 20; // right-shifted by 12 - uint32 fUnused : 9; -public: - bool remove : 1; - bool busy : 1; - bool unpublished : 1; - struct advisory_locking* advisory_locking; - struct file_descriptor* mandatory_locked_by; - - void SetType(uint32 type) - { - fType = type >> 12; - } - - uint32 Type() const - { - return fType << 12; - } -}; - struct vnode_hash_key { dev_t device; ino_t vnode; @@ -195,6 +166,7 @@ fs_volume* volume; char* device_name; recursive_lock rlock; // guards the vnodes list + // TODO: Make this a mutex! It is never used recursively. struct vnode* root_vnode; struct vnode* covers_vnode; KPartition* partition; @@ -254,7 +226,7 @@ sMountsTable will not be modified, - vnode::covered_by of any vnode in sVnodeTable will not be modified. - The thread trying to lock the lock must not hold sVnodeMutex or + The thread trying to lock the lock must not hold sVnodeLock or sMountMutex. */ static recursive_lock sMountOpLock; @@ -264,7 +236,7 @@ The holder is allowed to read access the vnode::covered_by field of any vnode. Additionally holding sMountOpLock allows for write access. - The thread trying to lock the must not hold sVnodeMutex. + The thread trying to lock the must not hold sVnodeLock. */ static mutex sVnodeCoveredByMutex = MUTEX_INITIALIZER("vfs_vnode_covered_by_lock"); @@ -281,7 +253,7 @@ You must not have this mutex held when calling create_sem(), as this might call vfs_free_unused_vnodes(). */ -static mutex sVnodeMutex = MUTEX_INITIALIZER("vfs_vnode_lock"); +static rw_lock sVnodeLock = RW_LOCK_INITIALIZER("vfs_vnode_lock"); /*! \brief Guards io_context::root. @@ -291,6 +263,13 @@ */ static mutex sIOContextRootLock = MUTEX_INITIALIZER("io_context::root lock"); +/*! \brief Guards sUnusedVnodeList and sUnusedVnodes. + + Innermost lock. Must not be held when acquiring any other lock. +*/ +static mutex sUnusedVnodesLock = MUTEX_INITIALIZER("unused vnodes"); + + #define VNODE_HASH_TABLE_SIZE 1024 static hash_table* sVnodeTable; static list sUnusedVnodeList; @@ -723,7 +702,7 @@ { struct fs_mount* mount; - MutexLocker nodeLocker(sVnodeMutex); + ReadLocker nodeLocker(sVnodeLock); MutexLocker mountLocker(sMountMutex); mount = find_mount(id); @@ -731,7 +710,7 @@ return B_BAD_VALUE; struct vnode* rootNode = mount->root_vnode; - if (rootNode == NULL || rootNode->busy || rootNode->ref_count == 0) { + if (rootNode == NULL || rootNode->IsBusy() || rootNode->ref_count == 0) { // might have been called during a mount/unmount operation return B_BUSY; } @@ -888,10 +867,49 @@ } +/*! \brief Looks up a vnode by mount and node ID in the sVnodeTable. + + The caller must hold the sVnodeLock (read lock at least). + + \param mountID the mount ID. + \param vnodeID the node ID. + + \return The vnode structure, if it was found in the hash table, \c NULL + otherwise. +*/ +static struct vnode* +lookup_vnode(dev_t mountID, ino_t vnodeID) +{ + struct vnode_hash_key key; + + key.device = mountID; + key.vnode = vnodeID; + + return (vnode*)hash_lookup(sVnodeTable, &key); +} + + +/*! Creates a new vnode with the given mount and node ID. + If the node already exists, it is returned instead and no new node is + created. In either case -- but not, if an error occurs -- the function write + locks \c sVnodeLock and keeps it locked for the caller when returning. On + error the lock is not not held on return. + + \param mountID The mount ID. + \param vnodeID The vnode ID. + \param _vnode Will be set to the new vnode on success. + \param _nodeCreated Will be set to \c true when the returned vnode has + been newly created, \c false when it already existed. Will not be + changed on error. + \return \c B_OK, when the vnode was successfully created and inserted or + a node with the given ID was found, \c B_NO_MEMORY or + \c B_ENTRY_NOT_FOUND on error. +*/ static status_t -create_new_vnode(struct vnode** _vnode, dev_t mountID, ino_t vnodeID) +create_new_vnode_and_lock(dev_t mountID, ino_t vnodeID, struct vnode*& _vnode, + bool& _nodeCreated) { - FUNCTION(("create_new_vnode()\n")); + FUNCTION(("create_new_vnode_and_lock()\n")); struct vnode* vnode = (struct vnode*)malloc(sizeof(struct vnode)); if (vnode == NULL) @@ -901,24 +919,40 @@ memset(vnode, 0, sizeof(struct vnode)); vnode->device = mountID; vnode->id = vnodeID; + vnode->ref_count = 1; + vnode->SetBusy(true); - // add the vnode to the mount structure + // look up the the node -- it might have been added by someone else in the + // meantime + rw_lock_write_lock(&sVnodeLock); + struct vnode* existingVnode = lookup_vnode(mountID, vnodeID); + if (existingVnode != NULL) { + free(vnode); + _vnode = existingVnode; + _nodeCreated = false; + return B_OK; + } + + // get the mount structure mutex_lock(&sMountMutex); vnode->mount = find_mount(mountID); if (!vnode->mount || vnode->mount->unmounting) { mutex_unlock(&sMountMutex); + rw_lock_write_unlock(&sVnodeLock); free(vnode); return B_ENTRY_NOT_FOUND; } + // add the vnode to the mount's node list and the hash table hash_insert(sVnodeTable, vnode); add_vnode_to_mount_list(vnode, vnode->mount); mutex_unlock(&sMountMutex); - vnode->ref_count = 1; - *_vnode = vnode; + _vnode = vnode; + _nodeCreated = true; + // keep the vnode lock locked return B_OK; } @@ -930,13 +964,14 @@ static void free_vnode(struct vnode* vnode, bool reenter) { - ASSERT_PRINT(vnode->ref_count == 0 && vnode->busy, "vnode: %p\n", vnode); + ASSERT_PRINT(vnode->ref_count == 0 && vnode->IsBusy(), "vnode: %p\n", + vnode); // write back any changes in this vnode's cache -- but only // if the vnode won't be deleted, in which case the changes // will be discarded - if (!vnode->remove && HAS_FS_CALL(vnode, fsync)) + if (!vnode->IsRemoved() && HAS_FS_CALL(vnode, fsync)) FS_CALL_NO_PARAMS(vnode, fsync); // Note: If this vnode has a cache attached, there will still be two @@ -951,8 +986,8 @@ // count, so that it will neither become negative nor 0. vnode->ref_count = 2; - if (!vnode->unpublished) { - if (vnode->remove) + if (!vnode->IsUnpublished()) { + if (vnode->IsRemoved()) FS_CALL(vnode, remove_vnode, reenter); else FS_CALL(vnode, put_vnode, reenter); @@ -968,9 +1003,9 @@ // The file system has removed the resources of the vnode now, so we can // make it available again (by removing the busy vnode from the hash). - mutex_lock(&sVnodeMutex); + rw_lock_write_lock(&sVnodeLock); hash_remove(sVnodeTable, vnode); - mutex_unlock(&sVnodeMutex); + rw_lock_write_unlock(&sVnodeLock); // if we have a VMCache attached, remove it if (vnode->cache) @@ -989,7 +1024,7 @@ The caller must, of course, own a reference to the vnode to call this function. - The caller must not hold the sVnodeMutex or the sMountMutex. + The caller must not hold the sVnodeLock or the sMountMutex. \param vnode the vnode. \param alwaysFree don't move this vnode into the unused list, but really @@ -1001,7 +1036,8 @@ static status_t dec_vnode_ref_count(struct vnode* vnode, bool alwaysFree, bool reenter) { - MutexLocker locker(sVnodeMutex); + ReadLocker locker(sVnodeLock); + AutoLocker<Vnode> nodeLocker(vnode); int32 oldRefCount = atomic_add(&vnode->ref_count, -1); @@ -1013,17 +1049,18 @@ if (oldRefCount != 1) return B_OK; - if (vnode->busy) + if (vnode->IsBusy()) panic("dec_vnode_ref_count: called on busy vnode %p\n", vnode); bool freeNode = false; // Just insert the vnode into an unused list if we don't need // to delete it - if (vnode->remove || alwaysFree) { - vnode->busy = true; + if (vnode->IsRemoved() || alwaysFree) { + vnode->SetBusy(true); freeNode = true; } else { + MutexLocker unusedVnodesLocker(sUnusedVnodesLock); list_add_item(&sUnusedVnodeList, vnode); if (++sUnusedVnodes > kMaxUnusedVnodes && low_resource_state( @@ -1032,12 +1069,13 @@ // there are too many unused vnodes so we free the oldest one // TODO: evaluate this mechanism vnode = (struct vnode*)list_remove_head_item(&sUnusedVnodeList); - vnode->busy = true; + vnode->SetBusy(true); freeNode = true; sUnusedVnodes--; } } + nodeLocker.Unlock(); locker.Unlock(); if (freeNode) @@ -1049,9 +1087,18 @@ /*! \brief Increments the reference counter of the given vnode. - The caller must either already have a reference to the vnode or hold - the sVnodeMutex. + The caller must make sure that the node isn't deleted while this function + is called. This can be done either: + - by ensuring that a reference to the node exists and remains in existence, + or + - by holding the vnode's lock (which also requires read locking sVnodeLock) + or by holding sVnodeLock write locked. + In the second case the caller is responsible for dealing with the ref count + 0 -> 1 transition. That is 1. this function must not be invoked when the + node is busy in the first place and 2. the node must possibly be removed + from the unused node list and the unused node counter must be incremented. + \param vnode the vnode. */ static void @@ -1063,28 +1110,6 @@ } -/*! \brief Looks up a vnode by mount and node ID in the sVnodeTable. - - The caller must hold the sVnodeMutex. - - \param mountID the mount ID. - \param vnodeID the node ID. - - \return The vnode structure, if it was found in the hash table, \c NULL - otherwise. -*/ -static struct vnode* -lookup_vnode(dev_t mountID, ino_t vnodeID) -{ - struct vnode_hash_key key; - - key.device = mountID; - key.vnode = vnodeID; - - return (vnode*)hash_lookup(sVnodeTable, &key); -} - - static bool is_special_node_type(int type) { @@ -1107,7 +1132,7 @@ If the node is not yet in memory, it will be loaded. - The caller must not hold the sVnodeMutex or the sMountMutex. + The caller must not hold the sVnodeLock or the sMountMutex. \param mountID the mount ID. \param vnodeID the node ID. @@ -1124,14 +1149,17 @@ FUNCTION(("get_vnode: mountid %ld vnid 0x%Lx %p\n", mountID, vnodeID, _vnode)); - mutex_lock(&sVnodeMutex); + rw_lock_read_lock(&sVnodeLock); int32 tries = 2000; // try for 10 secs restart: struct vnode* vnode = lookup_vnode(mountID, vnodeID); - if (vnode && vnode->busy) { - mutex_unlock(&sVnodeMutex); + AutoLocker<Vnode> nodeLocker(vnode); + + if (vnode && vnode->IsBusy()) { + nodeLocker.Unlock(); + rw_lock_read_unlock(&sVnodeLock); if (!canWait || --tries < 0) { // vnode doesn't seem to become unbusy dprintf("vnode %ld:%Ld is not becoming unbusy!\n", mountID, @@ -1139,7 +1167,7 @@ return B_BUSY; } snooze(5000); // 5 ms - mutex_lock(&sVnodeMutex); + rw_lock_read_lock(&sVnodeLock); goto restart; } @@ -1150,19 +1178,32 @@ if (vnode) { if (vnode->ref_count == 0) { // this vnode has been unused before + MutexLocker unusedVnodesLocker(sUnusedVnodesLock); list_remove_item(&sUnusedVnodeList, vnode); sUnusedVnodes--; } inc_vnode_ref_count(vnode); + + nodeLocker.Unlock(); + rw_lock_read_unlock(&sVnodeLock); } else { // we need to create a new vnode and read it in - status = create_new_vnode(&vnode, mountID, vnodeID); + rw_lock_read_unlock(&sVnodeLock); + // unlock -- create_new_vnode_and_lock() write-locks on success + bool nodeCreated; + status = create_new_vnode_and_lock(mountID, vnodeID, vnode, + nodeCreated); if (status != B_OK) - goto err; + return status; - vnode->busy = true; - mutex_unlock(&sVnodeMutex); + if (!nodeCreated) { + rw_lock_read_lock(&sVnodeLock); + rw_lock_write_unlock(&sVnodeLock); + goto restart; + } + rw_lock_write_unlock(&sVnodeLock); + int type; uint32 flags; status = FS_MOUNT_CALL(vnode->mount, get_vnode, vnodeID, vnode, &type, @@ -1181,38 +1222,33 @@ if (gotNode && publishSpecialSubNode) status = create_special_sub_node(vnode, flags); - mutex_lock(&sVnodeMutex); - if (status != B_OK) { - if (gotNode) { - mutex_unlock(&sVnodeMutex); + if (gotNode) FS_CALL(vnode, put_vnode, reenter); - mutex_lock(&sVnodeMutex); - } - goto err1; + rw_lock_write_lock(&sVnodeLock); + hash_remove(sVnodeTable, vnode); + remove_vnode_from_mount_list(vnode, vnode->mount); + rw_lock_write_unlock(&sVnodeLock); + + free(vnode); + return status; } - vnode->remove = (flags & B_VNODE_PUBLISH_REMOVED) != 0; - vnode->busy = false; + rw_lock_read_lock(&sVnodeLock); + vnode->Lock(); + + vnode->SetRemoved((flags & B_VNODE_PUBLISH_REMOVED) != 0); + vnode->SetBusy(false); + + vnode->Unlock(); + rw_lock_read_unlock(&sVnodeLock); } - mutex_unlock(&sVnodeMutex); - TRACE(("get_vnode: returning %p\n", vnode)); *_vnode = vnode; return B_OK; - -err1: - hash_remove(sVnodeTable, vnode); - remove_vnode_from_mount_list(vnode, vnode->mount); -err: - mutex_unlock(&sVnodeMutex); - if (vnode) - free(vnode); - - return status; } @@ -1221,7 +1257,7 @@ The caller must, of course, own a reference to the vnode to call this function. - The caller must not hold the sVnodeMutex or the sMountMutex. + The caller must not hold the sVnodeLock or the sMountMutex. \param vnode the vnode. */ @@ -1237,40 +1273,63 @@ { TRACE(("vnode_low_resource_handler(level = %ld)\n", level)); + // determine how many nodes to free uint32 count = 1; - switch (level) { - case B_NO_LOW_RESOURCE: - return; - case B_LOW_RESOURCE_NOTE: - count = sUnusedVnodes / 100; - break; - case B_LOW_RESOURCE_WARNING: - count = sUnusedVnodes / 10; - break; - case B_LOW_RESOURCE_CRITICAL: + { + MutexLocker unusedVnodesLocker(sUnusedVnodesLock); + + switch (level) { + case B_NO_LOW_RESOURCE: + return; + case B_LOW_RESOURCE_NOTE: + count = sUnusedVnodes / 100; + break; + case B_LOW_RESOURCE_WARNING: + count = sUnusedVnodes / 10; + break; + case B_LOW_RESOURCE_CRITICAL: + count = sUnusedVnodes; + break; + } + + if (count > sUnusedVnodes) count = sUnusedVnodes; - break; } - if (count > sUnusedVnodes) - count = sUnusedVnodes; + // Write back the modified pages of some unused vnodes and free them. - // Write back the modified pages of some unused vnodes and free them + for (uint32 i = 0; i < count; i++) { + ReadLocker vnodesReadLocker(sVnodeLock); - for (uint32 i = 0; i < count; i++) { - mutex_lock(&sVnodeMutex); - struct vnode* vnode = (struct vnode*)list_remove_head_item( + // get the first node + MutexLocker unusedVnodesLocker(sUnusedVnodesLock); + struct vnode* vnode = (struct vnode*)list_get_first_item( &sUnusedVnodeList); - if (vnode == NULL) { - mutex_unlock(&sVnodeMutex); + unusedVnodesLocker.Unlock(); + + if (vnode == NULL) break; - } - inc_vnode_ref_count(vnode); + // lock the node + AutoLocker<Vnode> nodeLocker(vnode); + + // check whether the node is still unused + unusedVnodesLocker.Lock(); + if (vnode != list_get_first_item(&sUnusedVnodeList)) + continue; + + // remove it + list_remove_head_item(&sUnusedVnodeList); sUnusedVnodes--; + unusedVnodesLocker.Unlock(); - mutex_unlock(&sVnodeMutex); + // grab a reference + inc_vnode_ref_count(vnode); + // write back changes and free the node + nodeLocker.Unlock(); + vnodesReadLocker.Unlock(); + if (vnode->cache != NULL) vnode->cache->WriteModified(); @@ -1297,12 +1356,14 @@ static struct advisory_locking* get_advisory_locking(struct vnode* vnode) { - mutex_lock(&sVnodeMutex); + rw_lock_read_lock(&sVnodeLock); + vnode->Lock(); struct advisory_locking* locking = vnode->advisory_locking; sem_id lock = locking != NULL ? locking->lock : B_ERROR; - mutex_unlock(&sVnodeMutex); + vnode->Unlock(); + rw_lock_read_unlock(&sVnodeLock); if (lock >= 0) lock = acquire_sem(lock); @@ -1350,7 +1411,8 @@ } // set our newly created locking object - MutexLocker _(sVnodeMutex); + ReadLocker _(sVnodeLock); + AutoLocker<Vnode> nodeLocker(vnode); if (vnode->advisory_locking == NULL) { vnode->advisory_locking = locking; lockingDeleter.Detach(); @@ -1490,11 +1552,13 @@ // longer used locking = get_advisory_locking(vnode); if (locking != NULL) { - MutexLocker locker(sVnodeMutex); + ReadLocker locker(sVnodeLock); + AutoLocker<Vnode> nodeLocker(vnode); // the locking could have been changed in the mean time if (locking->locks.IsEmpty()) { vnode->advisory_locking = NULL; + nodeLocker.Unlock(); locker.Unlock(); // we've detached the locking from the vnode, so we can @@ -1504,6 +1568,7 @@ delete locking; } else { // the locking is in use again + nodeLocker.Unlock(); locker.Unlock(); release_sem_etc(locking->lock, 1, B_DO_NOT_RESCHEDULE); } @@ -2007,6 +2072,10 @@ } +/*! Looks up the entry with name \a name in the directory represented by \a dir + and returns the respective vnode. + On success a reference to the vnode is acquired for the caller. +*/ static status_t lookup_dir_entry(struct vnode* dir, const char* name, struct vnode** _vnode) { @@ -2019,9 +2088,11 @@ if (status != B_OK) return status; - mutex_lock(&sVnodeMutex); + // The lookup() hook call get_vnode() or publish_vnode(), so we do already + // have a reference and just need to look the node up. + rw_lock_read_lock(&sVnodeLock); *_vnode = lookup_vnode(dir->device, id); - mutex_unlock(&sVnodeMutex); + rw_lock_read_unlock(&sVnodeLock); if (*_vnode == NULL) { panic("lookup_dir_entry(): could not lookup vnode (mountid 0x%lx vnid " @@ -2946,8 +3017,8 @@ kprintf(" mount: %p\n", vnode->mount); kprintf(" covered_by: %p\n", vnode->covered_by); kprintf(" cache: %p\n", vnode->cache); - kprintf(" flags: %s%s%s\n", vnode->remove ? "r" : "-", - vnode->busy ? "b" : "-", vnode->unpublished ? "u" : "-"); + kprintf(" flags: %s%s%s\n", vnode->IsRemoved() ? "r" : "-", + vnode->IsBusy() ? "b" : "-", vnode->IsUnpublished() ? "u" : "-"); kprintf(" advisory_lock: %p\n", vnode->advisory_locking); _dump_advisory_locking(vnode->advisory_locking); @@ -3079,8 +3150,8 @@ kprintf("%p%4ld%10Ld%5ld %p %p %p %s%s%s\n", vnode, vnode->device, vnode->id, vnode->ref_count, vnode->cache, vnode->private_node, - vnode->advisory_locking, vnode->remove ? "r" : "-", - vnode->busy ? "b" : "-", vnode->unpublished ? "u" : "-"); + vnode->advisory_locking, vnode->IsRemoved() ? "r" : "-", + vnode->IsBusy() ? "b" : "-", vnode->IsUnpublished() ? "u" : "-"); } [... truncated: 570 lines follow ...]