Added new RecursiveLock and RecursiveLocker classes.

Added file system global locking around creating, deleting, and renaming
inodes - that's not nice, but since we have only sequential journal access,
it won't hurt concurrency much. The locks are needed to prevent certain
things from happening like two threads could theoretically create the same
file twice (one of them would have failed, but relatively late in the process).
Anyway, a better locking scheme will be implemented at a later date, but maybe
not for R1.
Small cleanup.


git-svn-id: file:///srv/svn/repos/haiku/trunk/current@3562 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/file_systems/bfs/Inode.cpp

@@ -1788,6 +1788,8 @@ Inode::Remove(Transaction *transaction, const char *name, off_t *_id, bool isDir
 	if (GetTree(&tree) != B_OK)
 		RETURN_ERROR(B_BAD_VALUE);
 
+	RecursiveLocker locker(fVolume->Lock());
+
 	// does the file even exists?
 	off_t id;
 	if (tree->Find((uint8 *)name, (uint16)strlen(name), &id) < B_OK)
@@ -1880,6 +1882,10 @@ Inode::Create(Transaction *transaction, Inode *parent, const char *name, int32 m
 	Volume *volume = transaction->GetVolume();
 	BPlusTree *tree = NULL;
 
+	RecursiveLocker locker(volume->Lock());
+		// ToDo: it would be nicer to only lock the parent directory, if possible
+		//	(but that lock will already be held during any B+tree action)
+
 	if (parent && (mode & S_ATTR_DIR) == 0 && parent->IsContainer()) {
 		// check if the file already exists in the directory
 		if (parent->GetTree(&tree) != B_OK)

src/add-ons/kernel/file_systems/bfs/Lock.h

@@ -15,11 +15,9 @@
 
 
 // Configure here if and when real benaphores should be used
-// Benaphores doesn't make much sense in the kernel, so they are only
-// enabled for the user-space test application
+#define USE_BENAPHORE
+	// if defined, benaphores are used for the Semaphore/RecursiveLock classes
 #ifdef USER
-#	define USE_BENAPHORE
-	// if defined, benaphores are used for the Semaphore class
 //#	define FAST_LOCK
 	// the ReadWriteLock class uses a second Semaphore to
 	// speed up locking - only makes sense if USE_BENAPHORE
@@ -85,7 +83,7 @@ class Semaphore {
 #endif
 };
 
-// a convenience class to lock the benaphore
+// a convenience class to lock a Semaphore object
 
 class Locker {
 	public:
@@ -113,6 +111,103 @@ class Locker {
 };
 
 
+//**** Recursive Lock
+
+class RecursiveLock {
+	public:
+		RecursiveLock(const char *name)
+			:
+#ifdef USE_BENAPHORE
+			fSemaphore(create_sem(0, name)),
+			fCount(1),
+#else
+			fSemaphore(create_sem(1, name)),
+#endif
+			fOwner(-1)
+		{
+#ifndef USER
+			set_sem_owner(fSemaphore, B_SYSTEM_TEAM);
+#endif
+		}
+
+		status_t Lock()
+		{
+			thread_id thread = find_thread(NULL);
+			if (thread == fOwner) {
+				fOwnerCount++;
+				return B_OK;
+			}
+
+			status_t status;
+#ifdef USE_BENAPHORE
+			if (atomic_add(&fCount, -1) > 0)
+				status = B_OK;
+			else
+#endif
+				status = acquire_sem(fSemaphore);
+
+			if (status == B_OK) {
+				fOwner = thread;
+				fOwnerCount = 1;
+			}
+
+			return status;
+		}
+
+		status_t Unlock()
+		{
+			thread_id thread = find_thread(NULL);
+			if (thread != fOwner)
+				panic("RecursiveLock unlocked by %ld, owned by %ld\n", thread, fOwner);
+
+			if (--fOwnerCount == 0) {
+				fOwner = -1;
+#ifdef USE_BENAPHORE
+				if (atomic_add(&fCount, 1) < 0)
+#endif
+					return release_sem(fSemaphore);
+			}
+
+			return B_OK;
+		}
+
+	private:
+		sem_id	fSemaphore;
+#ifdef USE_BENAPHORE
+		vint32	fCount;
+#endif
+		thread_id	fOwner;
+		int32		fOwnerCount;
+};
+
+// a convenience class to lock an RecursiveLock object
+
+class RecursiveLocker {
+	public:
+		RecursiveLocker(RecursiveLock &lock)
+			: fLock(lock)
+		{
+			fStatus = lock.Lock();
+			ASSERT(fStatus == B_OK);
+		}
+
+		~RecursiveLocker()
+		{
+			if (fStatus == B_OK)
+				fLock.Unlock();
+		}
+
+		status_t Status() const
+		{
+			return fStatus;
+		}
+
+	private:
+		RecursiveLock	&fLock;
+		status_t		fStatus;
+};
+
+
 //**** Many Reader/Single Writer Lock
 
 // This is a "fast" implementation of a single writer/many reader
@@ -144,7 +239,7 @@ class Locker {
 // have to be called when there is no one waiting.
 // The disadvantage is the use of 2 real semaphores which is quite
 // expensive regarding that BeOS only allows for a total of 64k
-// semaphores.
+// semaphores (since every open BFS inode has such a lock).
 
 #ifdef FAST_LOCK
 class ReadWriteLock {
@@ -211,7 +306,7 @@ class ReadWriteLock {
 				// Acquire sem for all readers currently not using a semaphore.
 				// But if we are not the only write lock in the queue, just get
 				// the one for us
-				status = acquire_sem_etc(fSemaphore,readers <= 0 ? 1 : MAX_READERS - readers,0,0);
+				status = acquire_sem_etc(fSemaphore, readers <= 0 ? 1 : MAX_READERS - readers, 0, 0);
 			}
 			fWriteLock.Unlock();
 
@@ -220,10 +315,10 @@ class ReadWriteLock {
 		
 		void UnlockWrite()
 		{
-			int32 readers = atomic_add(&fCount,MAX_READERS);
+			int32 readers = atomic_add(&fCount, MAX_READERS);
 			if (readers < 0) {
 				// release sem for all readers only when we were the only writer
-				release_sem_etc(fSemaphore,readers <= -MAX_READERS ? 1 : -readers,0);
+				release_sem_etc(fSemaphore, readers <= -MAX_READERS ? 1 : -readers, 0);
 			}
 		}
 
@@ -281,12 +376,12 @@ class ReadWriteLock {
 		
 		status_t LockWrite()
 		{
-			return acquire_sem_etc(fSemaphore,MAX_READERS,0,0);
+			return acquire_sem_etc(fSemaphore, MAX_READERS, 0, 0);
 		}
 		
 		void UnlockWrite()
 		{
-			release_sem_etc(fSemaphore,MAX_READERS,0);
+			release_sem_etc(fSemaphore, MAX_READERS, 0);
 		}
 
 	private:

src/add-ons/kernel/file_systems/bfs/ToDo

@@ -49,6 +49,7 @@ Inode
  - exchange Inode::OldLastModified() with Inode::NewLastModified(), and don't change the last_modified field directly in Inode::WriteAt() for consistency in case of a crash
  - the size is only updated in bfs_close() - but if the system crashes before, the entry in the size index doesn't match the one in the inode anymore - it would be better to let the data.size not reflect the real file size in this case (since the max_xxx_range entries are always correct)
  - Inode::FillGapWithZeros() currently disabled; apart from being slow, it really shouldn't be executed while a transaction is running, because that stops all other threads from doing anything (which can be a long time for a 100 MB file)
+ - need better locking mechanism in combination with B+trees etc.!
 
 
 Indices

src/add-ons/kernel/file_systems/bfs/Volume.h

@@ -43,7 +43,7 @@ class Volume {
 		bool				IsValidSuperBlock();
 		bool				IsReadOnly() const;
 		void				Panic();
-		Semaphore			&Lock();
+		RecursiveLock		&Lock();
 
 		block_run			Root() const { return fSuperBlock.root_dir; }
 		Inode				*RootNode() const { return fRootNode; }
@@ -115,7 +115,7 @@ class Volume {
 		int					fDevice;
 		disk_super_block	fSuperBlock;
 		BlockAllocator		fBlockAllocator;
-		Semaphore			fLock;
+		RecursiveLock		fLock;
 		Journal				*fJournal;
 		vint32				fLogStart, fLogEnd;
 
@@ -143,7 +143,7 @@ Volume::IsReadOnly() const
 }
 
 
-inline Semaphore &
+inline RecursiveLock &
 Volume::Lock()
 {
 	 return fLock;

src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp

@@ -274,6 +274,8 @@ bfs_read_fs_stat(void *_ns, struct fs_info *info)
 
 	Volume *volume = (Volume *)_ns;
 
+	RecursiveLocker locker(volume->Lock());
+
 	// File system flags.
 	info->flags = B_FS_IS_PERSISTENT | B_FS_HAS_ATTR | B_FS_HAS_MIME | B_FS_HAS_QUERY |
 			(volume->IsReadOnly() ? B_FS_IS_READONLY : 0);
@@ -302,7 +304,7 @@ bfs_write_fs_stat(void *_ns, struct fs_info *info, long mask)
 	Volume *volume = (Volume *)_ns;
 	disk_super_block &superBlock = volume->SuperBlock();
 
-	Locker locker(volume->Lock());
+	RecursiveLocker locker(volume->Lock());
 
 	status_t status = B_BAD_VALUE;
 
@@ -1012,6 +1014,8 @@ bfs_rename(void *_ns, void *_oldDir, const char *oldName, void *_newDir, const c
 	if (oldDirectory == newDirectory && !strcmp(oldName, newName))
 		return B_OK;
 
+	RecursiveLocker locker(volume->Lock());
+
 	// get the directory's tree, and a pointer to the inode which should be changed
 	BPlusTree *tree;
 	status_t status = oldDirectory->GetTree(&tree);