bug-gnulib
[Top][All Lists]
Advanced

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

Re: coreutils-8.2 misc/ls-time test failure


From: Eric Blake
Subject: Re: coreutils-8.2 misc/ls-time test failure
Date: Sat, 19 Dec 2009 17:34:48 -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/17/2009 10:16 PM:
> On further investigation, the problem doesn't appear to be quite as
> pervasive as I thought.  It only happens when mtime is UTIME_OMIT; that
> is, when the call is only requesting a change in atime.  It appears that
> what the kernel is doing is treating it like read(), which modifies atime
> but not ctime.

Here's what I'm finally pushing to gnulib, and a followup to coreutils.
I've also reported a bug report on lkml, but no response so far...
http://lkml.org/lkml/2009/12/18/8

- --
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/

iEYEARECAAYFAkstcSgACgkQ84KuGfSFAYCQzgCfVzkDgN2pTM/47E86oPRhOtxc
S50AoMXmyfhbqCX/kZrlLm7OeLic27Qc
=c03D
-----END PGP SIGNATURE-----
>From 9963a98120addb9fd80299e5242554d62b002217 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 17 Dec 2009 12:30:47 -0700
Subject: [PATCH 1/3] utimens: check for ctime update

futimens/utimensat on Linux fails to bump ctime if mtime is
UTIME_OMIT and atime is specified.

* tests/test-utimens-common.h (check_ctime): Define.
* tests/test-utimens.h (test_utimens): Expose the Linux bug.
* tests/test-futimens.h (test_futimens): Likewise.
* tests/test-lutimens.h (test_lutimens): Likewise.
* doc/posix-functions/futimens.texi (futimens): Document the bug.
* doc/posix-functions/utimensat.texi (utimensat): Likewise.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                          |   10 +++++++
 doc/posix-functions/futimens.texi  |    4 +++
 doc/posix-functions/utimensat.texi |    8 ++++++
 tests/test-futimens.h              |   35 +++++++++++++++++++++++---
 tests/test-lutimens.h              |   48 ++++++++++++++++++++++++++++++-----
 tests/test-utimens-common.h        |   27 +++++++++++++------
 tests/test-utimens.h               |   37 ++++++++++++++++++++++++---
 7 files changed, 144 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 014e8f5..8a38774 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2009-12-19  Eric Blake  <address@hidden>
+
+       utimens: check for ctime update
+       * tests/test-utimens-common.h (check_ctime): Define.
+       * tests/test-utimens.h (test_utimens): Expose the Linux bug.
+       * tests/test-futimens.h (test_futimens): Likewise.
+       * tests/test-lutimens.h (test_lutimens): Likewise.
+       * doc/posix-functions/futimens.texi (futimens): Document the bug.
+       * doc/posix-functions/utimensat.texi (utimensat): Likewise.
+
 2009-12-19  Bruno Haible  <address@hidden>

        dprintf-posix: Check against memory leak fixed on 2009-12-15.
diff --git a/doc/posix-functions/futimens.texi 
b/doc/posix-functions/futimens.texi
index fee3d08..ce71bf4 100644
--- a/doc/posix-functions/futimens.texi
+++ b/doc/posix-functions/futimens.texi
@@ -24,6 +24,10 @@ futimens
 the @code{tv_sec} argument to be 0, and don't necessarily handle all
 file permissions in the manner required by POSIX:
 Linux kernel 2.6.25.
address@hidden
+When using @code{UTIME_OMIT} for the modification time, but specifying
+an access time, some systems fail to update the change time:
+Linux kernel 2.6.32.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/doc/posix-functions/utimensat.texi 
b/doc/posix-functions/utimensat.texi
index 67f5078..85fc218 100644
--- a/doc/posix-functions/utimensat.texi
+++ b/doc/posix-functions/utimensat.texi
@@ -22,10 +22,18 @@ utimensat
 @code{ENOSYS} on some platforms:
 Linux kernel 2.6.21.
 @item
