Btrfs clone support in copy operations

Btrfs clone support in copy operations

[Kieran Colford]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

I've put together a patch for supporting the btrfs clone operation for
copying

diff --git a/src/fileio.c b/src/fileio.c
index b1f9d3c..d7c1e6e 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -21,6 +21,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <limits.h>
 #include <fcntl.h>
 #include "sysstdio.h"
+#include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -1988,6 +1989,16 @@ permissions.  */)
 	oldsize = out_st.st_size;
     }
 
+  /* Perform the O(1) btrfs clone operation, if possible. */
+#ifdef __linux__
+# undef BTRFS_IOCTL_MAGIC
+# define BTRFS_IOCTL_MAGIC 0x94
+# undef BTRFS_IOC_CLONE
+# define BTRFS_IOC_CLONE _IOW (BTRFS_IOCTL_MAGIC, 9, int)
+  if (ioctl (ofd, BTRFS_IOC_CLONE, ifd) == 0)
+    goto done_copy;
+#endif
+
   immediate_quit = 1;
   QUIT;
   while (true)
@@ -2011,6 +2022,8 @@ permissions.  */)
 
   immediate_quit = 0;
 
+ done_copy:
+
 #ifndef MSDOS
   /* Preserve the original file permissions, and if requested, also its
      owner and group.  */

-- 
Kieran Colford <kieran@kcolford.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Btrfs clone support in copy operations

[sbaugh]

Kieran Colford <kieran@kcolford.com> writes:
> +  /* Perform the O(1) btrfs clone operation, if possible. */
> +#ifdef __linux__
> +# undef BTRFS_IOCTL_MAGIC
> +# define BTRFS_IOCTL_MAGIC 0x94
> +# undef BTRFS_IOC_CLONE
> +# define BTRFS_IOC_CLONE _IOW (BTRFS_IOCTL_MAGIC, 9, int)
> +  if (ioctl (ofd, BTRFS_IOC_CLONE, ifd) == 0)
> +    goto done_copy;
> +#endif

Perhaps you should use the FICLONE flag, which doesn't mention
BTRFS. (and is conceivably filesystem independent - I think XFS supports
it?)

Re: Btrfs clone support in copy operations

[Paul Eggert]

Thanks for the suggestion. I've created a bug report for this 
suggestion, Bug#23904. You can view its current status here:

http://bugs.gnu.org/23904

or add to the thread by sending email to 23904@debbugs.gnu.org.

bug#23904: Btrfs clone support in copy operations

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 150 bytes --]

Tags: patch

The attached proposed patch to Emacs master builds on a suggestion by 
Kieran Colford. Kieran, can you please comment on its two FIXMEs?

[-- Attachment #2: 0001-copy-file-now-uses-GNU-Linux-file-cloning.patch --]
[-- Type: text/x-patch, Size: 3811 bytes --]

From 4b2a532a87ec2a21afc205b83db4a779664ccd20 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 6 Jul 2016 02:42:58 +0200
Subject: [PATCH] copy-file now uses GNU/Linux file cloning

From a suggestion by Kieran Colford in:
http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00191.html
* configure.ac: Check for linux/fs.h.
* src/fileio.c [HAVE_LINUX_FS_H]: Include sys/ioctl.h and linux/fs.h.
(clone_file): New function.
(Fcopy_file): Use it.
---
 configure.ac |  1 +
 src/fileio.c | 64 ++++++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/configure.ac b/configure.ac
index aaddfcd..1798fa3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1636,6 +1636,7 @@ AC_DEFUN
 
 dnl checks for header files
 AC_CHECK_HEADERS_ONCE(
+  linux/fs.h
   malloc.h
   sys/systeminfo.h
   sys/sysinfo.h
diff --git a/src/fileio.c b/src/fileio.c
index b1f9d3c..c2e3a18 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -52,6 +52,11 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "region-cache.h"
 #include "frame.h"
 
+#ifdef HAVE_LINUX_FS_H
+# include <sys/ioctl.h>
+# include <linux/fs.h>
+#endif
+
 #ifdef WINDOWSNT
 #define NOMINMAX 1
 #include <windows.h>
@@ -1828,6 +1833,24 @@ barf_or_query_if_file_exists (Lisp_Object absname, bool known_to_exist,
     }
 }
 
+/* Copy all data to DEST from SOURCE if possible.  Return true if OK.  */
+static bool
+clone_file (int dest, int source)
+{
+#ifdef FICLONE
+  /* FIXME: Might this ioctl take a long time if the file is very
+     large?  Is the ioctl interruptible?  If so, what happens if the
+     ioctl is interrupted by a signal?  Might it partially copy the
+     file?  */
+  /* FIXME: Does this ioctl truncate the output file to be the same
+     size as the input file, if the output file already exists and is
+     larger than the input file?  */
+  if (ioctl (dest, FICLONE, source) == 0)
+    return true;
+#endif
+  return false;
+}
+
 DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6,
        "fCopy file: \nGCopy %s to file: \np\nP",
        doc: /* Copy FILE to NEWNAME.  Both args must be strings.
@@ -1988,28 +2011,31 @@ permissions.  */)
 	oldsize = out_st.st_size;
     }
 
