From 42e3c6f97874f37701385e7027c77e4366d7c450 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Wed, 10 Jul 2019 22:49:42 -0400 Subject: [PATCH] KPath: Use an object_cache for the path buffers in the normal case. This was (following the packagefs changes) the number-one (by call count) consumer of malloc() during the boot -- 52866 calls, and 100% of them either 1024 or 1025 bytes! Virtually all of these are ephemeral (indeed, the object_cache stats after a boot with this patch shows there is only a single slab of 64 buffers allocated, and most of them unused), so this is probably a significant performance boost. Change-Id: I659f5707510cbfeafa735d35eea7b92732ead666 --- headers/private/kernel/fs/KPath.h | 7 +- .../KDiskDeviceManager.cpp | 2 +- src/system/kernel/fs/KPath.cpp | 48 ++++++-- src/system/kernel/fs/vfs.cpp | 103 ++++++++++-------- 4 files changed, 100 insertions(+), 60 deletions(-) diff --git a/headers/private/kernel/fs/KPath.h b/headers/private/kernel/fs/KPath.h index 1298f81007..372b02fc2d 100644 --- a/headers/private/kernel/fs/KPath.h +++ b/headers/private/kernel/fs/KPath.h @@ -23,14 +23,14 @@ public: LAZY_ALLOC = 0x04 }; public: - KPath(size_t bufferSize = B_PATH_NAME_LENGTH); + KPath(size_t bufferSize = B_PATH_NAME_LENGTH + 1); KPath(const char* path, int32 flags = DEFAULT, - size_t bufferSize = B_PATH_NAME_LENGTH); + size_t bufferSize = B_PATH_NAME_LENGTH + 1); KPath(const KPath& other); ~KPath(); status_t SetTo(const char* path, int32 flags = DEFAULT, - size_t bufferSize = B_PATH_NAME_LENGTH); + size_t bufferSize = B_PATH_NAME_LENGTH + 1); void Adopt(KPath& other); status_t InitCheck() const; @@ -67,6 +67,7 @@ public: private: status_t _AllocateBuffer(); + void _FreeBuffer(); status_t _Normalize(const char* path, bool traverseLeafLink); void _ChopTrailingSlashes(); diff --git a/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp b/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp index 01aa97cc89..5a7a91b7fe 100644 --- a/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp +++ b/src/system/kernel/disk_device_manager/KDiskDeviceManager.cpp @@ -197,7 +197,7 @@ public: if (strcmp(event->name, "raw") == 0) { // a new raw device was added/removed - KPath path(B_PATH_NAME_LENGTH + 1); + KPath path; if (path.InitCheck() != B_OK || vfs_entry_ref_to_path(event->device, event->directory, event->name, true, path.LockBuffer(), diff --git a/src/system/kernel/fs/KPath.cpp b/src/system/kernel/fs/KPath.cpp index e0a70bbc72..1693cdfb22 100644 --- a/src/system/kernel/fs/KPath.cpp +++ b/src/system/kernel/fs/KPath.cpp @@ -15,6 +15,7 @@ #include #include +#include // debugging @@ -22,6 +23,11 @@ //#define TRACE(x) dprintf x +#ifdef _KERNEL_MODE +extern object_cache* sPathNameCache; +#endif + + KPath::KPath(size_t bufferSize) : fBuffer(NULL), @@ -66,7 +72,7 @@ KPath::KPath(const KPath& other) KPath::~KPath() { - free(fBuffer); + _FreeBuffer(); } @@ -74,12 +80,11 @@ status_t KPath::SetTo(const char* path, int32 flags, size_t bufferSize) { if (bufferSize == 0) - bufferSize = B_PATH_NAME_LENGTH; + bufferSize = B_PATH_NAME_LENGTH + 1; // free the previous buffer, if the buffer size differs if (fBuffer != NULL && fBufferSize != bufferSize) { - free(fBuffer); - fBuffer = NULL; + _FreeBuffer(); fBufferSize = 0; } @@ -102,7 +107,7 @@ KPath::SetTo(const char* path, int32 flags, size_t bufferSize) void KPath::Adopt(KPath& other) { - free(fBuffer); + _FreeBuffer(); fBuffer = other.fBuffer; fBufferSize = other.fBufferSize; @@ -218,7 +223,9 @@ void KPath::UnlockBuffer() { if (!fLocked) { - TRACE(("KPath::UnlockBuffer(): ERROR: Buffer not locked!\n")); +#ifdef _KERNEL_MODE + panic("KPath::UnlockBuffer(): Buffer not locked!"); +#endif return; } @@ -242,6 +249,12 @@ KPath::DetachBuffer() { char* buffer = fBuffer; + if (fBufferSize == (B_PATH_NAME_LENGTH + 1)) { + buffer = (char*)malloc(fBufferSize); + memcpy(buffer, fBuffer, fBufferSize); + _FreeBuffer(); + } + if (fBuffer != NULL) { fBuffer = NULL; fPathLength = 0; @@ -412,8 +425,14 @@ KPath::operator!=(const char* path) const status_t KPath::_AllocateBuffer() { - if (fBuffer == NULL && fBufferSize != 0) - fBuffer = (char*)malloc(fBufferSize); + if (fBuffer == NULL && fBufferSize != 0) { +#ifdef _KERNEL_MODE + if (fBufferSize == (B_PATH_NAME_LENGTH + 1)) + fBuffer = (char*)object_cache_alloc(sPathNameCache, 0); + else +#endif + fBuffer = (char*)malloc(fBufferSize); + } if (fBuffer == NULL) { fFailed = true; return B_NO_MEMORY; @@ -425,6 +444,19 @@ KPath::_AllocateBuffer() } +void +KPath::_FreeBuffer() +{ +#ifdef _KERNEL_MODE + if (fBufferSize == (B_PATH_NAME_LENGTH + 1)) + object_cache_free(sPathNameCache, fBuffer, 0); + else +#endif + free(fBuffer); + fBuffer = NULL; +} + + status_t KPath::_Normalize(const char* path, bool traverseLeafLink) { diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp index 7ac6152b1c..fa49e81935 100644 --- a/src/system/kernel/fs/vfs.cpp +++ b/src/system/kernel/fs/vfs.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -108,6 +109,7 @@ #endif +object_cache* sPathNameCache; const static size_t kMaxPathLength = 65536; // The absolute maximum path length (for getcwd() - this is not depending // on PATH_MAX @@ -2233,7 +2235,8 @@ vnode_path_to_vnode(struct vnode* vnode, char* path, bool traverseLeafLink, goto resolve_link_error; } - buffer = (char*)malloc(bufferSize = B_PATH_NAME_LENGTH); + bufferSize = B_PATH_NAME_LENGTH; + buffer = (char*)object_cache_alloc(sPathNameCache, 0); if (buffer == NULL) { status = B_NO_MEMORY; goto resolve_link_error; @@ -2291,7 +2294,7 @@ vnode_path_to_vnode(struct vnode* vnode, char* path, bool traverseLeafLink, ioContext, &nextVnode, &lastParentID); } - free(buffer); + object_cache_free(sPathNameCache, buffer, 0); if (status != B_OK) { put_vnode(vnode); @@ -4249,7 +4252,7 @@ vfs_get_vnode_from_path(const char* path, bool kernel, struct vnode** _vnode) TRACE(("vfs_get_vnode_from_path: entry. path = '%s', kernel %d\n", path, kernel)); - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -4346,7 +4349,7 @@ vfs_get_fs_node_from_path(fs_volume* volume, const char* path, TRACE(("vfs_get_fs_node_from_path(volume = %p, path = \"%s\", kernel %d)\n", volume, path, kernel)); - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -4396,7 +4399,7 @@ vfs_read_stat(int fd, const char* path, bool traverseLeafLink, if (path != NULL) { // path given: get the stat of the node referred to by (fd, path) - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -4598,7 +4601,7 @@ vfs_create_special_node(const char* path, fs_vnode* subVnode, mode_t mode, if (path) { // We've got a path. Get the dir vnode and the leaf name. - KPath tmpPathBuffer(B_PATH_NAME_LENGTH + 1); + KPath tmpPathBuffer; if (tmpPathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -5341,6 +5344,11 @@ vfs_init(kernel_args* args) || sMountsTable->Init(MOUNTS_HASH_TABLE_SIZE) != B_OK) panic("vfs_init: error creating mounts hash table\n"); + sPathNameCache = create_object_cache("path names", + B_PATH_NAME_LENGTH + 1, 8, NULL, NULL, NULL); + if (sPathNameCache == NULL) + panic("vfs_init: error creating path name object_cache\n"); + node_monitor_init(); sRoot = NULL; @@ -8166,7 +8174,7 @@ dev_t _kern_mount(const char* path, const char* device, const char* fsName, uint32 flags, const char* args, size_t argsLength) { - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8177,7 +8185,7 @@ _kern_mount(const char* path, const char* device, const char* fsName, status_t _kern_unmount(const char* path, uint32 flags) { - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8305,7 +8313,7 @@ _kern_open_entry_ref(dev_t device, ino_t inode, const char* name, int openMode, int _kern_open(int fd, const char* path, int openMode, int perms) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8357,7 +8365,7 @@ _kern_open_dir_entry_ref(dev_t device, ino_t inode, const char* name) int _kern_open_dir(int fd, const char* path) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8418,7 +8426,7 @@ _kern_create_dir_entry_ref(dev_t device, ino_t inode, const char* name, status_t _kern_create_dir(int fd, const char* path, int perms) { - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::DEFAULT); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8429,7 +8437,7 @@ _kern_create_dir(int fd, const char* path, int perms) status_t _kern_remove_dir(int fd, const char* path) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8458,7 +8466,7 @@ _kern_remove_dir(int fd, const char* path) status_t _kern_read_link(int fd, const char* path, char* buffer, size_t* _bufferSize) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8484,7 +8492,7 @@ _kern_read_link(int fd, const char* path, char* buffer, size_t* _bufferSize) status_t _kern_create_symlink(int fd, const char* path, const char* toPath, int mode) { - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8497,8 +8505,8 @@ status_t _kern_create_link(int pathFD, const char* path, int toFD, const char* toPath, bool traverseLeafLink) { - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); - KPath toPathBuffer(toPath, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path); + KPath toPathBuffer(toPath); if (pathBuffer.InitCheck() != B_OK || toPathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8523,7 +8531,7 @@ _kern_create_link(int pathFD, const char* path, int toFD, const char* toPath, status_t _kern_unlink(int fd, const char* path) { - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8552,8 +8560,8 @@ _kern_unlink(int fd, const char* path) status_t _kern_rename(int oldFD, const char* oldPath, int newFD, const char* newPath) { - KPath oldPathBuffer(oldPath, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); - KPath newPathBuffer(newPath, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath oldPathBuffer(oldPath); + KPath newPathBuffer(newPath); if (oldPathBuffer.InitCheck() != B_OK || newPathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8565,7 +8573,7 @@ _kern_rename(int oldFD, const char* oldPath, int newFD, const char* newPath) status_t _kern_access(int fd, const char* path, int mode, bool effectiveUserGroup) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8661,7 +8669,7 @@ _kern_write_stat(int fd, const char* path, bool traverseLeafLink, if (path != NULL) { // path given: write the stat of the node referred to by (fd, path) - KPath pathBuffer(path, KPath::DEFAULT, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8689,7 +8697,7 @@ _kern_write_stat(int fd, const char* path, bool traverseLeafLink, int _kern_open_attr_dir(int fd, const char* path, bool traverseLeafLink) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8701,7 +8709,7 @@ int _kern_open_attr(int fd, const char* path, const char* name, uint32 type, int openMode) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8770,7 +8778,7 @@ _kern_getcwd(char* buffer, size_t size) status_t _kern_setcwd(int fd, const char* path) { - KPath pathBuffer(path, KPath::LAZY_ALLOC, B_PATH_NAME_LENGTH + 1); + KPath pathBuffer(path, KPath::LAZY_ALLOC); if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8854,11 +8862,10 @@ _user_mount(const char* userPath, const char* userDevice, status_t _user_unmount(const char* userPath, uint32 flags) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); - if (!IS_USER_ADDRESS(userPath)) return B_BAD_ADDRESS; + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -8977,7 +8984,7 @@ _user_entry_ref_to_path(dev_t device, ino_t inode, const char* leaf, if (!IS_USER_ADDRESS(userPath)) return B_BAD_ADDRESS; - KPath path(B_PATH_NAME_LENGTH + 1); + KPath path; if (path.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9020,7 +9027,7 @@ _user_normalize_path(const char* userPath, bool traverseLink, char* buffer) return B_BAD_ADDRESS; // copy path from userland - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; char* path = pathBuffer.LockBuffer(); @@ -9071,7 +9078,7 @@ _user_open_entry_ref(dev_t device, ino_t inode, const char* userName, int _user_open(int fd, const char* userPath, int openMode, int perms) { - KPath path(B_PATH_NAME_LENGTH + 1); + KPath path; if (path.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9114,7 +9121,7 @@ _user_open_dir(int fd, const char* userPath) if (userPath == NULL) return dir_open(fd, NULL, false); - KPath path(B_PATH_NAME_LENGTH + 1); + KPath path; if (path.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9299,7 +9306,7 @@ _user_create_dir_entry_ref(dev_t device, ino_t inode, const char* userName, status_t _user_create_dir(int fd, const char* userPath, int perms) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9318,7 +9325,7 @@ _user_create_dir(int fd, const char* userPath, int perms) status_t _user_remove_dir(int fd, const char* userPath) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9340,7 +9347,7 @@ status_t _user_read_link(int fd, const char* userPath, char* userBuffer, size_t* userBufferSize) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1), linkBuffer; + KPath pathBuffer, linkBuffer; if (pathBuffer.InitCheck() != B_OK || linkBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9386,8 +9393,8 @@ status_t _user_create_symlink(int fd, const char* userPath, const char* userToPath, int mode) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); - KPath toPathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; + KPath toPathBuffer; if (pathBuffer.InitCheck() != B_OK || toPathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9411,8 +9418,8 @@ status_t _user_create_link(int pathFD, const char* userPath, int toFD, const char* userToPath, bool traverseLeafLink) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); - KPath toPathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; + KPath toPathBuffer; if (pathBuffer.InitCheck() != B_OK || toPathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9440,7 +9447,7 @@ _user_create_link(int pathFD, const char* userPath, int toFD, status_t _user_unlink(int fd, const char* userPath) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9460,8 +9467,8 @@ status_t _user_rename(int oldFD, const char* userOldPath, int newFD, const char* userNewPath) { - KPath oldPathBuffer(B_PATH_NAME_LENGTH + 1); - KPath newPathBuffer(B_PATH_NAME_LENGTH + 1); + KPath oldPathBuffer; + KPath newPathBuffer; if (oldPathBuffer.InitCheck() != B_OK || newPathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9484,7 +9491,7 @@ _user_rename(int oldFD, const char* userOldPath, int newFD, status_t _user_create_fifo(int fd, const char* userPath, mode_t perms) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9580,7 +9587,7 @@ _user_create_pipe(int* userFDs) status_t _user_access(int fd, const char* userPath, int mode, bool effectiveUserGroup) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9614,7 +9621,7 @@ _user_read_stat(int fd, const char* userPath, bool traverseLink, if (!IS_USER_ADDRESS(userPath)) return B_BAD_ADDRESS; - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9671,7 +9678,7 @@ _user_write_stat(int fd, const char* userPath, bool traverseLeafLink, if (!IS_USER_ADDRESS(userPath)) return B_BAD_ADDRESS; - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9706,7 +9713,7 @@ _user_write_stat(int fd, const char* userPath, bool traverseLeafLink, int _user_open_attr_dir(int fd, const char* userPath, bool traverseLeafLink) { - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9837,7 +9844,7 @@ _user_open_attr(int fd, const char* userPath, const char* userName, if (status != B_OK) return status; - KPath pathBuffer(B_PATH_NAME_LENGTH + 1); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -9999,7 +10006,7 @@ _user_setcwd(int fd, const char* userPath) { TRACE(("user_setcwd: path = %p\n", userPath)); - KPath pathBuffer(B_PATH_NAME_LENGTH); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY; @@ -10025,7 +10032,7 @@ _user_change_root(const char* userPath) return B_NOT_ALLOWED; // alloc path buffer - KPath pathBuffer(B_PATH_NAME_LENGTH); + KPath pathBuffer; if (pathBuffer.InitCheck() != B_OK) return B_NO_MEMORY;