From dbb33260cfcd83c0f90430eed54f1d449c53a764 Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Tue, 3 Jun 2025 08:01:20 +0200 Subject: [PATCH 1/2] Higher FD for logging --- CHANGELOG.md | 1 + src/client/logging.cpp | 57 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dcbf5d9e..ab1f98a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Directories shows . and .. to support scandir ([!248](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/248)) - updated fmt, spdlog and pytest versions ([!249](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/249)) - Updated Docker images for 0.9.5 ([!238](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/238)) + - Logging uses a higher FD limit ([!259](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/259)) ### Fixed - Dup3 is supported if O_CLOEXEC is not used (i.e. hexdump) ([!228](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/228)) diff --git a/src/client/logging.cpp b/src/client/logging.cpp index 60830432c..e44848926 100644 --- a/src/client/logging.cpp +++ b/src/client/logging.cpp @@ -43,6 +43,7 @@ #include #include #include +#include #ifdef GKFS_ENABLE_LOGGING @@ -336,8 +337,62 @@ logger::logger(const std::string& opts, const std::string& path, path); return; } + // Pseudo protect fd to a higher fd with dup2 + + struct rlimit rlim; + if(getrlimit(RLIMIT_NOFILE, &rlim) == -1) { + perror("GekkoFS Log: getrlimit(RLIMIT_NOFILE) failed"); + log_fd_ = fd; + fprintf(stderr, + "GekkoFS Log: Using original FD %d due to getrlimit error.\n", + log_fd_); + return; // Or error + } + + int target_fd = -1; + if(rlim.rlim_cur > 16) { // +3 for stdin/out/err + target_fd = rlim.rlim_cur - 16; + } else if(rlim.rlim_cur > 3) { // If limit is small, pick the highest + // possible that's not 0,1,2 + target_fd = rlim.rlim_cur - 1; + } else { + fprintf(stderr, + "GekkoFS Log: RLIMIT_NOFILE is too small (%llu) to pick a high FD. Using original FD %d.\n", + (unsigned long long) rlim.rlim_cur, fd); + log_fd_ = fd; + + return; + } - log_fd_ = fd; + // Check if target_fd is already the original_fd + if(target_fd == fd) { + log_fd_ = fd; + } else { + // Duplicate original_fd to target_fd. + // dup2 will close target_fd if it's already open. + if(dup2(fd, target_fd) == -1) { + perror("GekkoFS Log: dup2 failed"); + // Failed to move to high FD, try to use original_fd or error + // out + close(fd); // Close the one we opened + return; + } + // Now that original_fd is duplicated to target_fd, we can close + // original_fd. All writes should go to target_fd. + if(close(fd) == -1) { + perror("GekkoFS Log: Failed to close original_fd after dup2"); + } + log_fd_ = target_fd; + } + + flags = fcntl(log_fd_, F_GETFD); + if(flags == -1) { + perror("GekkoFS Log: fcntl(F_GETFD) failed on new log FD"); + } else { + if(fcntl(log_fd_, F_SETFD, flags | FD_CLOEXEC) == -1) { + perror("GekkoFS Log: fcntl(F_SETFD, FD_CLOEXEC) failed on new log FD"); + } + } } #ifdef GKFS_ENABLE_LOGGING -- GitLab From ba380ca2fb70501e7e6d0abbbd70c5f94564e012 Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Tue, 3 Jun 2025 13:11:04 +0200 Subject: [PATCH 2/2] setup a correct upper limit (not too high). avoid runtime error if the fd is invalid (just don't log) --- CHANGELOG.md | 1 + include/client/logging.hpp | 13 ++++--------- include/client/syscalls/args.hpp | 2 +- src/client/gkfs_libc.cpp | 3 ++- src/client/logging.cpp | 10 +++++++--- tests/java/java_fork.sh | 2 +- tests/java/java_vfork.sh | 2 +- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab1f98a15..ff7aee7a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - updated fmt, spdlog and pytest versions ([!249](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/249)) - Updated Docker images for 0.9.5 ([!238](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/238)) - Logging uses a higher FD limit ([!259](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/259)) + - Avoid throwing runtime error if the fd is invalid (may happen on destruction) ### Fixed - Dup3 is supported if O_CLOEXEC is not used (i.e. hexdump) ([!228](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/228)) diff --git a/include/client/logging.hpp b/include/client/logging.hpp index c92d9800b..4fdcb1b37 100644 --- a/include/client/logging.hpp +++ b/include/client/logging.hpp @@ -168,21 +168,16 @@ log_buffer(std::FILE* fp, Buffer&& buffer) { template static inline void log_buffer(int fd, Buffer&& buffer) { - - if(fd < 0) { - throw std::runtime_error("Invalid file descriptor"); + if(fd > 0) { + ::syscall_no_intercept(SYS_write, fd, buffer.data(), buffer.size()); } - - ::syscall_no_intercept(SYS_write, fd, buffer.data(), buffer.size()); } static inline void log_buffer(int fd, const void* buffer, std::size_t length) { - if(fd < 0) { - throw std::runtime_error("Invalid file descriptor"); + if(fd > 0) { + ::syscall_no_intercept(SYS_write, fd, buffer, length); } - - ::syscall_no_intercept(SYS_write, fd, buffer, length); } /** diff --git a/include/client/syscalls/args.hpp b/include/client/syscalls/args.hpp index c833f82dd..5ddd0e9f3 100644 --- a/include/client/syscalls/args.hpp +++ b/include/client/syscalls/args.hpp @@ -517,7 +517,7 @@ format_clone3_args_arg_to(FmtBuffer& buffer, const printable_arg& parg) { format_flag_set(buffer, ca->flags, flag_names); fmt::format_to(std::back_inserter(buffer), "|", "signal"); - format_signum_arg_to(buffer, {"", ca->exit_signal}); + format_signum_arg_to(buffer, {"", static_cast(ca->exit_signal)}); fmt::format_to(std::back_inserter(buffer), ",{}={}", "pidfd", (void*)ca->pidfd); fmt::format_to(std::back_inserter(buffer), ",{}={}", "child_tid", (void*)ca->child_tid); diff --git a/src/client/gkfs_libc.cpp b/src/client/gkfs_libc.cpp index 0c2721a11..c0c068f9a 100644 --- a/src/client/gkfs_libc.cpp +++ b/src/client/gkfs_libc.cpp @@ -110,7 +110,8 @@ static ResultEntry* results = nullptr; #ifdef GKFS_TRACE #define DEBUG_INFO(...) \ do { \ - LOG(DEBUG, __VA_ARGS__); \ + if(CTX->interception_enabled()) \ + LOG(DEBUG, __VA_ARGS__); \ } while(0) #else #define DEBUG_INFO(...) /* No debug output */ diff --git a/src/client/logging.cpp b/src/client/logging.cpp index e44848926..8a11fe21b 100644 --- a/src/client/logging.cpp +++ b/src/client/logging.cpp @@ -350,8 +350,9 @@ logger::logger(const std::string& opts, const std::string& path, } int target_fd = -1; - if(rlim.rlim_cur > 16) { // +3 for stdin/out/err - target_fd = rlim.rlim_cur - 16; + if(rlim.rlim_cur > + 25000) { // It shouldn't be too high, if not we have memory issues + target_fd = 24000; } else if(rlim.rlim_cur > 3) { // If limit is small, pick the highest // possible that's not 0,1,2 target_fd = rlim.rlim_cur - 1; @@ -371,6 +372,8 @@ logger::logger(const std::string& opts, const std::string& path, // Duplicate original_fd to target_fd. // dup2 will close target_fd if it's already open. if(dup2(fd, target_fd) == -1) { + fprintf(stderr, "GekkoFS Log: dup2 error for : %d.\n", + target_fd); perror("GekkoFS Log: dup2 failed"); // Failed to move to high FD, try to use original_fd or error // out @@ -390,11 +393,12 @@ logger::logger(const std::string& opts, const std::string& path, perror("GekkoFS Log: fcntl(F_GETFD) failed on new log FD"); } else { if(fcntl(log_fd_, F_SETFD, flags | FD_CLOEXEC) == -1) { - perror("GekkoFS Log: fcntl(F_SETFD, FD_CLOEXEC) failed on new log FD"); + perror("GekkoFS Log: fcntl failed on new log FD"); } } } + #ifdef GKFS_ENABLE_LOGGING const auto log_hermes_message = [](const std::string& msg, hermes::log::level l, int severity, diff --git a/tests/java/java_fork.sh b/tests/java/java_fork.sh index 8ff12c85a..cb9a57369 100755 --- a/tests/java/java_fork.sh +++ b/tests/java/java_fork.sh @@ -77,7 +77,7 @@ LD_PRELOAD=$GKFS mkdir $MNT/syscall $MNT/libc mkdir -p /builds/gitlab/hpc/gekkofs/gkfs/build/tests/run/java_fork/ export LIBGKFS_LOG_OUTPUT=/builds/gitlab/hpc/gekkofs/gkfs/build/tests/run/java_fork/client_syscall.txt # LOG should be none for java yet -export LIBGKFS_LOG=none +export LIBGKFS_LOG=all export LIBGKFS_LOG_SYSCALL_FILTER=epoll_wait,epoll_create OUTPUT_SYSCALL=`LD_PRELOAD=$GKFS $APPFORK $MNT/syscall/JAVA` diff --git a/tests/java/java_vfork.sh b/tests/java/java_vfork.sh index 7f21a0d0f..5cf64ee3f 100755 --- a/tests/java/java_vfork.sh +++ b/tests/java/java_vfork.sh @@ -77,7 +77,7 @@ LD_PRELOAD=$GKFS mkdir $MNT/syscall $MNT/libc mkdir -p /builds/gitlab/hpc/gekkofs/gkfs/build/tests/run/java_vfork/ export LIBGKFS_LOG_OUTPUT=/builds/gitlab/hpc/gekkofs/gkfs/build/tests/run/java_vfork/client_syscall.txt # LOG should be none for java yet -export LIBGKFS_LOG=none +export LIBGKFS_LOG=all export LIBGKFS_LOG_SYSCALL_FILTER=epoll_wait,epoll_create OUTPUT_SYSCALL=`LD_PRELOAD=$GKFS $APPVFORK $MNT/syscall/JAVA` -- GitLab