-  immediate_quit = 1;
-  QUIT;
-  while (true)
+  if (! clone_file (ofd, ifd))
     {
-      char buf[MAX_ALLOCA];
-      ptrdiff_t n = emacs_read (ifd, buf, sizeof buf);
-      if (n < 0)
-	report_file_error ("Read error", file);
-      if (n == 0)
-	break;
-      if (emacs_write_sig (ofd, buf, n) != n)
-	report_file_error ("Write error", newname);
-      newsize += n;
-    }
+      immediate_quit = 1;
+      QUIT;
+      while (true)
+	{
+	  char buf[MAX_ALLOCA];
+	  ptrdiff_t n = emacs_read (ifd, buf, sizeof buf);
+	  if (n < 0)
+	    report_file_error ("Read error", file);
+	  if (n == 0)
+	    break;
+	  if (emacs_write_sig (ofd, buf, n) != n)
+	    report_file_error ("Write error", newname);
+	  newsize += n;
+	}
 
-  /* Truncate any existing output file after writing the data.  This
-     is more likely to work than truncation before writing, if the
-     file system is out of space or the user is over disk quota.  */
-  if (newsize < oldsize && ftruncate (ofd, newsize) != 0)
-    report_file_error ("Truncating output file", newname);
+      /* Truncate any existing output file after writing the data.  This
+	 is more likely to work than truncation before writing, if the
+	 file system is out of space or the user is over disk quota.  */
+      if (newsize < oldsize && ftruncate (ofd, newsize) != 0)
+	report_file_error ("Truncating output file", newname);
 
