Re: [PATCH] New fallocate module

[Pádraig Brady]

Take 2 attached.

Still outstanding from the gnulib portion is support for solaris.
I may get time to set this up, but I've very little time lately,
so I would like not to require this for merging.

Also I didn't yet implement the optimization to coreutils
that Paul mentioned, to cache whether the filesystem will
just return ENOSYS all the time.

cheers,
Pádraig.

>From bd55931dd6286ff952fdcadd25d0bef263be4de2 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Thu, 21 May 2009 08:03:00 +0100
Subject: [PATCH] fallocate: New module to ensure this interface is available

fallocate() allows one to associate a unit of space
with a (portion of a) file before writing.  This info can
then be used to immediately determine if enough space is available,
and also allow efficient allocation on the storage device.
* m4/fallocate.m4: check we can link to fallocate()
* lib/fcntl.in.h: replacement stub if missing
* modules/fallocate: dependencies for new module
* MODULES.html.sh (File system functions): add it
---
 MODULES.html.sh   |    1 +
 lib/fcntl.in.h    |   17 +++++++++++++++++
 m4/fallocate.m4   |   26 ++++++++++++++++++++++++++
 modules/fallocate |   23 +++++++++++++++++++++++
 4 files changed, 67 insertions(+), 0 deletions(-)
 create mode 100644 m4/fallocate.m4
 create mode 100644 modules/fallocate

diff --git a/MODULES.html.sh b/MODULES.html.sh
index 06afa2d..6a1e058 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -2438,6 +2438,7 @@ func_all_modules ()
   func_module dirfd
   func_module double-slash-root
   func_module euidaccess
+  func_module fallocate
   func_module file-type
   func_module fileblocks
   func_module filemode
diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h
index fd7520e..709db88 100644
--- a/lib/fcntl.in.h
+++ b/lib/fcntl.in.h
@@ -63,6 +63,23 @@ extern int open (const char *filename, int flags, ...);
 extern void _gl_register_fd (int fd, const char *filename);
 #endif
 
