[haiku-commits] r34865 - haiku/trunk/src/system/kernel/fs

  • From: ingo_weinhold@xxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 3 Jan 2010 00:27:41 +0100 (CET)

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 ...]

Other related posts:

  • » [haiku-commits] r34865 - haiku/trunk/src/system/kernel/fs - ingo_weinhold