bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: touch


From: Eric Blake
Subject: Re: touch
Date: Wed, 30 Dec 2009 07:07:04 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.23) Gecko/20090812 Thunderbird/2.0.0.23 Mnenhy/0.7.6.666

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 12/30/2009 5:50 AM:
> So in general, the problem can be characterized precisely by the presence
> of UTIME_OMIT, and can be worked around with a mere stat() (no gettime()
> is needed, since UTIME_NOW still works).  Do you have a full name that
> you'd like to be credited by in THANKS, or do you desire to remain hidden
> behind your pseudonym?  Thanks again for helping us track down and work
> around this bug.

I have tested this on Linux, with xfs.  I will provide a followup patch
for the coreutils NEWS file.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAks7XogACgkQ84KuGfSFAYD2EQCdGSrNfLdcplGJZLRDNOoOsC82
i9sAniVI40TRhW1U2qgcRjEBZiscNxUV
=bqfD
-----END PGP SIGNATURE-----
>From 0e3e69f9f9b31a7d8516bb9699471db6a43bd3c8 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 30 Dec 2009 06:48:46 -0700
Subject: [PATCH] futimens, utimensat: work around ntfs-3g bug

With ntfs-3g, use of a single UTIME_OMIT failed to make any change
to the remaining two timestamps.  Furthermore, the previous fix
for ctime happens to be specific to xfs, rather than global to
the kernel.  Therefore, to be valid, a cache would have to be
per-device, which gets too expensive, especially considering that
the cost of a preparatory stat pulls the file into kernel cache
to speed up the resulting utimensat.  So, blindly massage UTIME_OMIT
on Linux, even on working filesystems like ext4.

The bugs in xfs and ntfs-3g were reported to the kernel folks,
and fixes written, but it will be several years before gnulib
can assume that file systems in use have picked up the fixes.

* lib/utimensat.c (rpl_utimensat): Drop attempts to cache whether
a ctime bug is present, and expand workaround to cover ntfs-3g.
* lib/utimens.c (fdutimens, lutimens): Likewise.
(utimensat_ctime_really, detect_ctime_bug): Drop cache mechanism.
(validate_timespec): Adjust return value.
* m4/futimens.m4 (gl_FUNC_FUTIMENS): Update comment.
* m4/utimensat.m4 (gl_FUNC_UTIMENSAT): Likewise.
Reported by ctrn3e8 <address@hidden>.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog       |   12 +++++
 lib/utimens.c   |  145 +++++++++++++++++--------------------------------------
 lib/utimensat.c |   70 ++++++++++-----------------
 m4/futimens.m4  |    6 ++-
 m4/utimensat.m4 |    6 ++-
 5 files changed, 89 insertions(+), 150 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 79ffe48..cc89881 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2009-12-30  Eric Blake  <address@hidden>
+
+       futimens, utimensat: work around ntfs-3g bug
+       * lib/utimensat.c (rpl_utimensat): Drop attempts to cache whether
+       a ctime bug is present, and expand workaround to cover ntfs-3g.
+       * lib/utimens.c (fdutimens, lutimens): Likewise.
+       (utimensat_ctime_really, detect_ctime_bug): Drop cache mechanism.
+       (validate_timespec): Adjust return value.
+       * m4/futimens.m4 (gl_FUNC_FUTIMENS): Update comment.
+       * m4/utimensat.m4 (gl_FUNC_UTIMENSAT): Likewise.
+       Reported by ctrn3e8 <address@hidden>.
+
 2009-12-29  Eric Blake  <address@hidden>

        link-warning: make usage consistent
diff --git a/lib/utimens.c b/lib/utimens.c
index d0fad37..cc0d1e8 100644
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -67,65 +67,25 @@ struct utimbuf
    utimensat doesn't exist, or is in glibc but kernel 2.6.18 fails with ENOSYS
    kernel 2.6.22 and earlier rejects AT_SYMLINK_NOFOLLOW
    kernel 2.6.25 and earlier reject UTIME_NOW/UTIME_OMIT with non-zero tv_sec
