Avoid fadvise (since it doesn't work as expected)

Currently, it appears that at least on Linux posix_fadvise() always
purges the cache given POSIX_FADV_DONTNEED, which is what bup uses, and
does nothing for POSIX_FADV_NOREUSE, which means that before this patch,
a bup save would completely clear the filesystem cache of any file data
traversed during the run.  Aside from being completely unintended, this
meant that active VM images, large databases, etc. would probably be
purged with every save.

Since it also looks like at least NetBSD doesn't do anything for
DONTNEED, and tools like tar, cpio, and duplicity don't use it, let's
just drop it.  That's simpler, and we can always add it back if/when
someone discovers it helps somewhere relevant.

Thanks to Nimen Nacnamme for reporting the issue.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
Acked-by: Greg Troxel <gdt@lexort.com>

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index caa5c021c183d2a98f5d8f74e4fe2e44f71d4ec7..43526c221403e1e8f06e41fae17c442390ed7cd4 100644 (file)
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -942,19 +942,6 @@ static PyObject *open_noatime(PyObject *self, PyObject *args)
 }
 
 
-static PyObject *fadvise_done(PyObject *self, PyObject *args)
-{
-    int fd = -1;
-    long long ofs = 0;
-    if (!PyArg_ParseTuple(args, "iL", &fd, &ofs))
-       return NULL;
-#ifdef POSIX_FADV_DONTNEED
-    posix_fadvise(fd, 0, ofs, POSIX_FADV_DONTNEED);
-#endif    
-    return Py_BuildValue("");
-}
-
-
 // Currently the Linux kernel and FUSE disagree over the type for
 // FS_IOC_GETFLAGS and FS_IOC_SETFLAGS.  The kernel actually uses int,
 // but FUSE chose long (matching the declaration in linux/fs.h).  So
@@ -1354,8 +1341,6 @@ static PyMethodDef helper_methods[] = {
         "Return a random 20-byte string" },
     { "open_noatime", open_noatime, METH_VARARGS,
        "open() the given filename for read with O_NOATIME if possible" },
-    { "fadvise_done", fadvise_done, METH_VARARGS,
-       "Inform the kernel that we're finished with earlier parts of a file" },
 #ifdef BUP_HAVE_FILE_ATTRS
     { "get_linux_file_attr", bup_get_linux_file_attr, METH_VARARGS,
       "Return the Linux attributes for the given file." },

diff --git a/lib/bup/hashsplit.py b/lib/bup/hashsplit.py
index 571ddf6434ac1723c1948ffc5c4ba6f59843a72c..987cadb09142581c4a6e4941091c855e90d9a156 100644 (file)
--- a/lib/bup/hashsplit.py
+++ b/lib/bup/hashsplit.py
@@ -50,9 +50,6 @@ def readfile_iter(files, progress=None):
                 progress(filenum, len(b))
             b = f.read(BLOB_READ_SIZE)
             ofs += len(b)
-            # Warning: ofs == 0 means 'done with the whole file'
-            # This will only happen here when the file is empty
-            fadvise_done(f, ofs)
             if not b:
                 break
             yield b
@@ -191,9 +188,3 @@ def open_noatime(name):
         except:
             pass
         raise
-
-
-def fadvise_done(f, ofs):
-    assert(ofs >= 0)
-    if ofs > 0 and hasattr(f, 'fileno'):
-        _helpers.fadvise_done(f.fileno(), ofs)