diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp b/src/add-ons/kernel/file_systems/bfs/Inode.cpp index 4664fbfc45..7e0ddd673b 100644 --- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp @@ -55,11 +55,13 @@ InodeAllocator::~InodeAllocator() status_t -InodeAllocator::New(block_run *parentRun, mode_t mode, block_run &run, Inode **_inode) +InodeAllocator::New(block_run *parentRun, mode_t mode, block_run &run, + Inode **_inode) { Volume *volume = fTransaction->GetVolume(); - status_t status = volume->AllocateForInode(*fTransaction, parentRun, mode, fRun); + status_t status = volume->AllocateForInode(*fTransaction, parentRun, mode, + fRun); if (status < B_OK) { // don't free the space in the destructor, because // the allocation failed @@ -101,7 +103,8 @@ InodeAllocator::CreateTree() if (fInode->IsRegularNode()) { if (tree->Insert(*fTransaction, ".", fInode->ID()) < B_OK - || tree->Insert(*fTransaction, "..", volume->ToVnode(fInode->Parent())) < B_OK) + || tree->Insert(*fTransaction, "..", + volume->ToVnode(fInode->Parent())) < B_OK) return B_ERROR; } return B_OK; @@ -833,11 +836,7 @@ Inode::ReadAttribute(const char *name, int32 type, off_t pos, uint8 *buffer, Inode *attribute; status_t status = GetAttribute(name, &attribute); if (status == B_OK) { - if (attribute->Lock().Lock() == B_OK) { - status = attribute->ReadAt(pos, (uint8 *)buffer, _length); - attribute->Lock().Unlock(); - } else - status = B_ERROR; + status = attribute->ReadAt(pos, (uint8 *)buffer, _length); ReleaseAttribute(attribute); } @@ -862,6 +861,8 @@ Inode::WriteAttribute(Transaction &transaction, const char *name, int32 type, // TODO: we actually depend on that the contents of "buffer" are constant. // If they get changed during the write (hey, user programs), we may mess // up our index trees! + // TODO: for attribute files, we need to log the first BPLUSTREE_MAX_KEY_LENGTH + // bytes of the data stream, or the same as above might happen. Index index(fVolume); index.SetTo(name); @@ -916,19 +917,24 @@ Inode::WriteAttribute(Transaction &transaction, const char *name, int32 type, // check if the data fits into the small_data section again NodeGetter node(fVolume, transaction, this); - status = _AddSmallData(transaction, node, name, type, buffer, *_length); + status = _AddSmallData(transaction, node, name, type, buffer, + *_length); + if (status == B_OK) { // it does - remove its file + attribute->Lock().UnlockWrite(); status = _RemoveAttribute(transaction, name, false, NULL); } else { - status = attribute->WriteAt(transaction, pos, buffer, _length); - if (status == B_OK) { - // The attribute type might have been changed - we need to adopt - // the new one - the attribute's inode will be written back on close - attribute->Node().type = HOST_ENDIAN_TO_BFS_INT32(type); - } - + // 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); attribute->Lock().UnlockWrite(); + + if (status == B_OK) { + status = attribute->WriteAt(transaction, pos, buffer, + _length); + } } } else status = B_ERROR; @@ -976,8 +982,8 @@ Inode::RemoveAttribute(Transaction &transaction, const char *name) uint32 length = smallData->DataSize(); if (length > BPLUSTREE_MAX_KEY_LENGTH) length = BPLUSTREE_MAX_KEY_LENGTH; - index.Update(transaction, name, smallData->Type(), smallData->Data(), - length, NULL, 0, this); + index.Update(transaction, name, smallData->Type(), + smallData->Data(), length, NULL, 0, this); } fSmallDataLock.Unlock(); } @@ -1224,14 +1230,6 @@ Inode::FindBlockRun(off_t pos, block_run &run, off_t &offset) status_t Inode::ReadAt(off_t pos, uint8 *buffer, size_t *_length) { -#if 0 - // call the right ReadAt() method, depending on the inode flags - - if (Flags() & INODE_LOGGED) - return ((Stream *)this)->ReadAt(pos, buffer, _length); - - return ((Stream *)this)->ReadAt(pos, buffer, _length); -#endif size_t length = *_length; // set/check boundaries for pos/length @@ -1266,27 +1264,28 @@ Inode::WriteAt(Transaction &transaction, off_t pos, const uint8 *buffer, << INODE_TIME_SHIFT); // TODO: support INODE_LOGGED! -#if 0 - if (Flags() & INODE_LOGGED) - return ((Stream *)this)->WriteAt(transaction, pos, buffer, _length); - return ((Stream *)this)->WriteAt(transaction, pos, buffer, _length); -#endif size_t length = *_length; + bool changeSize = false; // set/check boundaries for pos/length if (pos < 0) return B_BAD_VALUE; + if (pos + length > Size()) + changeSize = true; + + locker.Unlock(); + + // the transaction doesn't have to be started already + if (changeSize && !transaction.IsStarted()) + transaction.Start(fVolume, BlockNumber()); + + locker.Lock(); + if (pos + length > Size()) { off_t oldSize = Size(); - // the transaction doesn't have to be started already - // ToDo: what's that INODE_NO_TRANSACTION flag good for again? - if ((Flags() & INODE_NO_TRANSACTION) == 0 - && !transaction.IsStarted()) - transaction.Start(fVolume, BlockNumber()); - // let's grow the data stream to the size needed status_t status = SetFileSize(transaction, pos + length); if (status < B_OK) { diff --git a/src/add-ons/kernel/file_systems/bfs/Lock.h b/src/add-ons/kernel/file_systems/bfs/Lock.h index bf65640b99..7626bbbe92 100644 --- a/src/add-ons/kernel/file_systems/bfs/Lock.h +++ b/src/add-ons/kernel/file_systems/bfs/Lock.h @@ -479,7 +479,7 @@ class WriteLocked { : fLock(lock) { - fStatus = lock != NULL ? lock->LockWrite() : B_ERROR; + fStatus = lock != NULL ? lock->LockWrite() : B_NO_INIT; } ~WriteLocked() @@ -488,6 +488,14 @@ class WriteLocked { fLock->UnlockWrite(); } + status_t + Lock() + { + if (fStatus == B_OK) + return B_ERROR; + return fStatus = fLock->LockWrite(); + } + status_t IsLocked() { @@ -497,8 +505,9 @@ class WriteLocked { void Unlock() { - fLock->UnlockWrite(); - fStatus = B_NO_INIT; + if (fStatus == B_OK) + fLock->UnlockWrite(); + fStatus = B_ERROR; } private: diff --git a/src/add-ons/kernel/file_systems/bfs/bfs.h b/src/add-ons/kernel/file_systems/bfs/bfs.h index 23e2d95823..b7a7c2990f 100644 --- a/src/add-ons/kernel/file_systems/bfs/bfs.h +++ b/src/add-ons/kernel/file_systems/bfs/bfs.h @@ -217,7 +217,6 @@ enum inode_flags { INODE_PERMANENT_FLAGS = 0x0000ffff, INODE_WAS_WRITTEN = 0x00020000, - INODE_NO_TRANSACTION = 0x00040000, INODE_DONT_FREE_SPACE = 0x00080000, // only used by the "chkbfs" functionality INODE_CHKBFS_RUNNING = 0x00200000, }; diff --git a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp index 125eb4ca83..ff7f4df2fa 100644 --- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp @@ -694,20 +694,19 @@ bfs_write_stat(void *_ns, void *_node, const struct stat *stat, uint32 mask) Volume *volume = (Volume *)_ns; Inode *inode = (Inode *)_node; - // that may be incorrect here - I don't think we need write access to - // change most of the stat... - // we should definitely check a bit more if the new stats are correct and valid... + // TODO: we should definitely check a bit more if the new stats are + // valid - or even better, the VFS should check this before calling us status_t status = inode->CheckPermissions(W_OK); if (status < B_OK) RETURN_ERROR(status); + Transaction transaction(volume, inode->BlockNumber()); + WriteLocked locked(inode->Lock()); if (locked.IsLocked() < B_OK) RETURN_ERROR(B_ERROR); - Transaction transaction(volume, inode->BlockNumber()); - bfs_inode &node = inode->Node(); if (mask & B_STAT_SIZE) { @@ -737,7 +736,8 @@ bfs_write_stat(void *_ns, void *_node, const struct stat *stat, uint32 mask) if (mask & B_STAT_MODE) { PRINT(("original mode = %ld, stat->st_mode = %d\n", node.Mode(), stat->st_mode)); - node.mode = HOST_ENDIAN_TO_BFS_INT32(node.Mode() & ~S_IUMSK | stat->st_mode & S_IUMSK); + node.mode = HOST_ENDIAN_TO_BFS_INT32(node.Mode() & ~S_IUMSK + | stat->st_mode & S_IUMSK); } if (mask & B_STAT_UID) @@ -762,7 +762,8 @@ bfs_write_stat(void *_ns, void *_node, const struct stat *stat, uint32 mask) (bigtime_t)stat->st_crtime << INODE_TIME_SHIFT); } - if ((status = inode->WriteBack(transaction)) == B_OK) + status = inode->WriteBack(transaction); + if (status == B_OK) transaction.Done(); notify_stat_changed(volume->ID(), inode->ID(), mask); @@ -772,8 +773,8 @@ bfs_write_stat(void *_ns, void *_node, const struct stat *stat, uint32 mask) status_t -bfs_create(void *_ns, void *_directory, const char *name, int openMode, int mode, - void **_cookie, ino_t *_vnodeID) +bfs_create(void *_ns, void *_directory, const char *name, int openMode, + int mode, void **_cookie, ino_t *_vnodeID) { FUNCTION_START(("name = \"%s\", perms = %d, openMode = %d\n", name, mode, openMode)); @@ -1147,8 +1148,8 @@ bfs_open(void *_fs, void *_node, int openMode, void **_cookie) // Should we truncate the file? if (openMode & O_TRUNC) { - WriteLocked locked(inode->Lock()); Transaction transaction(volume, inode->BlockNumber()); + WriteLocked locked(inode->Lock()); status_t status = inode->SetFileSize(transaction, 0); if (status >= B_OK) @@ -1255,25 +1256,32 @@ bfs_free_cookie(void *_ns, void *_node, void *_cookie) return B_BAD_VALUE; file_cookie *cookie = (file_cookie *)_cookie; - Volume *volume = (Volume *)_ns; Inode *inode = (Inode *)_node; - WriteLocked locked(inode->Lock()); + Transaction transaction; + bool needsTrimming; - bool needsTrimming = inode->NeedsTrimming(); + { + ReadLocked locker(inode->Lock()); + needsTrimming = inode->NeedsTrimming(); - if ((cookie->open_mode & O_RWMASK) != 0 - && !inode->IsDeleted() - && (needsTrimming - || inode->OldLastModified() != inode->LastModified() - || inode->OldSize() != inode->Size())) { + if ((cookie->open_mode & O_RWMASK) != 0 + && !inode->IsDeleted() + && (needsTrimming + || inode->OldLastModified() != inode->LastModified() + || inode->OldSize() != inode->Size())) { + locker.Unlock(); + transaction.Start(volume, inode->BlockNumber()); + } + } + WriteLocked locker(inode->Lock()); + status_t status = B_ERROR; + + if (transaction.IsStarted()) { // trim the preallocated blocks and update the size, // and last_modified indices if needed - - Transaction transaction(volume, inode->BlockNumber()); - status_t status = B_OK; bool changedSize = false, changedTime = false; Index index(volume); @@ -1299,14 +1307,14 @@ bfs_free_cookie(void *_ns, void *_node, void *_cookie) inode->WriteBack(transaction); } - if (status == B_OK) - transaction.Done(); - if (changedSize || changedTime) { notify_stat_changed(volume->ID(), inode->ID(), - (changedTime ? B_STAT_MODIFICATION_TIME : 0) | (changedSize ? B_STAT_SIZE : 0)); + (changedTime ? B_STAT_MODIFICATION_TIME : 0) + | (changedSize ? B_STAT_SIZE : 0)); } } + if (status == B_OK) + transaction.Done(); if (inode->Flags() & INODE_CHKBFS_RUNNING) { // "chkbfs" exited abnormally, so we have to stop it here...