`write-region-inhibit-fsync' defaults to noninteractive.

* cmdargs.texi (Initial Options):
* files.texi (Customize Save): Document this.
* etc/NEWS: Document this.
* src/fileio.c (syms_of_fileio): Implement this.
* src/filelock.c (create_lock_file): If symbolic links don't work, so
we use a regular file as a lock file, do not fsync the lock file;
it's not needed.

Fixes: debbugs:14273

8 files changed, 73 insertions, 25 deletions

diff --git a/doc/emacs/ChangeLog b/doc/emacs/ChangeLog
index 2c47bdd4fad..348100c8408 100644
--- a/doc/emacs/ChangeLog
+++ b/doc/emacs/ChangeLog
@@ -1,3 +1,9 @@
+2013-05-05  Paul Eggert  <eggert@cs.ucla.edu>
+
+        `write-region-inhibit-fsync' defaults to noninteractive (Bug#14273).
+        * cmdargs.texi (Initial Options):
+        * files.texi (Customize Save): Document this.
+
 2013-05-04  Glenn Morris  <rgm@gnu.org>
 
         * calendar.texi (Importing Diary): Mention diary-from-outlook-function.
diff --git a/doc/emacs/cmdargs.texi b/doc/emacs/cmdargs.texi
index 19b439afc7f..e285104b5de 100644
--- a/doc/emacs/cmdargs.texi
+++ b/doc/emacs/cmdargs.texi
@@ -251,7 +251,8 @@ terminal's standard input stream (@code{stdin}) instead.
 but @file{site-start.el} is loaded nonetheless.  It also causes Emacs
 to exit after processing all the command options.  In addition, it
 disables auto-saving except in buffers for which auto-saving is
-explicitly requested.
+explicitly requested, and when saving files it omits the @code{fsync}
+system call unless otherwise requested.
 
 @item --script @var{file}
 @opindex --script
diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index 1f78747eaa6..13fa516af66 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -705,13 +705,27 @@ setting the latter variable, you can control how these modes handle
 final newlines.
 
 @vindex write-region-inhibit-fsync
-  When Emacs saves a file, it invokes the @code{fsync} system call to
-force the data immediately out to disk.  This is important for safety
-if the system crashes or in case of power outage.  However, it can be
-disruptive on laptops using power saving, as it may force a disk
-spin-up each time you save a file.  If you accept an increased risk of
-data loss, you can set @code{write-region-inhibit-fsync} to a
-non-@code{nil} value to disable the synchronization.
+  Normally, when a program writes a file, the operating system briefly
+caches the file's data in main memory before committing the data to
+disk.  This can greatly improve performance; for example, when running
+on laptops, it can avoid a disk spin-up each time a file is written.
+However, it risks data loss if the operating system crashes before
+committing the cache to disk.
+
+  To lessen this risk, Emacs can invoke the @code{fsync} system call
+after saving a file.  Using @code{fsync} does not eliminate the risk
+of data loss, partly because many systems do not implement
+@code{fsync} properly, and partly because Emacs's file-saving
+procedure typically relies also on directory updates that might not
+survive a crash even if @code{fsync} works properly.
+
+  The @code{write-region-inhibit-fsync} variable controls whether
+Emacs invokes @code{fsync} after saving a file.  The variable's
+default value is @code{nil} when Emacs is interactive, and @code{t}
+when Emacs runs in batch mode.
+
+  Emacs never uses @code{fsync} when writing auto-save files, as these
+files might lose data anyway.
 
 @node Interlocking
 @subsection Protection against Simultaneous Editing
diff --git a/etc/ChangeLog b/etc/ChangeLog
index afad0864c84..9cbd67c209f 100644
--- a/etc/ChangeLog
+++ b/etc/ChangeLog
@@ -1,3 +1,8 @@
+2013-05-05  Paul Eggert  <eggert@cs.ucla.edu>
+
+        `write-region-inhibit-fsync' defaults to noninteractive (Bug#14273).
+        * NEWS: Document this.
+
 2013-04-24  Tassilo Horn  <tsdh@gnu.org>
 
         * themes/tsdh-dark-theme.el (tsdh-dark): Add ido faces and remove
diff --git a/etc/NEWS b/etc/NEWS
index 4669021c472..13cdb8273d4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -61,6 +61,8 @@ simply disabling Transient Mark mode does the same thing.
 ** `initial-buffer-choice' can now specify a function to set up the
 initial buffer.
 
