Keep files open for non-zero-copy reads as well

Opening and closing files is bad for performance, so now that
we accurately track the situation of hitting too many open files
we can keep the band files open, even for the non-readbuf code
path.

This unifies the logic of opening (and caching file descriptors)
between the readbuf and non-readbuf code paths.

Author: torarnv

src/sparsebundlefs.cpp

@@ -87,9 +87,7 @@ struct sparsebundle_t {
     uint64_t band_size;
     uint64_t size;
     uint64_t times_opened;
-#if FUSE_SUPPORTS_ZERO_COPY
     map<string, int> open_files;
-#endif
     struct {
         bool allow_other = false;
         bool allow_root = false;
@@ -180,6 +178,59 @@ static int sparsebundle_open(const char *path, struct fuse_file_info *fi)
     return 0;
 }
 
+static void sparsebundle_close_files()
+{
+    sparsebundle_t *sparsebundle = sparsebundle_current();
+
+    if (sparsebundle->open_files.empty())
+        return;
+
+    syslog(LOG_DEBUG, "closing %zu open file descriptor(s)", sparsebundle->open_files.size());
+
+    map<string, int>::iterator iter;
+    for(iter = sparsebundle->open_files.begin(); iter != sparsebundle->open_files.end(); ++iter) {
+        close(iter->second);
+        syslog(LOG_DEBUG, "closed %s", iter->first.c_str());
+    }
+
+    sparsebundle->open_files.clear();
+}
+
+static rlim_t sparsebundle_max_files()
+{
+    struct rlimit fd_limit;
+    getrlimit(RLIMIT_NOFILE, &fd_limit);
+    return fd_limit.rlim_cur;
+}
+
+static int sparsebundle_open_file(const char *path)
+{
+    sparsebundle_t *sparsebundle = sparsebundle_current();
+
+    int fd = -1;
+    map<string, int>::const_iterator iter = sparsebundle->open_files.find(path);
+    if (iter != sparsebundle->open_files.end()) {
+        fd = iter->second;
+    } else {
+        syslog(LOG_DEBUG, "file %s not opened yet, opening", path);
+        if ((fd = open(path, O_RDONLY)) == -1) {
+            if (errno == EMFILE) {
+                syslog(LOG_DEBUG, "too many open file descriptors (max %ju)",
+                    uintmax_t(sparsebundle_max_files()));
+
+                sparsebundle_close_files();
+                return sparsebundle_open_file(path);
+            }
+            syslog(LOG_ERR, "failed to open %s: %s", path, strerror(errno));
+            return -errno;
+        }
+
+        sparsebundle->open_files[path] = fd;
+    }
+
+    return fd;
+}
+
 struct sparsebundle_read_operations {
     int (*process_band) (const char *, size_t, off_t, void *);
     int (*pad_with_zeroes) (size_t, void *);
@@ -255,14 +306,9 @@ static int sparsebundle_read_process_band(const char *band_path, size_t length,
     syslog(LOG_DEBUG, "reading %zu bytes at offset %ju into %p",
         length, uintmax_t(offset), static_cast<void *>(*buffer));
 
-    syslog(LOG_DEBUG, "opening %s", band_path);
-    int band_file = open(band_path, O_RDONLY);
+    int band_file = sparsebundle_open_file(band_path);
     if (band_file != -1) {
         read = pread(band_file, *buffer, length, offset);
-
-        syslog(LOG_DEBUG, "closing %s", band_path);
-        close(band_file);
-
         if (read == -1) {
             syslog(LOG_ERR, "failed to read band: %s", strerror(errno));
             return -errno;
@@ -305,51 +351,6 @@ static int sparsebundle_read(const char *path, char *buffer, size_t length, off_
 }
 
 #if FUSE_SUPPORTS_ZERO_COPY
-static void sparsebundle_read_buf_close_files()
-{
-    sparsebundle_t *sparsebundle = sparsebundle_current();
-
-    syslog(LOG_DEBUG, "closing %zu open file descriptor(s)", sparsebundle->open_files.size());
-
-    map<string, int>::iterator iter;
-    for(iter = sparsebundle->open_files.begin(); iter != sparsebundle->open_files.end(); ++iter) {
-        close(iter->second);
-        syslog(LOG_DEBUG, "closed %s", iter->first.c_str());
-    }
-
-    sparsebundle->open_files.clear();
-}
-
-static int sparsebundle_read_buf_prepare_file(const char *path)
-{
-    sparsebundle_t *sparsebundle = sparsebundle_current();
-
-    int fd = -1;
-    map<string, int>::const_iterator iter = sparsebundle->open_files.find(path);
-    if (iter != sparsebundle->open_files.end()) {
-        fd = iter->second;
-    } else {
-        syslog(LOG_DEBUG, "file %s not opened yet, opening", path);
-        if ((fd = open(path, O_RDONLY)) == -1) {
-            if (errno == EMFILE) {
-                struct rlimit fd_limit;
-                getrlimit(RLIMIT_NOFILE, &fd_limit);
-                syslog(LOG_DEBUG, "too many open files (%ju)",
-                    uintmax_t(fd_limit.rlim_cur));
-
-                sparsebundle_read_buf_close_files();
-                return sparsebundle_read_buf_prepare_file(path);
-            }
-            syslog(LOG_ERR, "failed to open band %s: %s", path, strerror(errno));
-            return -errno;
-        }
-
-        sparsebundle->open_files[path] = fd;
-    }
-
-    return fd;
-}
-
 static int sparsebundle_read_buf_process_band(const char *band_path, size_t length, off_t offset, void *read_data)
 {
     size_t read = 0;
@@ -359,7 +360,7 @@ static int sparsebundle_read_buf_process_band(const char *band_path, size_t leng
     syslog(LOG_DEBUG, "preparing %zu bytes at offset %ju", length,
         uintmax_t(offset));
 
-    int band_file_fd = sparsebundle_read_buf_prepare_file(band_path);
+    int band_file_fd = sparsebundle_open_file(band_path);
     if (band_file_fd != -1) {
         struct stat band_stat;
         stat(band_path, &band_stat);
@@ -382,7 +383,7 @@ static const char zero_device[] = "/dev/zero";
 static int sparsebundle_read_buf_pad_with_zeroes(size_t length, void *read_data)
 {
     vector<fuse_buf> *buffers = static_cast<vector<fuse_buf> *>(read_data);
-    int zero_device_fd = sparsebundle_read_buf_prepare_file(zero_device);
+    int zero_device_fd = sparsebundle_open_file(zero_device);
     fuse_buf buffer = { length, fuse_buf_flags(FUSE_BUF_IS_FD), 0, zero_device_fd, 0 };
     buffers->push_back(buffer);
 
@@ -440,11 +441,7 @@ static int sparsebundle_release(const char *path, struct fuse_file_info *)
 
     if (sparsebundle->times_opened == 0) {
         syslog(LOG_DEBUG, "no more references, cleaning up");
-
-#if FUSE_SUPPORTS_ZERO_COPY
-        if (!sparsebundle->open_files.empty())
-            sparsebundle_read_buf_close_files();
-#endif
+        sparsebundle_close_files();
     }
 
     return 0;
@@ -605,6 +602,9 @@ int main(int argc, char **argv)
         sparsebundle_filesystem_operations.read_buf = sparsebundle_read_buf;
 #endif
 
+    syslog(LOG_DEBUG, "max open file descriptors is %ju",
+        uintmax_t(sparsebundle_max_files()));
+
     int ret = fuse_main(args.argc, args.argv, &sparsebundle_filesystem_operations, &sparsebundle);
     syslog(LOG_DEBUG, "exiting with return code %d", ret);
     return ret;

tests/testhelpers.sh

@@ -53,7 +53,7 @@ function _test_can_handle_ulimit() {
         cat $f > /dev/null
     done
 
-    cat $test_output_file | grep -q "too many open files"
+    cat $test_output_file | grep -q "too many open file descriptors"
 
     umount $hfs_dir && rm -Rf $hfs_dir
     umount $mount_dir && rm -Rf $mount_dir