Current fallocate usage is not compatible with NFS

[voidpointertonull]

Chasing some fragmentation issues on a setup with HDD, I was surprised to see rsync having an opt-in --preallocate option without trying to opportunistically use fallocate by default.

Couldn't enjoy the expected upside though as I was just getting "Operation not supported (95)" messages. Looked at the requirements of fallocate, and it seemed to be all right with using NFS 4.2 exposing a filesystem supporting fallocate, even the "fallocate" utility works, so went deeper to see what's wrong.

strace rsync: fallocate(1, FALLOC_FL_KEEP_SIZE, 0, 8388607) = -1 EOPNOTSUPP (Operation not supported)

strace fallocate: fallocate(3, 0, 0, 8388608) = 0

Apparently rsync uses FALLOC_FL_KEEP_SIZE, but NFS imposes some restrictions on the mode, not allowing that on its own:

    if ((mode != 0) && (mode != (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE)))
        return -EOPNOTSUPP;

The tricky part I can think of is the case of dealing with differential changes where rsync would surely not want to just punch a huge hole, destroying already existing data. However currently with freshly created files, there's a huge missed opportunity for reducing fragmentation.

Also, just mildly related, but I noticed the strace outputs differing in size, caused by this piece of code:

    if (length & 1) /* make the length not match the desired length */
        length++;
    else
        length--;

What kind of magic ritual is this? Wouldn't be surprised if this made sense on some exotic setup, but neither the comment, nor the almost 8 years old commit message explains the significance. Ideally this should have been made conditional to be done only where it helps, but at least I believe that it should be swept away from the optimal path of using Linux with a well-behaving fallocate.

[HanabishiRecca]

Yeah, it's unclear why FALLOC_FL_KEEP_SIZE flag is set for do_fallocate. It seems to work totally fine without it.

--- a/syscall.c
+++ b/syscall.c
@@ -592,7 +592,7 @@ int do_utime(const char *path, STRUCT_STAT *stp)

 #ifdef SUPPORT_PREALLOCATION
 #ifdef FALLOC_FL_KEEP_SIZE
-#define DO_FALLOC_OPTIONS FALLOC_FL_KEEP_SIZE
+#define DO_FALLOC_OPTIONS 0
 #else
 #define DO_FALLOC_OPTIONS 0
 #endif