-  immediate_quit = 0;
+      immediate_quit = 0;
+    }
 
 #ifndef MSDOS
   /* Preserve the original file permissions, and if requested, also its
-- 
2.5.5

bug#23904: Btrfs clone support in copy operations

[Dmitry Antipov]

On 07/06/2016 03:49 AM, Paul Eggert wrote:

> The attached proposed patch to Emacs master builds on a suggestion by Kieran Colford.

IMHO this should use FICLONERANGE in a loop where you can
handle errors and use process_pending_signals as usual.

Dmitry

bug#23904: Btrfs clone support in copy operations

[Helmut Eller]

On Wed, Jul 06 2016, Dmitry Antipov wrote:

> On 07/06/2016 03:49 AM, Paul Eggert wrote:
>
>> The attached proposed patch to Emacs master builds on a suggestion
>> by Kieran Colford.
>
> IMHO this should use FICLONERANGE in a loop where you can
> handle errors and use process_pending_signals as usual.

This might also be a use case for sendfile(2).

Helmut

bug#23904: Btrfs clone support in copy operations

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

On 07/06/2016 04:45 AM, Dmitry Antipov wrote:
> IMHO this should use FICLONERANGE in a loop where you can
> handle errors and use process_pending_signals as usual. 

Thanks, that suggestion should resolve the FIXME about how FICLONE 
behaves when the output file is already larger than the input. However, 
what should the clone chunk size be,each time through the loop? I don't 
use btrfs so I don't know what a good size would be.

A downside of the suggestion is that the file copy won't be atomic. 
However, the existing code isn't atomic, so this is nothing new.

Revised patch attached. Basically untested, since I don't use any file 
systems that support FIOCLONERANGE. The patch still has FIXMEs for what 
happens if the FIOCLONERANGE ioctl is interrupted by a signal, and for a 
good clone chunk size (the patch guesses 1 GiB).

[-- Attachment #2: 0001-copy-file-now-uses-GNU-Linux-file-cloning.patch --]
[-- Type: text/x-patch, Size: 3674 bytes --]

From 9c60f6f603563c335737102fc54609b837f5d02a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 6 Jul 2016 12:44:46 +0200
Subject: [PATCH] copy-file now uses GNU/Linux file cloning

From a suggestion by Kieran Colford (see Bug#23904).
* configure.ac: Check for linux/fs.h.
* src/fileio.c [HAVE_LINUX_FS_H]: Include sys/ioctl.h and linux/fs.h.
(clone_file_range): New function.
(Fcopy_file): Use it.
---
 configure.ac |  1 +
 src/fileio.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/configure.ac b/configure.ac
index aaddfcd..1798fa3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1636,6 +1636,7 @@ AC_DEFUN
 
 dnl checks for header files
 AC_CHECK_HEADERS_ONCE(
+  linux/fs.h
   malloc.h
   sys/systeminfo.h
   sys/sysinfo.h
diff --git a/src/fileio.c b/src/fileio.c
index b1f9d3c..f980247 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -52,6 +52,11 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "region-cache.h"
 #include "frame.h"
 
+#ifdef HAVE_LINUX_FS_H
+# include <sys/ioctl.h>
+# include <linux/fs.h>
+#endif
+
 #ifdef WINDOWSNT
 #define NOMINMAX 1
 #include <windows.h>
@@ -1828,6 +1833,23 @@ barf_or_query_if_file_exists (Lisp_Object absname, bool known_to_exist,
     }
 }
 
+/* Copy data to DEST from SOURCE if possible, starting at byte offset
+   OFF and continuing for LENGTH bytes.  Return true if OK.  */
+static bool
+clone_file_range (int dest, int source, off_t off, off_t length)
+{
+#ifdef FICLONERANGE
+  /* FIXME: Is the ioctl interruptible?  If so, what happens if the
+     ioctl is interrupted by a signal?  Might it partially copy the
+     range?  */
+  struct file_clone_range range = { src_fd: source, src_offset: off,
+				    src_length: length, dest_offset: off };
+  if (ioctl (dest, FICLONERANGE, &range) == 0)
+    return true;
+#endif
+  return false;
+}
+
 DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6,
        "fCopy file: \nGCopy %s to file: \np\nP",
        doc: /* Copy FILE to NEWNAME.  Both args must be strings.
@@ -1974,7 +1996,7 @@ permissions.  */)
 
   record_unwind_protect_int (close_file_unwind, ofd);
 
-  off_t oldsize = 0, newsize = 0;
+  off_t oldsize = 0, newsize;
 
   if (already_exists)
     {
@@ -1990,18 +2012,32 @@ permissions.  */)
 
   immediate_quit = 1;
   QUIT;