+#ifdef REPLACE_FALLOCATE
+# undef fallocate
+# define fallocate rpl_fallocate
+#include <sys/types.h>
+#include <errno.h>
+#undef FALLOC_FL_KEEP_SIZE /* Ensure this name is available.  */
+#define FALLOC_FL_KEEP_SIZE 0x01
+static inline int fallocate (int fd, int mode, off_t offset, off_t len)
+{
+  /* This is a valid replacement for missing glibc fallocate(),
+     because code calling fallocate() must also handle this error
+     in the case that the kernel or filesystem don't support this.  */
+  return ENOSYS;
+  /* FIXME: support fcntl(fd, F_ALLOCSP, ...) on solaris.  */
+}
+#endif
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/m4/fallocate.m4 b/m4/fallocate.m4
new file mode 100644
index 0000000..b218253
--- /dev/null
+++ b/m4/fallocate.m4
@@ -0,0 +1,26 @@
+# fallocate.m4 serial 1
+dnl Copyright (C) 2009 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+AC_DEFUN([gl_FUNC_FALLOCATE],
+[
+  AC_CHECK_HEADERS_ONCE([linux/falloc.h])
+
+  dnl Persuade glibc <fcntl.h> to declare fallocate().
+  AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
+
+  AC_CACHE_CHECK([for fallocate], [gl_cv_func_fallocate], [
+    AC_TRY_LINK([#include <fcntl.h>        /* fallocate() declaration */
+                 #include <linux/falloc.h> /* FALLOC_FL_KEEP_SIZE define */],
+      [fallocate(-1, FALLOC_FL_KEEP_SIZE, 0, 0);],
+      gl_cv_func_fallocate=yes, gl_cv_func_fallocate=no)])
+
+    if test $gl_cv_func_fallocate = yes; then
+      AC_DEFINE([HAVE_FALLOCATE], [1], [Defined if fallocate() exists])
+    else
+      AC_REQUIRE([AC_C_INLINE])
+      AC_DEFINE([REPLACE_FALLOCATE], [1], [Include replacement])
+    fi
+])
diff --git a/modules/fallocate b/modules/fallocate
new file mode 100644
index 0000000..bbcb5db
--- /dev/null
+++ b/modules/fallocate
@@ -0,0 +1,23 @@
+Description:
+Ensure fallocate() is available
+
+Files:
+m4/fallocate.m4
+
+Depends-on:
+errno
+extensions
+fcntl
+
+configure.ac:
+gl_FUNC_FALLOCATE
+
+Makefile.am:
+
+Include:
+
+License:
+LGPL
+
+Maintainer:
+Pádraig Brady
-- 
1.5.3.6

>From 7202ea62e6f6358931c50327df4a7b70b84332b7 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Mon, 18 May 2009 08:38:18 +0100
Subject: [PATCH] cp,mv,install: Try to fallocate() the destination file

This will allocate the file efficiently and also
give immediate indication of insufficient space for the copy.
* bootstrap.conf: include fallocate module from gnulib.
* src/copy.c (copy_reg): call fallocate() for non sparse files.
* NEWS: mention the change.
---
 NEWS           |    6 ++++++
 bootstrap.conf |    1 +
 src/copy.c     |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/NEWS b/NEWS
index 31f1b1a..80bc22b 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,12 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   truncate -s failed to skip all whitespace in the option argument in
   some locales.
 
+** Changes in behavior
+
+  cp, install, mv: now try to allocate the destination file as an extent.
+  This will give immediate feedback if there is not enough space for the
+  copy, and allow more efficient layout of the file on the storage device.
+
 * Noteworthy changes in release 7.4 (2009-05-07) [stable]
 
 ** Bug fixes
diff --git a/bootstrap.conf b/bootstrap.conf
index d34e908..d044b7e 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,6 +79,7 @@ gnulib_modules="
   euidaccess
   exclude
   exitfail
+  fallocate
   fchdir
   fcntl
   fcntl-safer
diff --git a/src/copy.c b/src/copy.c
index 511f705..eb00fac 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -54,6 +54,10 @@
 #include "areadlink.h"
 #include "yesno.h"
 
+#if HAVE_LINUX_FALLOC_H
+#include <linux/falloc.h>
+#endif
+
 #if USE_XATTR
 # include <attr/error_context.h>
 # include <attr/libattr.h>
@@ -603,6 +607,9 @@ copy_reg (char const *src_name, char const *dst_name,
     bool last_write_made_hole = false;
     bool make_holes = false;
 
+    /* Size to fallocate.  */
+    off_t alloc_size = 0;
+
     if (S_ISREG (sb.st_mode))
       {
        /* Even with --sparse=always, try to create holes only
@@ -615,10 +622,41 @@ copy_reg (char const *src_name, char const *dst_name,
           blocks.  If the file has fewer blocks than would normally be
           needed for a file of its size, then at least one of the blocks in
           the file is a hole.  */
-       if (x->sparse_mode == SPARSE_AUTO && S_ISREG (src_open_sb.st_mode)
+        else if (x->sparse_mode == SPARSE_AUTO && S_ISREG (src_open_sb.st_mode)
            && ST_NBLOCKS (src_open_sb) < src_open_sb.st_size / ST_NBLOCKSIZE)
          make_holes = true;
 #endif
+
+       /* If not making a sparse file, try to fallocate() the destination.
+          I.E. tell the system that this amount of space should be treated
+          as a unit.  This will allow the system to inform us immediately
+          if there is not enough space available.  The info can also be used
+          to lay out the blocks more efficiently on the storage device.  */
+       else
+         {
+           /* Get the maximum space needed for a file.
+              If the current st_blocks is smaller than size, then
+              sparse_mode must be SPARSE_NEVER, so we allocate the full
+              st_size.  If the allocation is larger than size due to
+              a previous fallocate call, then we maintain this allocation.  */
+           alloc_size = src_open_sb.st_size;
+#if HAVE_STRUCT_STAT_ST_BLOCKS
+           alloc_size = MAX (ST_NBLOCKS (src_open_sb) * ST_NBLOCKSIZE,
+                             alloc_size);
+#endif
+           /* FIXME: Should we use ioctl(BLKGETSIZE64) on block devices?  */
+
+           /* FALLOC_FL_KEEP_SIZE means st_size is not updated.  */
+           int err = fallocate (dest_desc, FALLOC_FL_KEEP_SIZE, 0, alloc_size);
+           if (err != 0)
+             alloc_size = 0; /* reset for truncation test later on.  */
+           if (err != ENOTSUP && err != ENOSYS)
+             {
+               error (0, err, _("allocating %s"), quote (dst_name));
+               return_val = false;
+               goto close_src_and_dst_desc;
+             }
+         }
       }
 
     /* If not making a sparse file, try to use a more-efficient
@@ -753,6 +791,22 @@ copy_reg (char const *src_name, char const *dst_name,
            goto close_src_and_dst_desc;
          }
       }
+
+    /* If we fallocated space for the destination, check that
+       we were able to copy at least st_size bytes. Otherwise
+       the source was probably truncated, so we'll truncate
+       the destination to match.  */
+
+    if (alloc_size && n_read_total < src_open_sb.st_size)
+      {
+       /* Assume we have ftruncate() if we have fallocate()  */
+       if (ftruncate (dest_desc, n_read_total) < 0)
+         {
+           error (0, errno, _("truncating %s"), quote (dst_name));
+           return_val = false;
+           goto close_src_and_dst_desc;
+         }
+      }
   }
 
   if (x->preserve_timestamps)
-- 
1.5.3.6