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 1467850df1db24b40a7a72f8d8106a595eee8988..7effeb86e9e7afb38100d225636fe0fe6ccd023c 100644 (file)
--- 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 7652e2e0d9442a619033a5e0a113905fa5b8696d..add67488914a33c98b357053da76e96f4d2b4101 100644 (file)
--- a/src/InodeCache.cpp
+++ b/src/InodeCache.cpp
@@ -34,6 +34,14 @@
 #include <sys/mman.h>
 #include <unistd.h>
 
+#ifdef HAVE_LINUX_FS_H
+#  include <linux/magic.h>
+#  include <sys/statfs.h>
+#elif defined(HAVE_STRUCT_STATFS_F_FSTYPENAME)
+#  include <sys/mount.h>
+#  include <sys/param.h>
+#endif
+
 #include <atomic>
 #include <type_traits>
 
@@ -78,6 +86,54 @@ static_assert(
 
 const void* MMAP_FAILED = reinterpret_cast<void*>(-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<std::string> 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<SharedRegion*>(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 0d76e572819f5660adc3f781fbb1df6bb04ec2e8..6bb11a5c435ffd20a8bfd07f8411bd92ef8e03ec 100644 (file)
--- 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 2464ae1a36285ffb2e07538630456514f7b3246a..80b7b97d717d289f08c043d6ea88dfbf3170983c 100644 (file)
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -54,14 +54,6 @@ extern "C" {
 #  include <sys/time.h>
 #endif
 
-#ifdef HAVE_LINUX_FS_H
-#  include <linux/magic.h>
-#  include <sys/statfs.h>
-#elif defined(HAVE_STRUCT_STATFS_F_FSTYPENAME)
-#  include <sys/mount.h>
-#  include <sys/param.h>
-#endif
-
 #ifdef __linux__
 #  ifdef HAVE_SYS_IOCTL_H
 #    include <sys/ioctl.h>
@@ -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 b12c1405fabe2fa965d2acaff7fff6c84849f765..75d59eaf89893d7b1ca786fa31a97ae66f50b7e4 100644 (file)
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -218,14 +218,6 @@ std::optional<size_t> 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 f57659aea51e865ff183e0b3cb466fd6f61c1929..fe49d566b760c0213e148f33d9555f6d6be91db4 100644 (file)
--- 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 fa5396f3a0176fb85f59e201355aba479b4fd3f1..d66e21f6277995c86c2f963ff71631b265325cd9 100644 (file)
--- a/unittest/test_InodeCache.cpp
+++ b/unittest/test_InodeCache.cpp
@@ -23,12 +23,25 @@
 #include "../src/Util.hpp"
 #include "TestUtil.hpp"
 
+#include <Fd.hpp>
+
 #include "third_party/doctest.h"
 
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
 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")
 {