-  while (true)
-    {
-      char buf[MAX_ALLOCA];
-      ptrdiff_t n = emacs_read (ifd, buf, sizeof buf);
-      if (n < 0)
-	report_file_error ("Read error", file);
-      if (n == 0)
-	break;
-      if (emacs_write_sig (ofd, buf, n) != n)
-	report_file_error ("Write error", newname);
-      newsize += n;
-    }
+
+  off_t insize = st.st_size;
+  /* FIXME: is 1 GiB a good number?  */
+  off_t max_clone_chunk_size = 1 << 30;
+  off_t chunk_size = min (insize, max_clone_chunk_size);
+
+  if (clone_file_range (ofd, ifd, 0, chunk_size))
+    for (newsize = chunk_size; newsize < insize; newsize += chunk_size)
+      {
+	chunk_size = min (insize - newsize, max_clone_chunk_size);
+	if (! clone_file_range (ofd, ifd, newsize, chunk_size))
+	  report_file_error ("Clone error", file);
+      }
+  else
+    for (newsize = 0; ; newsize += chunk_size)
+      {
+	char buf[MAX_ALLOCA];
+	ptrdiff_t n = emacs_read (ifd, buf, sizeof buf);
+	if (n < 0)
+	  report_file_error ("Read error", file);
+	if (n == 0)
+	  break;
+	if (emacs_write_sig (ofd, buf, n) != n)
+	  report_file_error ("Write error", newname);
+	chunk_size = n;
+      }
 
   /* Truncate any existing output file after writing the data.  This
      is more likely to work than truncation before writing, if the
-- 
2.5.5

bug#23904: Btrfs clone support in copy operations

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 6 Jul 2016 13:48:26 +0200
> Cc: sbaugh@catern.com, Kieran Colford <kieran@kcolford.com>
> 
> Revised patch attached. Basically untested, since I don't use any file 
> systems that support FIOCLONERANGE. The patch still has FIXMEs for what 
> happens if the FIOCLONERANGE ioctl is interrupted by a signal, and for a 
> good clone chunk size (the patch guesses 1 GiB).

Thanks.

However, I wonder: are we sure users will always want this
unconditionally, completely out of their control?  Does 'cp' from
Coreutils do the same unconditionally, for example?

bug#23904: Btrfs clone support in copy operations

[Paul Eggert]

On 07/06/2016 04:58 PM, Eli Zaretskii wrote:
> are we sure users will always want this
> unconditionally, completely out of their control?  Does 'cp' from
> Coreutils do the same unconditionally, for example?

No, coreutils cp has an option. As I recall it was put in soon after the 
btrfs feature was added, and we were worried about bugs in btrfs. This 
was several years ago.  We're thinking of making cloning the default in 
cp (on systems that support it) but haven't gotten around to it.

We could add another optional argument to copy-file as well, if there's 
a significant need. I would omit such an option at first, though, as 
that's simpler.

bug#23904: Btrfs clone support in copy operations

[Eli Zaretskii]

> Cc: dmantipov@yandex.ru, 23904@debbugs.gnu.org, sbaugh@catern.com,
>  kieran@kcolford.com
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 6 Jul 2016 17:08:33 +0200
> 
> No, coreutils cp has an option. As I recall it was put in soon after the 
> btrfs feature was added, and we were worried about bugs in btrfs. This 
> was several years ago.  We're thinking of making cloning the default in 
> cp (on systems that support it) but haven't gotten around to it.

When 'cp' switches to using this by default, will it still have an
option to disable it?  If so, I would think it will be too bold for
Emacs not provide such an option for users, even as opt-out.

bug#23904: Btrfs clone support in copy operations

[Paul Eggert]

On 07/06/2016 05:26 PM, Eli Zaretskii wrote:
> When 'cp' switches to using this by default, will it still have an
> option to disable it?
Yes. It already has that option and we wouldn't remove it due to 
backward-compatibility concerns, at least, not for many years. The 
situation for Emacs is a bit different, as Emacs doesn't have the 
backward-compatibility issues.

bug#23904: Btrfs clone support in copy operations

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

I looked into this a bit more, and it appears that there's little point to 
giving the user an option to clone or not clone when copying files, as the 
cloning (or not cloning) can occur anyway.

We plan to release a coreutils version soon and to change the default to clone 
after the release. For Emacs master the next release is far away, so there is 
plenty of time to try out the FICLONE performance improvement. I installed the 
attached two patches and am marking this bug report as done.

The second patch merely updates the documentation to discuss the issue; it is 
logically independent of the first patch, since the issue can occur both with 
and without the first patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-copy-file-now-uses-GNU-Linux-file-cloning.patch --]
[-- Type: text/x-diff; name="0001-copy-file-now-uses-GNU-Linux-file-cloning.patch", Size: 2728 bytes --]

From f9527cb5a5cdd05b2c98be089023b64ee9ee318b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 10 Sep 2016 12:51:27 -0700
Subject: [PATCH 1/2] copy-file now uses GNU/Linux file cloning

From a suggestion by Kieran Colford (see Bug#23904).
* configure.ac: Check for linux/fs.h.
* src/fileio.c [HAVE_LINUX_FS_H]: Include sys/ioctl.h and linux/fs.h.
(clone_file): New function.
(Fcopy_file): Use it.
---
 configure.ac |  1 +
 src/fileio.c | 33 +++++++++++++++++++++++++--------
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9856228..82a672b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1638,6 +1638,7 @@ AC_DEFUN
 
 dnl checks for header files
 AC_CHECK_HEADERS_ONCE(
+  linux/fs.h
   malloc.h
   sys/systeminfo.h
   sys/sysinfo.h
diff --git a/src/fileio.c b/src/fileio.c
index bf6bf62..b4316b3 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -52,6 +52,11 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "region-cache.h"
 #include "frame.h"
 
+#ifdef HAVE_LINUX_FS_H
+# include <sys/ioctl.h>
+# include <linux/fs.h>
+#endif
+
 #ifdef WINDOWSNT
 #define NOMINMAX 1
 #include <windows.h>
@@ -1829,6 +1834,16 @@ barf_or_query_if_file_exists (Lisp_Object absname, bool known_to_exist,
     }
 }
 
+/* Copy data to DEST from SOURCE if possible.  Return true if OK.  */
+static bool
+clone_file (int dest, int source)
+{
+#ifdef FICLONE
+  return ioctl (dest, FICLONE, source) == 0;
+#endif
+  return false;
+}
+
 DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6,
        "fCopy file: \nGCopy %s to file: \np\nP",
        doc: /* Copy FILE to NEWNAME.  Both args must be strings.
@@ -1975,7 +1990,7 @@ permissions.  */)
 
   record_unwind_protect_int (close_file_unwind, ofd);
 