-   kernel 2.6.32 and earlier fail to bump ctime if mtime is UTIME_OMIT
+   kernel 2.6.32 used with xfs or ntfs-3g fail to honor UTIME_OMIT
    utimensat completely works
    For each cache variable: 0 = unknown, 1 = yes, -1 = no.  */
 static int utimensat_works_really;
 static int lutimensat_works_really;
-static int utimensat_ctime_really;
-
-/* Determine whether the kernel has a ctime bug.  ST1 and ST2
-   correspond to stat data before and after a successful time change.
-   TIMES contains the timestamps that were used during the time change
-   (mtime will be UTIME_OMIT).  Update the cache variable if there is
-   conclusive evidence of the kernel working or being buggy.  Return
-   true if TIMES has been updated and another kernel call is needed,
-   whether or not the kernel is known to have the bug.  */
-static bool
-detect_ctime_bug (struct stat *st1, struct stat *st2, struct timespec times[2])
-{
-  struct timespec now;
-  if (st1->st_ctime != st2->st_ctime
-      || get_stat_ctime_ns (st1) != get_stat_ctime_ns (st2))
-    {
-      utimensat_ctime_really = 1;
-      return false;
-    }
-  /* The results are inconclusive if the ctime in st1 is within a file
-     system quantization window of now.  For FAT, this is 2 seconds,
-     for systems with sub-second resolution, a typical resolution is
-     10 milliseconds; to be safe we declare an inconsistent result if
-     ctime is within a 20 millisecond window.  Avoid an extra gettime
-     call if atime makes sense.  It is unlikely that the original
-     ctime is later than now, but rather than deal with the overflow,
-     we treat that as consistent evidence of the bug.  */
-  if (times[0].tv_nsec == UTIME_NOW)
-    now = get_stat_atime (st2);
-  else
-    gettime (&now);
-  if (now.tv_sec < st2->st_ctime
-      || 2 < now.tv_sec - st2->st_ctime
-      || (get_stat_ctime_ns (st2)
-          && now.tv_sec - st2->st_ctime < 2
-          && (20000000 < (1000000000 * (now.tv_sec - st2->st_ctime)
-                          + now.tv_nsec - get_stat_ctime_ns (st2)))))
-    utimensat_ctime_really = -1;
-  times[1] = get_stat_mtime (st2);
-  return true;
-}
 #endif /* HAVE_UTIMENSAT || HAVE_FUTIMENS */

 /* Validate the requested timestamps.  Return 0 if the resulting
    timespec can be used for utimensat (after possibly modifying it to
    work around bugs in utimensat).  Return a positive value if the
    timespec needs further adjustment based on stat results: 1 if any
-   adjustment is needed for utimes, and 2 if mtime was UTIME_OMIT and
-   an adjustment is needed for utimensat.  Return -1, with errno set
-   to EINVAL, if timespec is out of range.  */
+   adjustment is needed for utimes, and 2 if any adjustment is needed
+   for Linux utimensat.  Return -1, with errno set to EINVAL, if
+   timespec is out of range.  */
 static int
 validate_timespec (struct timespec timespec[2])
 {
   int result = 0;
+  int utime_omit_count = 0;
   assert (timespec);
   if ((timespec[0].tv_nsec != UTIME_NOW
        && timespec[0].tv_nsec != UTIME_OMIT
@@ -146,18 +106,18 @@ validate_timespec (struct timespec timespec[2])
     {
       timespec[0].tv_sec = 0;
       result = 1;
+      if (timespec[0].tv_nsec == UTIME_OMIT)
+        utime_omit_count++;
     }
-  if (timespec[1].tv_nsec == UTIME_NOW)
+  if (timespec[1].tv_nsec == UTIME_NOW
+      || timespec[1].tv_nsec == UTIME_OMIT)
     {
       timespec[1].tv_sec = 0;
       result = 1;
+      if (timespec[1].tv_nsec == UTIME_OMIT)
+        utime_omit_count++;
     }
-  else if (timespec[1].tv_nsec == UTIME_OMIT)
-    {
-      timespec[1].tv_sec = 0;
-      result = 2;
-    }
-  return result;
+  return result + (utime_omit_count == 1);
 }

 /* Normalize any UTIME_NOW or UTIME_OMIT values in *TS, using stat
@@ -259,20 +219,26 @@ fdutimens (char const *file, int fd, struct timespec 
const timespec[2])
   if (0 <= utimensat_works_really)
     {
       int result;
-      struct stat st1;
-      struct stat st2;
-      /* Linux kernel 2.6.32 has a bug where it fails to bump ctime if
-         UTIME_OMIT was used for mtime.  It costs time to do an extra
-         [f]stat up front, so we cache whether the function works.  */
-      if (utimensat_ctime_really <= 0 && adjustment_needed == 2)
+# if __linux__
+      struct stat st;
+      /* As recently as Linux kernel 2.6.32 (Dec 2009), several file
+         systems (xfs, ntfs-3g) have bugs with a single UTIME_OMIT,
+         but work if both times are either explicitly specified or
+         UTIME_NOW.  Work around it with a preparatory [f]stat prior
+         to calling futimens/utimensat; fortunately, there is not much
+         timing impact due to the extra syscall even on file systems
+         where UTIME_OMIT would have worked.  FIXME: Simplify this in
+         2012, when file system bugs are no longer common.  */
+      if (adjustment_needed == 2)
         {
-          if (fd < 0 ? stat (file, &st1) : fstat (fd, &st1))
+          if (fd < 0 ? stat (file, &st) : fstat (fd, &st))
             return -1;
           if (ts[0].tv_nsec == UTIME_OMIT)
-            return 0;
-          if (utimensat_ctime_really < 0)
-            ts[1] = get_stat_mtime (&st1);
+            ts[0] = get_stat_atime (&st);
+          else if (ts[1].tv_nsec == UTIME_OMIT)
+            ts[1] = get_stat_mtime (&st);
         }
+# endif /* __linux__ */
 # if HAVE_UTIMENSAT
       if (fd < 0)
         {
@@ -291,16 +257,6 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
           if (result == 0 || errno != ENOSYS)
             {
               utimensat_works_really = 1;
-              if (result == 0 && utimensat_ctime_really == 0
-                  && adjustment_needed == 2)
-                {
-                  /* Perform a followup stat to see if the kernel has
-                     a ctime bug.  */
-                  if (stat (file, &st2))
-                    return -1;
-                  if (detect_ctime_bug (&st1, &st2, ts))
-                    result = utimensat (AT_FDCWD, file, ts, 0);
-                }
               return result;
             }
         }
@@ -316,15 +272,6 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
         if (result == 0 || errno != ENOSYS)
           {
             utimensat_works_really = 1;
-            /* Work around the same bug as above.  */
-            if (result == 0 && utimensat_ctime_really == 0
-                && adjustment_needed == 2)
-              {
-                if (fstat (fd, &st2))
-                  return -1;
-                if (detect_ctime_bug (&st1, &st2, ts))
-                  result = futimens (fd, ts);
-              }
             return result;
           }
       }
@@ -474,20 +421,26 @@ lutimens (char const *file, struct timespec const 
timespec[2])
   if (0 <= lutimensat_works_really)
     {
       int result;
-      struct stat st1;
-      struct stat st2;
-      /* Linux kernel 2.6.32 has a bug where it fails to bump ctime if
-         UTIME_OMIT was used for mtime.  It costs time to do an extra
-         lstat up front, so we cache whether the function works.  */
-      if (utimensat_ctime_really <= 0 && adjustment_needed == 2)
+# if __linux__
+      struct stat st;
+      /* As recently as Linux kernel 2.6.32 (Dec 2009), several file
+         systems (xfs, ntfs-3g) have bugs with a single UTIME_OMIT,
+         but work if both times are either explicitly specified or
+         UTIME_NOW.  Work around it with a preparatory lstat prior to
+         calling utimensat; fortunately, there is not much timing
+         impact due to the extra syscall even on file systems where
+         UTIME_OMIT would have worked.  FIXME: Simplify this in 2012,
+         when file system bugs are no longer common.  */
+      if (adjustment_needed == 2)
         {
-          if (lstat (file, &st1))
+          if (lstat (file, &st))
             return -1;
           if (ts[0].tv_nsec == UTIME_OMIT)
-            return 0;
-          if (utimensat_ctime_really < 0)
-            ts[1] = get_stat_mtime (&st1);
+            ts[0] = get_stat_atime (&st);
+          else if (ts[1].tv_nsec == UTIME_OMIT)
+            ts[1] = get_stat_mtime (&st);
         }
+# endif /* __linux__ */
       result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
 # ifdef __linux__
       /* Work around a kernel bug:
@@ -504,16 +457,6 @@ lutimens (char const *file, struct timespec const 
timespec[2])
         {
           utimensat_works_really = 1;
           lutimensat_works_really = 1;
-          if (result == 0 && utimensat_ctime_really == 0
-              && adjustment_needed == 2)
-            {
-              /* Perform a followup stat to see if the kernel has a
-                 ctime bug.  */
-              if (lstat (file, &st2))
-                return -1;
-              if (detect_ctime_bug (&st1, &st2, ts))
-                result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
-            }
           return result;
         }
     }