+This function fails with @code{ENOSYS} if passed the flag
address@hidden on a regular file:
+Linux kernel 2.6.22.
address@hidden
 When using @code{UTIME_OMIT} or @code{UTIME_NOW}, some systems require
 the @code{tv_sec} argument to be 0, and don't necessarily handle all
 file permissions in the manner required by POSIX:
 Linux kernel 2.6.25.
address@hidden
+When using @code{UTIME_OMIT} for the modification time, but specifying
+an access time, some systems fail to update the change time:
+Linux kernel 2.6.32.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/tests/test-futimens.h b/tests/test-futimens.h
index 7c05bbf..795aa9e 100644
--- a/tests/test-futimens.h
+++ b/tests/test-futimens.h
@@ -53,6 +53,10 @@ test_futimens (int (*func) (int, struct timespec const *),
      UTIMECMP_TRUNCATE_SOURCE to compensate, with st1 as the
      source.  */
   ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
+  if (check_ctime)
+    ASSERT (st1.st_ctime < st2.st_ctime
+            || (st1.st_ctime == st2.st_ctime
+                && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   {
     /* On some NFS systems, the 'now' timestamp of creat or a NULL
        timespec is determined by the server, but the 'now' timestamp
@@ -101,17 +105,40 @@ test_futimens (int (*func) (int, struct timespec const *),
     ASSERT (st2.st_mtime == Y2K);
     ASSERT (0 <= get_stat_mtime_ns (&st2));
     ASSERT (get_stat_mtime_ns (&st2) < BILLION);
+    if (check_ctime)
+      ASSERT (st1.st_ctime < st2.st_ctime
+              || (st1.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   }

   /* Play with UTIME_OMIT, UTIME_NOW.  */
   {
+    struct stat st3;
     struct timespec ts[2] = { { BILLION, UTIME_OMIT }, { 0, UTIME_NOW } };
+    nap ();
+    ASSERT (func (fd, ts) == 0);
+    ASSERT (fstat (fd, &st3) == 0);
+    ASSERT (st3.st_atime == Y2K);
+    ASSERT (0 <= get_stat_atime_ns (&st3));
+    ASSERT (get_stat_atime_ns (&st3) <= BILLION / 2);
+    ASSERT (utimecmp (BASE "file", &st1, &st3, 0) <= 0);
+    if (check_ctime)
+      ASSERT (st2.st_ctime < st3.st_ctime
+              || (st2.st_ctime == st3.st_ctime
+                  && get_stat_ctime_ns (&st2) < get_stat_ctime_ns (&st3)));
+    nap ();
+    ts[0].tv_nsec = 0;
+    ts[1].tv_nsec = UTIME_OMIT;
     ASSERT (func (fd, ts) == 0);
     ASSERT (fstat (fd, &st2) == 0);
-    ASSERT (st2.st_atime == Y2K);
-    ASSERT (0 <= get_stat_atime_ns (&st2));
-    ASSERT (get_stat_atime_ns (&st2) <= BILLION / 2);
-    ASSERT (utimecmp (BASE "file", &st1, &st2, 0) <= 0);
+    ASSERT (st2.st_atime == BILLION);
+    ASSERT (get_stat_atime_ns (&st2) == 0);
+    ASSERT (st3.st_mtime == st2.st_mtime);
+    ASSERT (get_stat_mtime_ns (&st3) == get_stat_mtime_ns (&st2));
+    if (check_ctime)
+      ASSERT (st3.st_ctime < st2.st_ctime
+              || (st3.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st3) < get_stat_ctime_ns (&st2)));
   }

   /* Cleanup.  */
diff --git a/tests/test-lutimens.h b/tests/test-lutimens.h
index c9302c8..f19df80 100644
--- a/tests/test-lutimens.h
+++ b/tests/test-lutimens.h
@@ -54,11 +54,16 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
   }
   {
     struct timespec ts[2] = { { Y2K, 0 }, { Y2K, 0 } };
+    nap ();
     ASSERT (func (BASE "file", ts) == 0);
   }
-  ASSERT (stat (BASE "file", &st1) == 0);
-  ASSERT (st1.st_atime == Y2K);
-  ASSERT (st1.st_mtime == Y2K);
+  ASSERT (stat (BASE "file", &st2) == 0);
+  ASSERT (st2.st_atime == Y2K);
+  ASSERT (st2.st_mtime == Y2K);
+  if (check_ctime)
+    ASSERT (st1.st_ctime < st2.st_ctime
+            || (st1.st_ctime == st2.st_ctime
+                && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));

   /* Play with symlink timestamps.  */
   if (symlink (BASE "file", BASE "link"))
@@ -98,6 +103,8 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
   if (st1.st_atime != st2.st_atime
       || get_stat_atime_ns (&st1) != get_stat_atime_ns (&st2))
     atime_supported = false;
+  ASSERT (st1.st_ctime == st2.st_ctime);
+  ASSERT (get_stat_ctime_ns (&st1) == get_stat_ctime_ns (&st2));

   /* Invalid arguments.  */
   {
@@ -123,6 +130,7 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
   /* Set both times.  */
   {
     struct timespec ts[2] = { { Y2K, BILLION / 2 - 1 }, { Y2K, BILLION - 1 } };
+    nap ();
     ASSERT (func (BASE "link", ts) == 0);
     ASSERT (lstat (BASE "link", &st2) == 0);
     if (atime_supported)
@@ -134,20 +142,46 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
     ASSERT (st2.st_mtime == Y2K);
     ASSERT (0 <= get_stat_mtime_ns (&st2));
     ASSERT (get_stat_mtime_ns (&st2) < BILLION);
+    if (check_ctime)
+      ASSERT (st1.st_ctime < st2.st_ctime
+              || (st1.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   }

   /* Play with UTIME_OMIT, UTIME_NOW.  */
   {
+    struct stat st3;
     struct timespec ts[2] = { { BILLION, UTIME_OMIT }, { 0, UTIME_NOW } };
+    nap ();
+    ASSERT (func (BASE "link", ts) == 0);
+    ASSERT (lstat (BASE "link", &st3) == 0);
+    if (atime_supported)
+      {
+        ASSERT (st3.st_atime == Y2K);
+        ASSERT (0 <= get_stat_atime_ns (&st3));
+        ASSERT (get_stat_atime_ns (&st3) < BILLION / 2);
+      }
+    ASSERT (utimecmp (BASE "link", &st1, &st3, 0) <= 0);
+    if (check_ctime)
+      ASSERT (st2.st_ctime < st3.st_ctime
+              || (st2.st_ctime == st3.st_ctime
+                  && get_stat_ctime_ns (&st2) < get_stat_ctime_ns (&st3)));
+    nap ();
+    ts[0].tv_nsec = 0;
+    ts[1].tv_nsec = UTIME_OMIT;
     ASSERT (func (BASE "link", ts) == 0);
     ASSERT (lstat (BASE "link", &st2) == 0);
     if (atime_supported)
       {
-        ASSERT (st2.st_atime == Y2K);
-        ASSERT (0 <= get_stat_atime_ns (&st2));
-        ASSERT (get_stat_atime_ns (&st2) < BILLION / 2);
+        ASSERT (st2.st_atime == BILLION);
+        ASSERT (get_stat_atime_ns (&st2) == 0);
       }
-    ASSERT (utimecmp (BASE "link", &st1, &st2, 0) <= 0);
+    ASSERT (st3.st_mtime == st2.st_mtime);
+    ASSERT (get_stat_mtime_ns (&st3) == get_stat_mtime_ns (&st2));
+    if (check_ctime)
+      ASSERT (st3.st_ctime < st2.st_ctime
+              || (st3.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st3) < get_stat_ctime_ns (&st2)));
   }

   /* Symlink to directory.  */
diff --git a/tests/test-utimens-common.h b/tests/test-utimens-common.h
index 6c404cc..707971a 100644
--- a/tests/test-utimens-common.h
+++ b/tests/test-utimens-common.h
@@ -19,14 +19,14 @@
 #ifndef GL_TEST_UTIMENS_COMMON
 # define GL_TEST_UTIMENS_COMMON

-#include <fcntl.h>
-#include <errno.h>
-#include <string.h>
-#include <unistd.h>
+# include <fcntl.h>
+# include <errno.h>
+# include <string.h>
+# include <unistd.h>

-#include "stat-time.h"
-#include "timespec.h"
-#include "utimecmp.h"
+# include "stat-time.h"
+# include "timespec.h"
+# include "utimecmp.h"

 enum {
   BILLION = 1000 * 1000 * 1000,
@@ -62,9 +62,18 @@ nap (void)
      a quantization boundary equal to the resolution.  Our usage of
      utimecmp allows equality, so no need to waste 980 milliseconds
      if the replacement usleep rounds to 1 second.  */
-#if HAVE_USLEEP
+# if HAVE_USLEEP
   usleep (20 * 1000); /* 20 milliseconds.  */
-#endif
+# endif
 }

+# if (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__
+/* Skip ctime tests on native Windows, since it is either a copy of
+   mtime or birth time (depending on the file system), rather than a
+   properly tracked change time.  */
+#  define check_ctime 0
+# else
+#  define check_ctime 1
+# endif
+
 #endif /* GL_TEST_UTIMENS_COMMON */
diff --git a/tests/test-utimens.h b/tests/test-utimens.h
index 710741a..b211b41 100644
--- a/tests/test-utimens.h
+++ b/tests/test-utimens.h
@@ -37,6 +37,10 @@ test_utimens (int (*func) (char const *, struct timespec 
const *), bool print)
   ASSERT (func (BASE "file", NULL) == 0);
   ASSERT (stat (BASE "file", &st2) == 0);
   ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
+  if (check_ctime)
+    ASSERT (st1.st_ctime < st2.st_ctime
+            || (st1.st_ctime == st2.st_ctime
+                && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   {
     /* On some NFS systems, the 'now' timestamp of creat or a NULL
        timespec is determined by the server, but the 'now' timestamp
@@ -97,18 +101,41 @@ test_utimens (int (*func) (char const *, struct timespec 
const *), bool print)
     ASSERT (st2.st_mtime == Y2K);
     ASSERT (0 <= get_stat_mtime_ns (&st2));
     ASSERT (get_stat_mtime_ns (&st2) < BILLION);
+    if (check_ctime)
+      ASSERT (st1.st_ctime < st2.st_ctime
+              || (st1.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st1) < get_stat_ctime_ns (&st2)));
   }

   /* Play with UTIME_OMIT, UTIME_NOW.  */
   {
+    struct stat st3;
     struct timespec ts[2] = { { BILLION, UTIME_OMIT }, { 0, UTIME_NOW } };
+    nap ();
     ASSERT (func (BASE "file", ts) == 0);
-    ASSERT (stat (BASE "file", &st2) == 0);
-    ASSERT (st2.st_atime == Y2K);
-    ASSERT (0 <= get_stat_atime_ns (&st2));
-    ASSERT (get_stat_atime_ns (&st2) < BILLION / 2);
+    ASSERT (stat (BASE "file", &st3) == 0);
+    ASSERT (st3.st_atime == Y2K);
+    ASSERT (0 <= get_stat_atime_ns (&st3));
+    ASSERT (get_stat_atime_ns (&st3) < BILLION / 2);
     /* See comment above about this utimecmp call.  */
-    ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
+    ASSERT (0 <= utimecmp (BASE "file", &st3, &st1, UTIMECMP_TRUNCATE_SOURCE));
+    if (check_ctime)
+      ASSERT (st2.st_ctime < st3.st_ctime
+              || (st2.st_ctime == st3.st_ctime
+                  && get_stat_ctime_ns (&st2) < get_stat_ctime_ns (&st3)));
+    nap ();
+    ts[0].tv_nsec = 0;
+    ts[1].tv_nsec = UTIME_OMIT;
+    ASSERT (func (BASE "file", ts) == 0);
+    ASSERT (stat (BASE "file", &st2) == 0);
+    ASSERT (st2.st_atime == BILLION);
+    ASSERT (get_stat_atime_ns (&st2) == 0);
+    ASSERT (st3.st_mtime == st2.st_mtime);
+    ASSERT (get_stat_mtime_ns (&st3) == get_stat_mtime_ns (&st2));
+    if (check_ctime)
+      ASSERT (st3.st_ctime < st2.st_ctime
+              || (st3.st_ctime == st2.st_ctime
+                  && get_stat_ctime_ns (&st3) < get_stat_ctime_ns (&st2)));
   }

   /* Make sure this dereferences symlinks.  */
-- 
1.6.5.rc1


>From 68f0f1af0f2cc75615d89234494dca043e99a73d Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 17 Dec 2009 16:57:37 -0700
Subject: [PATCH 2/3] utimens: work around Linux ctime bug

Force a ctime update by using stat() before any utimensat call
with mtime of UTIME_OMIT.  But avoid extra stat()s in later
calls, by doing extra work on the first instance in order to
cache whether the bug is actually present.

* lib/utimens.c (detect_ctime_bug): New helper function.
(update_timespec): Differentiate between workaround needed for
this bug vs. what is needed for systems that lack utimensat.
(fdutimens, lutimens): Work around bug.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog     |    6 ++
 lib/utimens.c |  153 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 139 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8a38774..04bdbcc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-12-19  Eric Blake  <address@hidden>

+       utimens: work around Linux ctime bug
+       * lib/utimens.c (detect_ctime_bug): New helper function.
+       (update_timespec): Differentiate between workaround needed for
+       this bug vs. what is needed for systems that lack utimensat.
+       (fdutimens, lutimens): Work around bug.
+
        utimens: check for ctime update
        * tests/test-utimens-common.h (check_ctime): Define.
        * tests/test-utimens.h (test_utimens): Expose the Linux bug.
diff --git a/lib/utimens.c b/lib/utimens.c
index d8e2f32..915f1e7 100644
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -53,27 +53,76 @@ struct utimbuf
 #undef futimens
 #undef utimensat

-#if HAVE_UTIMENSAT || HAVE_FUTIMENS
-/* Cache variables for whether the utimensat syscall works; used to
-   avoid calling the syscall if we know it will just fail with ENOSYS.
-   There are some Linux kernel versions where a flag of 0 passes, but
-   not AT_SYMLINK_NOFOLLOW.  0 = unknown, 1 = yes, -1 = no.  */
-static int utimensat_works_really;
-static int lutimensat_works_really;
-#endif /* HAVE_UTIMENSAT || HAVE_UTIMENSAT */
-
 /* Solaris 9 mistakenly succeeds when given a non-directory with a
    trailing slash.  Force the use of rpl_stat for a fix.  */
 #ifndef REPLACE_FUNC_STAT_FILE
 # define REPLACE_FUNC_STAT_FILE 0
 #endif

+#if HAVE_UTIMENSAT || HAVE_FUTIMENS
+/* Cache variables for whether the utimensat syscall works; used to
+   avoid calling the syscall if we know it will just fail with ENOSYS,
+   and to avoid unnecessary work in massaging timestamps if the
+   syscall will work.  Multiple variables are needed, to distinguish
+   between the following scenarios on Linux:
+   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
+   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 1 if the timespec needs
-   further adjustment based on stat results for utimes or other less
-   powerful interfaces.  Return -1, with errno set to EINVAL, if
-   timespec is out of range.  */
+   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.  */
 static int
 validate_timespec (struct timespec timespec[2])
 {
@@ -90,20 +139,25 @@ validate_timespec (struct timespec timespec[2])
       return -1;
     }
   /* Work around Linux kernel 2.6.25 bug, where utimensat fails with
-     EINVAL if tv_sec is not 0 when using the flag values of
-     tv_nsec.  */
+     EINVAL if tv_sec is not 0 when using the flag values of tv_nsec.
+     Flag a Linux kernel 2.6.32 bug, where an mtime of UTIME_OMIT
+     fails to bump ctime.  */
   if (timespec[0].tv_nsec == UTIME_NOW
       || timespec[0].tv_nsec == UTIME_OMIT)
     {
       timespec[0].tv_sec = 0;
       result = 1;
     }
-  if (timespec[1].tv_nsec == UTIME_NOW
-      || timespec[1].tv_nsec == UTIME_OMIT)
+  if (timespec[1].tv_nsec == UTIME_NOW)
     {
       timespec[1].tv_sec = 0;
       result = 1;
     }
+  else if (timespec[1].tv_nsec == UTIME_OMIT)
+    {
+      timespec[1].tv_sec = 0;
+      result = 2;
+    }
   return result;
 }

@@ -205,10 +259,25 @@ fdutimens (char const *file, int fd, struct timespec 
const timespec[2])
 #if HAVE_UTIMENSAT || HAVE_FUTIMENS
   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 (fd < 0 ? stat (file, &st1) : fstat (fd, &st1))
+            return -1;
+          if (ts[0].tv_nsec == UTIME_OMIT)
+            return 0;
+          if (utimensat_ctime_really < 0)
+            ts[1] = get_stat_mtime (&st1);
+        }
 # if HAVE_UTIMENSAT
       if (fd < 0)
         {
-          int result = utimensat (AT_FDCWD, file, ts, 0);
+          result = utimensat (AT_FDCWD, file, ts, 0);
 #  ifdef __linux__
           /* Work around a kernel bug:
              http://bugzilla.redhat.com/442352
@@ -223,13 +292,23 @@ 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;
             }
         }
 # endif /* HAVE_UTIMENSAT */
 # if HAVE_FUTIMENS
       {
-        int result = futimens (fd, ts);
+        result = futimens (fd, ts);
 #  ifdef __linux__
         /* Work around the same bug as above.  */
         if (0 < result)
@@ -238,6 +317,15 @@ 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;
           }
       }
@@ -386,7 +474,22 @@ lutimens (char const *file, struct timespec const 
timespec[2])
 #if HAVE_UTIMENSAT
   if (0 <= lutimensat_works_really)
     {
-      int result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
+      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 (lstat (file, &st1))
+            return -1;
+          if (ts[0].tv_nsec == UTIME_OMIT)
+            return 0;
+          if (utimensat_ctime_really < 0)
+            ts[1] = get_stat_mtime (&st1);
+        }
+      result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
 # ifdef __linux__
       /* Work around a kernel bug:
          http://bugzilla.redhat.com/442352
@@ -402,6 +505,16 @@ 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;
         }
     }
-- 
1.6.5.rc1


>From 9a669cf64253a2b2149d7f7cc5e0664c1bc7dda9 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 17 Dec 2009 07:32:00 -0700
Subject: [PATCH 3/3] futimens, utimensat: work around Linux bug

futimens is trivial - let fdutimens do the work.  utimensat
is tougher: we don't want to call into local_utimensat,
because that can cause unnecessary chdir.  So we have to
repeat the logic from utimens.c.

* m4/futimens.m4 (gl_FUNC_FUTIMENS): Detect ctime bug.
* m4/utimensat.m4 (gl_FUNC_UTIMENSAT): Likewise.
* lib/utimensat.c (rpl_utimensat): Work around it.
* lib/futimens.c (rpl_futimens): Adjust comment.
---
 ChangeLog       |    6 +++++
 lib/futimens.c  |    2 +-
 lib/utimensat.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 m4/futimens.m4  |   33 ++++++++++++++++++++-------
 m4/utimensat.m4 |   28 ++++++++++++++++++------
 5 files changed, 111 insertions(+), 22 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 04bdbcc..eb560d5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-12-19  Eric Blake  <address@hidden>

+       futimens, utimensat: work around Linux bug
+       * m4/futimens.m4 (gl_FUNC_FUTIMENS): Detect ctime bug.
+       * m4/utimensat.m4 (gl_FUNC_UTIMENSAT): Likewise.
+       * lib/utimensat.c (rpl_utimensat): Work around it.
+       * lib/futimens.c (rpl_futimens): Adjust comment.
+
        utimens: work around Linux ctime bug
        * lib/utimens.c (detect_ctime_bug): New helper function.
        (update_timespec): Differentiate between workaround needed for
diff --git a/lib/futimens.c b/lib/futimens.c
index 031a464..e93ffc4 100644
--- a/lib/futimens.c
+++ b/lib/futimens.c
@@ -32,6 +32,6 @@ futimens (int fd, struct timespec const times[2])
 {
   /* fdutimens also works around bugs in native futimens, when running
      with glibc compiled against newer headers but on a Linux kernel
-     older than 2.6.26.  */
+     older than 2.6.32.  */
   return fdutimens (NULL, fd, times);
 }
diff --git a/lib/utimensat.c b/lib/utimensat.c
index 3808439..03e65bf 100644
--- a/lib/utimensat.c
+++ b/lib/utimensat.c
@@ -23,6 +23,8 @@
 #include <errno.h>
 #include <fcntl.h>

+#include "stat-time.h"
+#include "timespec.h"
 #include "utimens.h"

 #if HAVE_UTIMENSAT
@@ -31,10 +33,11 @@

 /* If we have a native utimensat, but are compiling this file, then
    utimensat was defined to rpl_utimensat by our replacement
-   sys/stat.h.  We assume the native version might fail with ENOSYS
-   (as is the case when using newer glibc but older Linux kernel).  In
-   this scenario, rpl_utimensat checks whether the native version is
-   usable, and local_utimensat provides the fallback manipulation.  */
+   sys/stat.h.  We assume the native version might fail with ENOSYS,
+   or succeed without properly affecting ctime (as is the case when
+   using newer glibc but older Linux kernel).  In this scenario,
+   rpl_utimensat checks whether the native version is usable, and
+   local_utimensat provides the fallback manipulation.  */

 static int local_utimensat (int, char const *, struct timespec const[2], int);
 # define AT_FUNC_NAME local_utimensat
@@ -45,10 +48,30 @@ int
 rpl_utimensat (int fd, char const *file, struct timespec const times[2],
                int flag)
 {
+  /* 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 = utimensat (fd, file, times, flag);
+      int result;
+      struct stat st1;
+      struct stat st2;
+      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)
+        {
+          if (fstatat (fd, file, &st1, flag))
+            return -1;
+          if (utimensat_ctime_really < 0)
+            {
+              ts[0] = times[0];
+              ts[1] = get_stat_mtime (&st1);
+              times = ts;
+            }
+        }
+      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
          local_utimensat works around.  Meanwhile, EINVAL for a bad
@@ -59,6 +82,37 @@ 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 0547e7a..ba5d6b6 100644
--- a/m4/futimens.m4
+++ b/m4/futimens.m4
@@ -1,4 +1,4 @@
-# serial 1
+# serial 2
 # See if we need to provide futimens replacement.

 dnl Copyright (C) 2009 Free Software Foundation, Inc.
@@ -22,17 +22,32 @@ AC_DEFUN([gl_FUNC_FUTIMENS],
       [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
 #include <fcntl.h>
 #include <sys/stat.h>
+#include <unistd.h>
+]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_NOW } };
+      int fd = creat ("conftest.file", 0600);
+      struct stat st;
+      if (fd < 0) return 1;
+      if (futimens (AT_FDCWD, NULL)) return 2;
+      if (futimens (fd, ts)) return 3;
+      sleep (1);
+      ts[0].tv_nsec = UTIME_NOW;
+      ts[1].tv_nsec = UTIME_OMIT;
+      if (futimens (fd, ts)) return 4;
+      if (fstat (fd, &st)) return 5;
+      if (st.st_ctime < st.st_atime) return 6;
+      ]])],
+         [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 #ifdef __linux__
-/* The Linux kernel added futimens in 2.6.22, but it had bugs until 2.6.26.
-   Always replace futimens to support older kernels.  */
+/* 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.  */
 choke me
 #endif
-]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_OMIT } };
-      if (futimens (AT_FDCWD, NULL)) return 1;
-      return futimens (open (".", O_RDONLY), ts);]])],
-         [gl_cv_func_futimens_works=yes],
-         [gl_cv_func_futimens_works="needs runtime check"],
-         [gl_cv_func_futimens_works="guessing no"])])
+      ]])],
+           [gl_cv_func_futimens_works=yes],
+           [gl_cv_func_futimens_works="needs runtime check"])],
+         [gl_cv_func_futimens_works=no],
+         [gl_cv_func_futimens_works="guessing no"])
+      rm -f conftest.file])
     if test "$gl_cv_func_futimens_works" != yes; then
       REPLACE_FUTIMENS=1
       AC_LIBOBJ([futimens])
