From e15a7ad3a398f37a44b9fa7207ed7e1922dc9840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Sat, 4 Dec 2004 01:21:12 +0000 Subject: [PATCH] Fixed a big allocation leak: preallocation was done for all streams, but only regular files were stripped back when bfs_close() was called. Now, we don't preallocate any blocks for attributes, attribute directories, and symbolic links (which do have a stream when longer than 144 bytes). Also, bfs_release_vnode() now trims back all streams that need to be trimmed - this catches standard directories, which could also slip through before. Removed the remaining blocks of INODE_NO_CACHE support as this is not needed for Haiku. Added method Inode::NeedsTrimming() that determines if its data stream can be trimmed (indices are never trimmed to reduce fragmentation). git-svn-id: file:///srv/svn/repos/haiku/trunk/current@10356 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/add-ons/kernel/file_systems/bfs/Inode.cpp | 50 +++++++++++-------- src/add-ons/kernel/file_systems/bfs/Inode.h | 8 +-- src/add-ons/kernel/file_systems/bfs/bfs.h | 1 - .../file_systems/bfs/kernel_interface.cpp | 31 +++++++----- 4 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp b/src/add-ons/kernel/file_systems/bfs/Inode.cpp index aae157f3c1..1ffe9e30d0 100644 --- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp @@ -1201,12 +1201,6 @@ Inode::WriteAt(Transaction &transaction, off_t pos, const uint8 *buffer, size_t if (pos + length > Size()) { off_t oldSize = Size(); - // uncached files can't be resized (Inode::SetFileSize() also - // doesn't allow this, but this way we don't have to start a - // transaction to find out). - if (Flags() & INODE_NO_CACHE) - return B_BAD_VALUE; - // 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 @@ -1388,8 +1382,13 @@ Inode::GrowStream(Transaction &transaction, off_t size) // blocks we need to allocate may be different from the one we request // from the block allocator - // should we preallocate some blocks (currently, always 64k)? - if (blocksRequested < (65536 >> fVolume->BlockShift()) && fVolume->FreeBlocks() > 128) + // Should we preallocate some blocks (currently, always 64k)? + // Attributes, attribute directories, and long symlinks usually won't get that big, + // and should stay close to the inode - preallocating could be counterproductive. + // Also, if free disk space is tight, we probably don't want to do this as well. + if (!IsAttribute() && !IsAttributeDirectory() && !IsSymLink() + && blocksRequested < (65536 >> fVolume->BlockShift()) + && fVolume->FreeBlocks() > 128) blocksRequested = 65536 >> fVolume->BlockShift(); while (blocksNeeded > 0) { @@ -1795,11 +1794,7 @@ Inode::ShrinkStream(Transaction &transaction, off_t size) status_t Inode::SetFileSize(Transaction &transaction, off_t size) { - if (size < 0 - // uncached files can't be resized (Stream::WriteAt() specifically - // denies growing uncached files because of efficiency, so it had to be - // adapted if this ever changes [which will probably happen in OpenBeOS]). - || Flags() & INODE_NO_CACHE) + if (size < 0) return B_BAD_VALUE; off_t oldSize = Size(); @@ -1835,8 +1830,29 @@ Inode::Append(Transaction &transaction, off_t bytes) } +/** Checks wether or not this inode's data stream needs to be trimmed + * because of an earlier preallocation. + * Returns true if there are any blocks to be trimmed. + */ + +bool +Inode::NeedsTrimming() +{ + // We never trim preallocated index blocks to make them grow as smooth as possible. + // There are only few indices anyway, so this doesn't hurt + if (IsIndex()) + return false; + + off_t roundedSize = (Size() + fVolume->BlockSize() - 1) >> fVolume->BlockShift(); + + return Node().data.MaxDirectRange() > roundedSize + || Node().data.MaxIndirectRange() > roundedSize + || Node().data.MaxDoubleIndirectRange() > roundedSize; +} + + status_t -Inode::Trim(Transaction &transaction) +Inode::TrimPreallocation(Transaction &transaction) { status_t status = ShrinkStream(transaction, Size()); if (status < B_OK) @@ -1994,12 +2010,6 @@ Inode::Remove(Transaction &transaction, const char *name, off_t *_id, bool isDir return B_ENTRY_NOT_FOUND; } - // You can't unlink a mounted image or the VM file while it is being used - while - // this is not really necessary, it copies the behaviour of the original BFS - // and let you and me feel a little bit safer - if (inode->Flags() & INODE_NO_CACHE) - return B_NOT_ALLOWED; - // Inode::IsContainer() is true also for indices (furthermore, the S_IFDIR // bit is set for indices in BFS, not for attribute directories) - but you // should really be able to do whatever you want with your indices diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.h b/src/add-ons/kernel/file_systems/bfs/Inode.h index b8dbffb310..b26e6c8bd0 100644 --- a/src/add-ons/kernel/file_systems/bfs/Inode.h +++ b/src/add-ons/kernel/file_systems/bfs/Inode.h @@ -66,8 +66,9 @@ class Inode { bool IsIndex() const { return (Mode() & (S_INDEX_DIR | 0777)) == S_INDEX_DIR; } // that's a stupid check, but AFAIK the only possible method... - bool IsAttribute() const { return Mode() & S_ATTR; } - bool IsFile() const { return Mode() & S_IFREG; } + bool IsAttributeDirectory() const { return (Mode() & S_ATTR_DIR) != 0; } + bool IsAttribute() const { return (Mode() & S_ATTR) != 0; } + bool IsFile() const { return S_ISREG(Mode()); } bool IsRegularNode() const { return (Mode() & (S_ATTR_DIR | S_INDEX_DIR | S_ATTR)) == 0; } // a regular node in the standard namespace (i.e. not an index or attribute) bool IsSymLink() const { return S_ISLNK(Mode()); } @@ -125,7 +126,8 @@ class Inode { status_t SetFileSize(Transaction &transaction, off_t size); status_t Append(Transaction &transaction, off_t bytes); - status_t Trim(Transaction &transaction); + status_t TrimPreallocation(Transaction &transaction); + bool NeedsTrimming(); status_t Free(Transaction &transaction); status_t Sync(); diff --git a/src/add-ons/kernel/file_systems/bfs/bfs.h b/src/add-ons/kernel/file_systems/bfs/bfs.h index 196757a9ef..b7d1c75e16 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_NO_CACHE = 0x00010000, INODE_WAS_WRITTEN = 0x00020000, INODE_NO_TRANSACTION = 0x00040000, INODE_DONT_FREE_SPACE = 0x00080000, // only used by the "chkbfs" functionality 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 f2bc562c7e..211881696f 100644 --- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp @@ -244,11 +244,22 @@ restartIfBusy: static status_t -bfs_release_vnode(void *ns, void *_node, bool reenter) +bfs_release_vnode(void *_ns, void *_node, bool reenter) { //FUNCTION_START(("node = %p\n", _node)); + + Volume *volume = (Volume *)_ns; Inode *inode = (Inode *)_node; + // since a directory's size can be changed without having it opened, + // we need to take care about their preallocated blocks here + if (inode->NeedsTrimming()) { + Transaction transaction(volume, inode->BlockNumber()); + + if (inode->TrimPreallocation(transaction) == B_OK) + transaction.Done(); + } + delete inode; return B_NO_ERROR; @@ -1049,10 +1060,6 @@ bfs_open(void *_ns, void *_node, int omode, void **_cookie) Volume *volume = (Volume *)_ns; Inode *inode = (Inode *)_node; - // we can't open a file which uses uncached access twice - if (inode->Flags() & INODE_NO_CACHE) - return B_BUSY; - // opening a directory read-only is allowed, although you can't read // any data from it. if (inode->IsDirectory() && omode & O_RWMASK) { @@ -1159,10 +1166,7 @@ bfs_write(void *_ns, void *_node, void *_cookie, off_t pos, const void *buffer, if (status == B_OK) transaction.Done(); - if (status == B_OK && (inode->Flags() & INODE_NO_CACHE) == 0) { - // uncached files don't cause notifications during access, and - // never want to write back any cached blocks - + if (status == B_OK) { // periodically notify if the file size has changed // ToDo: should we better test for a change in the last_modified time only? if (cookie->last_size != inode->Size() @@ -1205,7 +1209,10 @@ bfs_free_cookie(void *_ns, void *_node, void *_cookie) Volume *volume = (Volume *)_ns; Inode *inode = (Inode *)_node; - if (cookie->open_mode & O_RWMASK) { + bool needsTrimming = inode->NeedsTrimming(); + + if (cookie->open_mode & O_RWMASK + && (needsTrimming || inode->OldLastModified() != inode->LastModified())) { #ifdef UNSAFE_GET_VNODE RecursiveLocker locker(volume->Lock()); #endif @@ -1219,8 +1226,8 @@ bfs_free_cookie(void *_ns, void *_node, void *_cookie) bool changed = false; Index index(volume); - if (inode->OldSize() != inode->Size()) { - status = inode->Trim(transaction); + if (needsTrimming) { + status = inode->TrimPreallocation(transaction); if (status < B_OK) FATAL(("Could not trim preallocated blocks!"));