From: Joel Rosdahl Date: Fri, 5 Aug 2022 14:39:29 +0000 (+0200) Subject: feat: Improve inode cache robustness X-Git-Tag: v4.7~119 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5edcc6c36d881e0778c8d6b0b430a1ca054426d8;p=thirdparty%2Fccache.git feat: Improve inode cache robustness - Only enable the inode cache at compile-time if it's possible to determine filesystem type. - Only use the inode cache at run-time if the filesystem type is known to work with the inode cache instead of refusing just on NFS. --- diff --git a/cmake/GenerateConfigurationFile.cmake b/cmake/GenerateConfigurationFile.cmake index 1467850df..7effeb86e 100644 --- a/cmake/GenerateConfigurationFile.cmake +++ b/cmake/GenerateConfigurationFile.cmake @@ -104,7 +104,10 @@ endif() # alias set(MTR_ENABLED "${ENABLE_TRACING}") -if(HAVE_SYS_MMAN_H AND HAVE_PTHREAD_MUTEXATTR_SETPSHARED AND (HAVE_STRUCT_STAT_ST_MTIM OR HAVE_STRUCT_STAT_ST_MTIMESPEC)) +if(HAVE_SYS_MMAN_H + AND HAVE_PTHREAD_MUTEXATTR_SETPSHARED + AND (HAVE_STRUCT_STAT_ST_MTIM OR HAVE_STRUCT_STAT_ST_MTIMESPEC) + AND (HAVE_LINUX_FS_H OR HAVE_STRUCT_STATFS_F_FSTYPENAME)) set(INODE_CACHE_SUPPORTED 1) endif() diff --git a/src/InodeCache.cpp b/src/InodeCache.cpp index 7652e2e0d..add674889 100644 --- a/src/InodeCache.cpp +++ b/src/InodeCache.cpp @@ -34,6 +34,14 @@ #include #include +#ifdef HAVE_LINUX_FS_H +# include +# include +#elif defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) +# include +# include +#endif + #include #include @@ -78,6 +86,54 @@ static_assert( const void* MMAP_FAILED = reinterpret_cast(-1); // NOLINT: Must cast here +bool +fd_is_on_known_to_work_file_system(int fd) +{ + bool known_to_work = false; + struct statfs buf; + if (fstatfs(fd, &buf) != 0) { + LOG("fstatfs failed: {}", strerror(errno)); + } else { +#ifdef HAVE_LINUX_FS_H + switch (buf.f_type) { + // Is a filesystem you know works with the inode cache missing in this + // list? Please submit an issue or pull request to the ccache project. + case 0x9123683e: // BTRFS_SUPER_MAGIC + case 0xef53: // EXT2_SUPER_MAGIC + case 0x01021994: // TMPFS_MAGIC + case 0x58465342: // XFS_SUPER_MAGIC + known_to_work = true; + break; + default: + LOG("Filesystem type 0x{:x} not known to work for the inode cache", + buf.f_type); + } +#elif defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) // macOS X and some BSDs + static const std::vector known_to_work_filesystems = { + // Is a filesystem you know works with the inode cache missing in this + // list? Please submit an issue or pull request to the ccache project. + "apfs", + "xfs", + }; + if (std::find(known_to_work_filesystems.begin(), + known_to_work_filesystems.end(), + buf.f_fstypename) + != known_to_work_filesystems.end()) { + known_to_work = true; + } else { + LOG("Filesystem type {} not known to work for the inode cache", + buf.f_fstypename); + } +#else +# error Inconsistency: INODE_CACHE_SUPPORTED is set but we should not get here +#endif + } + if (!known_to_work) { + LOG_RAW("Not using the inode cache"); + } + return known_to_work; +} + } // namespace struct InodeCache::Key @@ -125,11 +181,7 @@ InodeCache::mmap_file(const std::string& inode_cache_file) LOG("Failed to open inode cache {}: {}", inode_cache_file, strerror(errno)); return false; } - bool is_nfs; - if (Util::is_nfs_fd(*fd, &is_nfs) == 0 && is_nfs) { - LOG( - "Inode cache not supported because the cache file is located on nfs: {}", - inode_cache_file); + if (!fd_is_on_known_to_work_file_system(*fd)) { return false; } SharedRegion* sr = reinterpret_cast(mmap( @@ -239,12 +291,7 @@ InodeCache::create_new_file(const std::string& filename) Finalizer temp_file_remover([&] { unlink(tmp_file.path.c_str()); }); - bool is_nfs; - if (Util::is_nfs_fd(*tmp_file.fd, &is_nfs) == 0 && is_nfs) { - LOG( - "Inode cache not supported because the cache file would be located on" - " nfs: {}", - filename); + if (!fd_is_on_known_to_work_file_system(*tmp_file.fd)) { return false; } int err = Util::fallocate(*tmp_file.fd, sizeof(SharedRegion)); @@ -335,6 +382,12 @@ InodeCache::~InodeCache() } } +bool +InodeCache::available(int fd) +{ + return fd_is_on_known_to_work_file_system(fd); +} + bool InodeCache::get(const std::string& path, ContentType type, diff --git a/src/InodeCache.hpp b/src/InodeCache.hpp index 0d76e5728..6bb11a5c4 100644 --- a/src/InodeCache.hpp +++ b/src/InodeCache.hpp @@ -44,6 +44,10 @@ public: InodeCache(const Config& config); ~InodeCache(); + // Return whether it's possible to use the inode cache on the filesystem + // associated with `fd`. + static bool available(int fd); + // Get saved hash digest and return value from a previous call to // do_hash_file() in hashutil.cpp. // diff --git a/src/Util.cpp b/src/Util.cpp index 2464ae1a3..80b7b97d7 100644 --- a/src/Util.cpp +++ b/src/Util.cpp @@ -54,14 +54,6 @@ extern "C" { # include #endif -#ifdef HAVE_LINUX_FS_H -# include -# include -#elif defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) -# include -# include -#endif - #ifdef __linux__ # ifdef HAVE_SYS_IOCTL_H # include @@ -808,29 +800,6 @@ is_ccache_executable(const std::string_view path) return util::starts_with(name, "ccache"); } -#if defined(HAVE_LINUX_FS_H) || defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) -int -is_nfs_fd(int fd, bool* is_nfs) -{ - struct statfs buf; - if (fstatfs(fd, &buf) != 0) { - return errno; - } -# ifdef HAVE_LINUX_FS_H - *is_nfs = buf.f_type == NFS_SUPER_MAGIC; -# else // Mac OS X and some other BSD flavors - *is_nfs = strcmp(buf.f_fstypename, "nfs") == 0; -# endif - return 0; -} -#else -int -is_nfs_fd(int /*fd*/, bool* /*is_nfs*/) -{ - return -1; -} -#endif - bool is_precompiled_header(std::string_view path) { diff --git a/src/Util.hpp b/src/Util.hpp index b12c1405f..75d59eaf8 100644 --- a/src/Util.hpp +++ b/src/Util.hpp @@ -218,14 +218,6 @@ std::optional is_absolute_path_with_prefix(std::string_view path); // Detmine if `path` refers to a ccache executable. bool is_ccache_executable(std::string_view path); -// Test if a file is on nfs. -// -// Sets is_nfs to the result if fstatfs is available and no error occurred. -// -// Returns 0 if is_nfs was set, -1 if fstatfs is not available or errno if an -// error occurred. -int is_nfs_fd(int fd, bool* is_nfs); - // Return whether `ch` is a directory separator, i.e. '/' on POSIX systems and // '/' or '\\' on Windows systems. inline bool diff --git a/test/suites/inode_cache.bash b/test/suites/inode_cache.bash index f57659aea..fe49d566b 100644 --- a/test/suites/inode_cache.bash +++ b/test/suites/inode_cache.bash @@ -4,9 +4,14 @@ SUITE_inode_cache_PROBE() { return fi - mkdir -p "${CCACHE_DIR}" - if [ "$(stat -fLc %T "${CCACHE_DIR}")" = "nfs" ]; then - echo "ccache directory is on NFS" + unset CCACHE_NODIRECT + export CCACHE_TEMPDIR="${CCACHE_DIR}/tmp" + + touch test.c + $CCACHE $COMPILER -c test.c + if [[ ! -f "${CCACHE_TEMPDIR}/inode-cache.v1" ]]; then + local fs_type=$(stat -fLc %T "${CCACHE_DIR}") + echo "inode cache not supported on ${fs_type}" fi } diff --git a/unittest/test_InodeCache.cpp b/unittest/test_InodeCache.cpp index fa5396f3a..d66e21f62 100644 --- a/unittest/test_InodeCache.cpp +++ b/unittest/test_InodeCache.cpp @@ -23,12 +23,25 @@ #include "../src/Util.hpp" #include "TestUtil.hpp" +#include + #include "third_party/doctest.h" +#include +#include +#include + using TestUtil::TestContext; namespace { +bool +inode_cache_available() +{ + Fd fd(open(Util::get_actual_cwd().c_str(), O_RDONLY)); + return fd && InodeCache::available(*fd); +} + void init(Config& config) { @@ -51,7 +64,7 @@ put(InodeCache& inode_cache, } // namespace -TEST_SUITE_BEGIN("InodeCache"); +TEST_SUITE_BEGIN("InodeCache" * doctest::skip(!inode_cache_available())); TEST_CASE("Test disabled") {