[haiku-commits] r36707 - haiku/trunk/src/add-ons/kernel/file_systems/bfs

  • From: axeld@xxxxxxxxxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 7 May 2010 10:36:31 +0200 (CEST)

Author: axeld
Date: 2010-05-07 10:36:31 +0200 (Fri, 07 May 2010)
New Revision: 36707
Changeset: http://dev.haiku-os.org/changeset/36707/haiku
Ticket: http://dev.haiku-os.org/ticket/5878

Modified:
   haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp
Log:
* No longer keep the write lock when accessing the attribute data; instead, we
  now make sure no one else touched it in the mean time (in order to retrieve
  consistent data for the index update). This should fix bug #5878.
* Squashed a TODO by locking the inode in the transaction as well.
* Fixed a quasi bug that would not have read the attribute data if there was no
  live query for it; "fortunately", Volume::CheckForLiveQuery() is not
  implemented yet, and always returns true.


Modified: haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp
===================================================================
--- haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp   2010-05-07 
07:56:26 UTC (rev 36706)
+++ haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp   2010-05-07 
08:36:31 UTC (rev 36707)
@@ -1077,7 +1077,7 @@
        // might happen.
 
        Index index(fVolume);
-       index.SetTo(name);
+       bool hasIndex = index.SetTo(name) == B_OK;
 
        Inode* attribute = NULL;
        status_t status = B_OK;
@@ -1131,48 +1131,60 @@
        }
 
        if (attribute != NULL) {
-               // TODO: we need to lock the inode in the transaction, see 
WriteAt()!
-               if (rw_lock_write_lock(&attribute->fLock) == B_OK) {
+               WriteLocker writeLocker(attribute->fLock);
+
+               if (hasIndex || fVolume->CheckForLiveQuery(name)) {
                        // Save the old attribute data (if this fails, 
oldLength will
                        // reflect it)
-                       if (fVolume->CheckForLiveQuery(name) && 
attribute->Size() > 0) {
+                       while (attribute->Size() > 0) {
+                               bigtime_t oldModified = 
attribute->LastModified();
+                               writeLocker.Unlock();
+
                                oldLength = BPLUSTREE_MAX_KEY_LENGTH;
                                if (attribute->ReadAt(0, oldBuffer, &oldLength) 
== B_OK)
                                        oldData = oldBuffer;
-                       }
 
-                       // check if the data fits into the small_data section 
again
-                       NodeGetter node(fVolume, transaction, this);
-                       status = _AddSmallData(transaction, node, name, type, 
pos, buffer,
-                               *_length);
+                               writeLocker.Lock();
 
-                       if (status == B_OK) {
-                               // it does - remove its file
-                               rw_lock_write_unlock(&attribute->fLock);
-                               status = _RemoveAttribute(transaction, name, 
false, NULL);
-                       } else {
-                               // The attribute type might have been changed - 
we need to
-                               // adopt the new one
-                               attribute->Node().type = 
HOST_ENDIAN_TO_BFS_INT32(type);
-                               status = attribute->WriteBack(transaction);
-                               rw_lock_write_unlock(&attribute->fLock);
+                               // Read until the data hasn't changed in between
+                               if (oldModified == attribute->LastModified())
+                                       break;
 
-                               if (status == B_OK) {
-                                       status = 
attribute->WriteAt(transaction, pos, buffer,
-                                               _length);
-                               }
+                               oldLength = 0;
                        }
+               }
 
+               // check if the data fits into the small_data section again
+               NodeGetter node(fVolume, transaction, this);
+               status = _AddSmallData(transaction, node, name, type, pos, 
buffer,
+                       *_length);
+
+               if (status == B_OK) {
+                       // it does - remove its file
+                       writeLocker.Unlock();
+                       status = _RemoveAttribute(transaction, name, false, 
NULL);
+               } else {
+                       // The attribute type might have been changed - we need 
to
+                       // adopt the new one
+                       attribute->Node().type = HOST_ENDIAN_TO_BFS_INT32(type);
+                       status = attribute->WriteBack(transaction);
+                       writeLocker.Unlock();
+
                        if (status == B_OK) {
-                               // Update status time on attribute write
-                               Node().status_change_time = 
HOST_ENDIAN_TO_BFS_INT64(
-                                       
bfs_inode::ToInode(real_time_clock_usecs()));
-
-                               status = WriteBack(transaction);
+                               status = attribute->WriteAt(transaction, pos, 
buffer,
+                                       _length);
                        }
-               } else
-                       status = B_ERROR;
+               }
 
+               if (status == B_OK) {
+                       // Update status time on attribute write
+                       Node().status_change_time = HOST_ENDIAN_TO_BFS_INT64(
+                               bfs_inode::ToInode(real_time_clock_usecs()));
+
+                       status = WriteBack(transaction);
+               }
+
+               attribute->WriteLockInTransaction(transaction);
                ReleaseAttribute(attribute);
        }
 
@@ -2629,7 +2641,7 @@
        } else if (parent != NULL && (mode & S_ATTR_DIR) == 0) {
                return B_BAD_VALUE;
        } else if ((openMode & O_DIRECTORY) != 0) {
-               // TODO: we might need to return B_NOT_A_DIRECTORY here 
+               // TODO: we might need to return B_NOT_A_DIRECTORY here
                return B_ENTRY_NOT_FOUND;
        }
 


Other related posts:

  • » [haiku-commits] r36707 - haiku/trunk/src/add-ons/kernel/file_systems/bfs - axeld