+** `write-region-inhibit-fsync' now defaults to t in batch mode.
+
 ** ACL support has been added.
 +++
 *** Emacs preserves the ACL entries of files when backing up.
diff --git a/src/ChangeLog b/src/ChangeLog
index 86327fc7fb2..a3e466954cb 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,11 @@
+2013-05-05  Paul Eggert  <eggert@cs.ucla.edu>
+
+        `write-region-inhibit-fsync' defaults to noninteractive (Bug#14273).
+        * fileio.c (syms_of_fileio): Implement this.
+        * filelock.c (create_lock_file): If symbolic links don't work, so
+        we use a regular file as a lock file, do not fsync the lock file;
+        it's not needed.
+
 2013-05-04  Stefan Monnier  <monnier@iro.umontreal.ca>
 
         * minibuf.c (Fread_minibuffer, Feval_minibuffer): Move to Elisp.
@@ -51,6 +59,7 @@
         size.
 
 2013-04-26  Paul Eggert  <eggert@cs.ucla.edu>
+
         Port better to AIX (Bug#14258).
         * lisp.h (ENUM_BF) [__IBMC__]: Make it 'unsigned int' here, too,
         to pacify AIX xlc.
diff --git a/src/fileio.c b/src/fileio.c
index 1049522e5a9..38b98a243b2 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4979,15 +4979,14 @@ This calls `write-region-annotate-functions' at the start, and
 
   immediate_quit = 0;
 
-  /* fsync appears to change the modtime on BSD4.2.
-     Disk full in NFS may be reported here.  */
-  /* mib says that closing the file will try to write as fast as NFS can do
-     it, and that means the fsync here is not crucial for autosave files.  */
+  /* fsync is not crucial for auto-save files, since they might lose
+     some work anyway.  */
   if (!auto_saving && !write_region_inhibit_fsync)
     {
-      /* Transfer data and metadata to disk, retrying if interrupted.  Also,
-         ignore EINVAL which happens when fsync is not supported on this
-         file.  */
+      /* Transfer data and metadata to disk, retrying if interrupted.
+         fsync can report a write failure here, e.g., due to disk full
+         under NFS.  But ignore EINVAL, which means fsync is not
+         supported on this file.  */
       while (fsync (desc) != 0)
         if (errno != EINTR)
           {
@@ -6069,11 +6068,29 @@ in the buffer; this is the default behavior, because the auto-save
 file is usually more useful if it contains the deleted text.  */);
   Vauto_save_include_big_deletions = Qnil;
 
+  /* fsync can be a significant performance hit.  Often it doesn't
+     suffice to make the file-save operation survive a crash.  For
+     batch scripts, which are typically part of larger shell commands
+     that don't fsync other files, its effect on performance can be
+     significant so its utility is particularly questionable.
+     Hence, for now by default fsync is used only when interactive.
+
+     For more on why fsync often fails to work on today's hardware, see:
+     Zheng M, Tucek J, Qin F, Lillibridge M. Understanding the
+     robustness of SSDs under power fault. 11th USENIX Conference on
+     File and Storage Technologies, 2013 (FAST '13), 271-84
+     http://www.usenix.org/system/files/conference/fast13/fast13-final80.pdf
+
+     For more on why fsync does not suffice even if it works properly, see:
+     Roche X. Necessary step(s) to synchronize filename operations on disk.
+     Austin Group Defect 672, 2013-03-19
+     http://austingroupbugs.net/view.php?id=672  */
   DEFVAR_BOOL ("write-region-inhibit-fsync", write_region_inhibit_fsync,
                doc: /* Non-nil means don't call fsync in `write-region'.
 This variable affects calls to `write-region' as well as save commands.
-A non-nil value may result in data loss!  */);
-  write_region_inhibit_fsync = 0;
+Setting this to nil may avoid data loss if the system loses power or
+the operating system crashes.  */);
+  write_region_inhibit_fsync = noninteractive;
 
   DEFVAR_BOOL ("delete-by-moving-to-trash", delete_by_moving_to_trash,
                doc: /* Specifies whether to use the system's trash can.
diff --git a/src/filelock.c b/src/filelock.c
index f17d3182eab..de6aba8385c 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -437,14 +437,8 @@ create_lock_file (char *lfname, char *lock_info_str, bool force)
           if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len
               || (need_fchmod && fchmod (fd, world_readable) != 0))
             err = errno;
-          else
-            while (fsync (fd) != 0)
-              if (errno != EINTR)
-                {
-                  if (errno != EINVAL)
-                    err = errno;
-                  break;
-                }
+          /* There is no need to call fsync here, as the contents of
+             the lock file need not survive system crashes.  */
           if (emacs_close (fd) != 0)
             err = errno;
           if (!err && rename_lock_file (nonce, lfname, force) != 0)