Changeset 25407

Timestamp:

05/09/08 19:53:13 (4 days ago)

Author:

bonefish

Message:

Advisory locking fixes:
* Made the access strategy to vnode::advisory_locking consistent.
  get_advisory_locking() was guarding it with sVnodeMutex,
  create_advisory_locking() was using atomic_pointer_test_and_set(), and
  release_advisory_lock() just set it unguardedly. We do use sVnodeMutex
  consequently, now.
* Beautified create_advisory_locking() (got rid of the gotos,
  reorganized the control flow).
* Fixed race conditions in acquire_advisory_lock(). It was always
  unlocking and relocking the advisory_locking object when it didn't
  have to wait, but in the meantime someone else could have changed the
  locking situation. Reorganized the control flow, so that it only drops
  the lock when it has to fail or wait. Using create_advisory_locking()
  upfront simplifies the code quite a bit (and fixes another race
  condition).

APR's testprocmutex test seems happy now, at least.

Files:

• haiku/trunk/src/system/kernel/fs/vfs.cpp (modified) (8 diffs)

haiku/trunk/src/system/kernel/fs/vfs.cpp

@@ -28 +28 @@
-#include <StorageDefs.h>
-
+#include <AutoDeleter.h>
-#include <block_cache.h>
-#include <boot/kernel_args.h>
@@ -152 +153 @@
-        sem_id                  wait_sem;
-        LockList                locks;
+
+        advisory_locking()
+                :
+                lock(-1),
+                wait_sem(-1)
+        {
+        }
+
+        ~advisory_locking()
+        {
+                if (lock >= 0)
+                        delete_sem(lock);
+                if (wait_sem >= 0)
+                        delete_sem(wait_sem);
+        }
-};
-
@@ -190 +206 @@
-/*!     \brief Guards sVnodeTable.
-
-to read/write access
+read/write access to
-        any unbusy vnode in that table, save to the immutable fields (device, id,
-        private_node, mount) to which
@@ -1123 +1139 @@
-                return B_FILE_ERROR;
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-                        return B_OK;
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
-}
-
@@ -1288 +1300 @@
-                locking = get_advisory_locking(vnode);
-                if (locking != NULL) {
+                        MutexLocker locker(sVnodeMutex);
+
-                        // the locking could have been changed in the mean time
-                        if (locking->locks.IsEmpty()) {
-                                vnode->advisory_locking = NULL;
+                                locker.Unlock();
-
-                                // we've detached the locking from the vnode, so we can
@@ -1299 +1314 @@
-                        } else {
-                                // the locking is in use again
+                                locker.Unlock();
-                                release_sem_etc(locking->lock, 1, B_DO_NOT_RESCHEDULE);
-                        }
@@ -1329 +1345 @@
-        // TODO: do deadlock detection!
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-                // test for collisions
-                LockList::Iterator iterator = locking->locks.GetIterator();
-                while (iterator.HasNext()) {
-                        struct advisory_lock *lock = iterator.Next();
-
+
-                        // TODO: locks from the same team might be joinable!
-                        if (lock->team != team && advisory_lock_intersects(lock, flock)) {
@@ -1353 +1375 @@
-                }
-
-
+
+
+
+
+
+
-                        put_advisory_locking(locking);
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
-
-        // install new lock
-
-        locking = get_advisory_locking(vnode);
-        if (locking == NULL) {
-                // we need to create a new locking object
-                status = create_advisory_locking(vnode);
-                if (status < B_OK)
-                        return status;
-
-                locking = vnode->advisory_locking;
-                        // we own the locking object, so it can't go away
-        }
-
-        struct advisory_lock *lock = (struct advisory_lock *)malloc(