diff --git a/m4/utimensat.m4 b/m4/utimensat.m4
index 2a6033f..21e1ea5 100644
--- a/m4/utimensat.m4
+++ b/m4/utimensat.m4
@@ -1,4 +1,4 @@
-# serial 1
+# serial 2
 # See if we need to provide utimensat replacement.

 dnl Copyright (C) 2009 Free Software Foundation, Inc.
@@ -22,15 +22,29 @@ AC_DEFUN([gl_FUNC_UTIMENSAT],
       [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
 #include <fcntl.h>
 #include <sys/stat.h>
+#include <unistd.h>
+]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_NOW } };
+      struct stat st;
+      const char *f = "conftest.file";
+      if (close (creat (f, 0600))) return 1;
+      if (utimensat (AT_FDCWD, f, NULL, AT_SYMLINK_NOFOLLOW)) return 2;
+      if (utimensat (AT_FDCWD, f, ts, 0)) return 3;
+      sleep (1);
+      ts[0].tv_nsec = UTIME_NOW;
+      ts[1].tv_nsec = UTIME_OMIT;
+      if (utimensat (AT_FDCWD, f, ts, 0)) return 4;
+      if (stat (f, &st)) return 5;
+      if (st.st_ctime < st.st_atime) return 6;]])],
+         [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 #ifdef __linux__
-/* The Linux kernel added utimensat in 2.6.22, but it had bugs until 2.6.26.
-   Always replace utimensat to support older kernels.  */
+/* 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.  */
 choke me
 #endif
-]], [[struct timespec ts[2] = { { 1, UTIME_OMIT }, { 1, UTIME_OMIT } };
-      return utimensat (AT_FDCWD, ".", NULL, AT_SYMLINK_NOFOLLOW);]])],
-         [gl_cv_func_utimensat_works=yes],
-         [gl_cv_func_utimensat_works="needs runtime check"],
+      ]])],
+           [gl_cv_func_utimensat_works=yes],
+           [gl_cv_func_utimensat_works="needs runtime check"])],
+         [gl_cv_func_utimensat_works=no],
          [gl_cv_func_utimensat_works="guessing no"])])
     if test "$gl_cv_func_utimensat_works" != yes; then
       REPLACE_UTIMENSAT=1