diff --git a/lib/utimensat.c b/lib/utimensat.c
index 03e65bf..25ca53c 100644
--- a/lib/utimensat.c
+++ b/lib/utimensat.c
@@ -50,27 +50,38 @@ rpl_utimensat (int fd, char const *file, struct timespec 
const times[2],
 {
   /* See comments in utimens.c for details.  */
   static int utimensat_works_really; /* 0 = unknown, 1 = yes, -1 = no.  */
-  static int utimensat_ctime_really; /* 0 = unknown, 1 = yes, -1 = no.  */
   if (0 <= utimensat_works_really)
     {
       int result;
-      struct stat st1;
-      struct stat st2;
+# ifdef __linux__
+      struct stat st;
       struct timespec ts[2];
-      /* Linux kernel 2.6.32 has a bug where mtime of UTIME_OMIT fails
-         to change ctime.  */
-      if (utimensat_ctime_really <= 0 && times
-          && times[0].tv_nsec != UTIME_OMIT && times[1].tv_nsec == UTIME_OMIT)
+      /* As recently as Linux kernel 2.6.32 (Dec 2009), several file
+         systems (xfs, ntfs-3g) have bugs with a single UTIME_OMIT,
+         but work if both times are either explicitly specified or
+         UTIME_NOW.  Work around it with a preparatory [l]stat prior
+         to calling utimensat; fortunately, there is not much timing
+         impact due to the extra syscall even on file systems where
+         UTIME_OMIT would have worked.  FIXME: Simplify this in 2012,
+         when file system bugs are no longer common.  */
+      if (times && (times[0].tv_nsec == UTIME_OMIT
+                    || times[1].tv_nsec == UTIME_OMIT))
         {
-          if (fstatat (fd, file, &st1, flag))
+          if (fstatat (fd, file, &st, flag))
             return -1;
-          if (utimensat_ctime_really < 0)
-            {
-              ts[0] = times[0];
-              ts[1] = get_stat_mtime (&st1);
-              times = ts;
-            }
+          if (times[0].tv_nsec == UTIME_OMIT && times[1].tv_nsec == UTIME_OMIT)
+            return 0;
+          if (times[0].tv_nsec == UTIME_OMIT)
+            ts[0] = get_stat_atime (&st);
+          else
+            ts[0] = times[0];
+          if (times[1].tv_nsec == UTIME_OMIT)
+            ts[1] = get_stat_mtime (&st);
+          else
+            ts[1] = times[1];
+          times = ts;
         }
+# endif /* __linux__ */
       result = utimensat (fd, file, times, flag);
       /* Linux kernel 2.6.25 has a bug where it returns EINVAL for
          UTIME_NOW or UTIME_OMIT with non-zero tv_sec, which
@@ -82,37 +93,6 @@ rpl_utimensat (int fd, char const *file, struct timespec 
const times[2],
       if (result == 0 || (errno != ENOSYS && errno != EINVAL))
         {
           utimensat_works_really = 1;
-          if (result == 0 && utimensat_ctime_really == 0 && times
-              && times[0].tv_nsec != UTIME_OMIT
-              && times[1].tv_nsec == UTIME_OMIT)
-            {
-              /* Perform a followup [l]stat.  See detect_ctime_bug in
-                 utimens.c for more details.  */
-              struct timespec now;
-              if (fstatat (fd, file, &st2, flag))
-                return -1;
-              if (st1.st_ctime != st2.st_ctime
-                  || get_stat_ctime_ns (&st1) != get_stat_ctime_ns (&st2))
-                {
-                  utimensat_ctime_really = 1;
-                  return result;
-                }
-              if (times[0].tv_nsec == UTIME_NOW)
-                now = get_stat_atime (&st2);
-              else
-                gettime (&now);
-              if (now.tv_sec < st2.st_ctime
-                  || 2 < now.tv_sec - st2.st_ctime
-                  || (get_stat_ctime_ns (&st2)
-                      && now.tv_sec - st2.st_ctime < 2
-                      && (20000000 < (1000000000 * (now.tv_sec - st2.st_ctime)
-                                      + now.tv_nsec
-                                      - get_stat_ctime_ns (&st2)))))
-                utimensat_ctime_really = -1;
-              ts[0] = times[0];
-              ts[1] = get_stat_mtime (&st2);
-              result = utimensat (fd, file, ts, flag);
-            }
           return result;
         }
     }