-  off_t oldsize = 0, newsize = 0;
+  off_t oldsize = 0, newsize;
 
   if (already_exists)
     {
@@ -1991,17 +2006,19 @@ permissions.  */)
 
   immediate_quit = 1;
   QUIT;
-  while (true)
+
+  if (clone_file (ofd, ifd))
+    newsize = st.st_size;
+  else
     {
       char buf[MAX_ALLOCA];
-      ptrdiff_t n = emacs_read (ifd, buf, sizeof buf);
+      ptrdiff_t n;
+      for (newsize = 0; 0 < (n = emacs_read (ifd, buf, sizeof buf));
+	   newsize += n)
+	if (emacs_write_sig (ofd, buf, n) != n)
+	  report_file_error ("Write error", newname);
       if (n < 0)
 	report_file_error ("Read error", file);
-      if (n == 0)
-	break;
-      if (emacs_write_sig (ofd, buf, n) != n)
-	report_file_error ("Write error", newname);
-      newsize += n;
     }
 
   /* Truncate any existing output file after writing the data.  This
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Document-file-synchronization-issues.patch --]
[-- Type: text/x-diff; name="0002-Document-file-synchronization-issues.patch", Size: 4130 bytes --]

From 62f0eb5f8ce6a05288c0b7b7cf8ec3dc373583e4 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 10 Sep 2016 19:12:21 -0700
Subject: [PATCH 2/2] Document file synchronization issues

* doc/lispref/files.texi (Files and Storage): New section.
---
 doc/emacs/files.texi     |  4 ++--
 doc/lispref/backups.texi |  5 +++++
 doc/lispref/files.texi   | 25 +++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index f195a41..7bf4690 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -1554,8 +1554,8 @@ Misc File Ops
 
 @findex copy-file
 @cindex copying files
-  @kbd{M-x copy-file} reads the file @var{old} and writes a new file
-named @var{new} with the same contents.
+  @kbd{M-x copy-file} copies the contents of the file @var{old} to the
+file @var{new}.
 
 @findex copy-directory
   @kbd{M-x copy-directory} copies directories, similar to the
diff --git a/doc/lispref/backups.texi b/doc/lispref/backups.texi
index b9e6466..35a1865 100644
--- a/doc/lispref/backups.texi
+++ b/doc/lispref/backups.texi
@@ -41,6 +41,11 @@ Backup Files
 file gets a new name.  You can delete old numbered backups when you
 don't want them any more, or Emacs can delete them automatically.
 
+  For performance, the operating system may not write the backup
+file's contents to secondary storage immediately, or may alias the
+backup data with the original until one or the other is later
+modified.  @xref{Files and Storage}.
+
 @menu
 * Making Backups::     How Emacs makes backup files, and when.
 * Rename or Copy::     Two alternatives: renaming the old file or copying it.
diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 0aea1df..b912d7b 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -41,6 +41,7 @@ Files
                                simultaneous editing by two people.
 * Information about Files::  Testing existence, accessibility, size of files.
 * Changing Files::           Renaming files, changing permissions, etc.
+* Files and Storage::        Surviving power and media failures
 * File Names::               Decomposing and expanding file names.
 * Contents of Directories::  Getting a list of the files in a directory.
 * Create/Delete Dirs::       Creating and Deleting Directories.
@@ -1496,6 +1497,10 @@ Changing Files
 system-dependent error message that describes the reason for the
 failure.
 
+  For performance, the operating system may cache or alias changes
+made by these functions instead of writing them immediately to
+secondary storage.  @xref{Files and Storage}.
+
   In the functions that have an argument @var{newname}, if a file by the
 name of @var{newname} already exists, the actions taken depend on the
 value of the argument @var{ok-if-already-exists}:
@@ -1794,6 +1799,26 @@ Changing Files
 @var{filename}, @code{nil} otherwise.
 @end defun
 
+@node Files and Storage
+@section Files and Secondary Storage
+@cindex secondary storage
+
+After Emacs changes a file, there are two reasons the changes might
+not survive later failures of power or media, both having to do with
+efficiency.  First, the operating system might alias written data with
+data already stored elsewhere on secondary storage until one file or
+the other is later modified; this will lose both files if the only
+copy on secondary storage is lost due to media failure.  Second, the
+operating system might not write data to secondary storage
+immediately, which will lose the data if power is lost.
+
+Although both sorts of failures can largely be avoided by a suitably
+configured file system, such systems are typically more expensive or
+less efficient.  In more-typical systems, to survive media failure you
+can copy the file to a different device, and to survive a power
+failure you can invoke the @command{sync} utility (@pxref{sync
+invocation,,, coreutils, The @sc{gnu} @code{Coreutils} Manual}).
+
 @node File Names
 @section File Names
 @cindex file names
-- 
2.7.4

bug#23904: Btrfs clone support in copy operations

[Eli Zaretskii]

> Cc: dmantipov@yandex.ru, 23904-done@debbugs.gnu.org, sbaugh@catern.com,
>  kieran@kcolford.com
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 10 Sep 2016 19:16:39 -0700
> 
> 
> [1:text/plain Hide]
> 
> I looked into this a bit more, and it appears that there's little point to 
> giving the user an option to clone or not clone when copying files, as the 
> cloning (or not cloning) can occur anyway.
> 
> We plan to release a coreutils version soon and to change the default to clone 
> after the release. For Emacs master the next release is far away, so there is 
> plenty of time to try out the FICLONE performance improvement. I installed the 
> attached two patches and am marking this bug report as done.

The first patch is fine with me.

> The second patch merely updates the documentation to discuss the issue; it is 
> logically independent of the first patch, since the issue can occur both with 
> and without the first patch.

I wonder why we need that text in the manual, it doesn't seem to
provide any useful actionable info, just general description of an
issue.

bug#23904: Btrfs clone support in copy operations

[Paul Eggert]

Eli Zaretskii wrote:
> I wonder why we need that text in the manual, it doesn't seem to
> provide any useful actionable info, just general description of an
> issue.

The intent is to say that Emacs does not guarantee that its changes to files 
might not survive power outages and the like. The new section briefly suggests 
actions to take if this is of concern. Feel free to remove this stuff if you 
think it so obvious or out-of-scope that it should not be in the manual.

bug#23904: Btrfs clone support in copy operations

[Eli Zaretskii]

> Cc: dmantipov@yandex.ru, 23904@debbugs.gnu.org, sbaugh@catern.com,
>  kieran@kcolford.com
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 11 Sep 2016 10:17:17 -0700
> 
> Eli Zaretskii wrote:
> > I wonder why we need that text in the manual, it doesn't seem to
> > provide any useful actionable info, just general description of an
> > issue.
> 
> The intent is to say that Emacs does not guarantee that its changes to files 
> might not survive power outages and the like. The new section briefly suggests 
> actions to take if this is of concern. Feel free to remove this stuff if you 
> think it so obvious or out-of-scope that it should not be in the manual.

Does anyone else see this text as useful?  I'm not going to remove it
if someone might find it helpful.

bug#23904: Btrfs clone support in copy operations

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

Come to think of it, the user manual already documents 
write-region-inhibit-fsync, so it was a bit odd that the reference manual was 
silent about it. I installed the attached further patch to document 
write-region-inhibit-fsync in the reference manual, and to help the reference 
manual avoid a dependency on the coreutils manual.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-unnecessary-ref-to-coreutils-manual.patch --]
[-- Type: text/x-diff; name="0001-Remove-unnecessary-ref-to-coreutils-manual.patch", Size: 2107 bytes --]

From a2ce6f5da40e6b19e293588f2872a9fdb06b9bb1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 11 Sep 2016 15:09:04 -0700
Subject: [PATCH] Remove unnecessary ref to coreutils manual

* doc/lispref/files.texi: Document write-region-inhibit-fsync.
---
 doc/lispref/files.texi | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index b912d7b..9a56d0a 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -661,6 +661,15 @@ Writing to Files
 files that the user does not need to know about.
 @end deffn
 
+@defvar write-region-inhibit-fsync
+If this variable's value is @code{nil}, @code{write-region} uses the
+@code{fsync} system call after writing a file.  Although this slows
+Emacs down, it lessens the risk of data loss after power failure.  If
+the value is @code{t}, Emacs does not use @code{fsync}.  The default
+value is @code{nil} when Emacs is interactive, and @code{t} when Emacs
+runs in batch mode.  @xref{Files and Storage}.
+@end defvar
+
 @defmac with-temp-file file body@dots{}
 @anchor{Definition of with-temp-file}
 The @code{with-temp-file} macro evaluates the @var{body} forms with a
@@ -1812,12 +1821,15 @@ Files and Storage
 operating system might not write data to secondary storage
 immediately, which will lose the data if power is lost.
 
+@findex write-region
+@vindex write-region-inhibit-fsync
 Although both sorts of failures can largely be avoided by a suitably
 configured file system, such systems are typically more expensive or
 less efficient.  In more-typical systems, to survive media failure you
 can copy the file to a different device, and to survive a power
-failure you can invoke the @command{sync} utility (@pxref{sync
-invocation,,, coreutils, The @sc{gnu} @code{Coreutils} Manual}).
+failure you can use the @code{write-region} function with the
+@code{write-region-inhibit-fsync} variable set to @code{nil}.
+@xref{Writing to Files}.
 
 @node File Names
 @section File Names
-- 
2.7.4

bug#23904: Btrfs clone support in copy operations

[Eli Zaretskii]

> Cc: dmantipov@yandex.ru, 23904@debbugs.gnu.org, sbaugh@catern.com,
>  kieran@kcolford.com
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 11 Sep 2016 15:44:45 -0700
> 
> Come to think of it, the user manual already documents 
> write-region-inhibit-fsync, so it was a bit odd that the reference manual was 
> silent about it. I installed the attached further patch to document 
> write-region-inhibit-fsync in the reference manual, and to help the reference 
> manual avoid a dependency on the coreutils manual.

Thanks, I think now that subsection makes more sense.

But please remove the explicit @vindex entry, since @defvar already
creates one, and in the correct place.