-- 
1.6.5.rc1

>From 7efd3517b80c023e95c8af82724b2d0f41585cc3 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 19 Dec 2009 17:29:40 -0700
Subject: [PATCH] touch: fix ctime regression in 'touch -a'

Regression introduced in coreutils 8.1 due to a bug in the Linux
kernel implementation of utimensat with mtime of UTIME_OMIT.

* gnulib: Update to latest, to pick up utimensat fix.
* NEWS: Mention the change.
* THANKS: Update.
Reported by John Stanley.
---
 NEWS   |    4 ++++
 gnulib |    2 +-
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/NEWS b/NEWS
index f0b71a8..e0287cc 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   a commmand of the above form would fail for all subdirectories.
   [bug introduced in coreutils-8.0]

+  touch -a once again guarantees that a file's change time is
+  adjusted, working around a bug in current Linux kernels.
+  [bug introduced in coreutils-8.1]
+

 * Noteworthy changes in release 8.2 (2009-12-11) [stable]

diff --git a/gnulib b/gnulib
index 5016c20..9a669cf 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 5016c2066bfca00f5a24e0d2abaca25f0fce75fb
+Subproject commit 9a669cf64253a2b2149d7f7cc5e0664c1bc7dda9
-- 
1.6.5.rc1


reply via email to

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