diff --git a/m4/futimens.m4 b/m4/futimens.m4
index ba5d6b6..3b144c3 100644
--- a/m4/futimens.m4
+++ b/m4/futimens.m4
@@ -1,4 +1,4 @@
-# serial 2
+# serial 3
 # See if we need to provide futimens replacement.

 dnl Copyright (C) 2009 Free Software Foundation, Inc.
@@ -36,10 +36,12 @@ AC_DEFUN([gl_FUNC_FUTIMENS],
       if (fstat (fd, &st)) return 5;
       if (st.st_ctime < st.st_atime) return 6;
       ]])],
+dnl FIXME: simplify this in 2012, when file system bugs are no longer common
          [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 #ifdef __linux__
 /* The Linux kernel added futimens in 2.6.22, but has bugs with UTIME_OMIT
-   in 2.6.32.  Always replace futimens to support older kernels.  */
+   in several file systems as recently as 2.6.32.  Always replace futimens
+   to support older kernels.  */
 choke me
 #endif
       ]])],
diff --git a/m4/utimensat.m4 b/m4/utimensat.m4
index 21e1ea5..edb1709 100644
--- a/m4/utimensat.m4
+++ b/m4/utimensat.m4
@@ -1,4 +1,4 @@
-# serial 2
+# serial 3
 # See if we need to provide utimensat replacement.

 dnl Copyright (C) 2009 Free Software Foundation, Inc.
@@ -35,10 +35,12 @@ AC_DEFUN([gl_FUNC_UTIMENSAT],
       if (utimensat (AT_FDCWD, f, ts, 0)) return 4;
       if (stat (f, &st)) return 5;
       if (st.st_ctime < st.st_atime) return 6;]])],
+dnl FIXME: simplify this in 2012, when file system bugs are no longer common
          [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 #ifdef __linux__
 /* The Linux kernel added utimensat in 2.6.22, but has bugs with UTIME_OMIT
-   in 2.6.32.  Always replace utimensat to support older kernels.  */
+   in several file systems as recently as 2.6.32.  Always replace utimensat
+   to support older kernels.  */
 choke me
 #endif
       ]])],
-- 
1.6.4.2


reply via email to

[Prev in Thread] Current Thread [Next in Thread]