[PATCH] utimensat implementation

[PATCH] utimensat implementation

[Ulrich Drepper]

The next revision of POSIX will support fine-grained filesystem timestamps the way we already support.  struct stat will report nanosecond values.  So far so good.

During the development one additional problem was found: there is no interface to set the file timestamp with that precision.  utimes only takes a timeval structure which allows only micro-second resolution.

This is why the utimensat() interface was created.  It is basically the same as our futimesat() interface but it takes a timespec structure.

While adding this new interface two more features got added.  Programmers sometimes/often only want to set one value.  This currently requires reading the current value with stat() and then use this value in the futimesat() call.  This is slow and might create even wrong results (the file could have been updated in the meantime).  If the tv_nsec value of either of the elements of the utimes parameter to utimensat() is UTIME_OMIT no update of that respective value is performed.

The second extension allows to set one of the values to the correct current time.  Today it is only possible to set both values at the same time.  Once again this is slower and might to lead to incorrect results.  The use of UTIME_NOW for the respective tv_nsec value implements this functionality.

The resulting patch is attached.  It modifies the do_utimes function which is also used in the compat code.  The callers are adjusted.  Most of the added code are checks for invalid parameters.  In fact, I think one problem where the old code wouldn't recognize certain overflows (if tv_nsec * 1000 overflows).

I've tested the code on x86-64.

Signed-off-by: Ulrich Drepper <drepper@redhat.com>

diff --git a/arch/x86_64/ia32/ia32entry.S b/arch/x86_64/ia32/ia32entry.S
index 796df69..33121e2 100644
--- a/arch/x86_64/ia32/ia32entry.S
+++ b/arch/x86_64/ia32/ia32entry.S
@@ -714,9 +714,10 @@ ia32_sys_call_table:
 	.quad compat_sys_get_robust_list
 	.quad sys_splice
 	.quad sys_sync_file_range
-	.quad sys_tee
+	.quad sys_tee			/* 315 */
 	.quad compat_sys_vmsplice
 	.quad compat_sys_move_pages
 	.quad sys_getcpu
 	.quad sys_epoll_pwait
+	.quad sys_utimensat		/* 320 */
 ia32_syscall_end:		
diff --git a/fs/compat.c b/fs/compat.c
index 040a8be..1644cd1 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -79,28 +79,55 @@ int compat_printk(const char *fmt, ...)
  */
 asmlinkage long compat_sys_utime(char __user *filename, struct compat_utimbuf __user *t)
 {
-	struct timeval tv[2];
+	struct timespec tv[2];
 
 	if (t) {
 		if (get_user(tv[0].tv_sec, &t->actime) ||
 		    get_user(tv[1].tv_sec, &t->modtime))
 			return -EFAULT;
-		tv[0].tv_usec = 0;
-		tv[1].tv_usec = 0;
+		tv[0].tv_nsec = 0;
+		tv[1].tv_nsec = 0;
 	}
 	return do_utimes(AT_FDCWD, filename, t ? tv : NULL);
 }
 
+asmlinkage long compat_sys_utimensat(unsigned int dfd, char __user *filename, struct compat_timespec __user *t)
+{
+	struct timespec tv[2];
+
+	if  (t) {
+		if (get_compat_timespec(&tv[0], &t[0]) ||
+		    get_compat_timespec(&tv[1], &t[1]))
+			return -EFAULT;
+
+		if ((tv[0].tv_nsec == UTIME_OMIT || tv[0].tv_nsec == UTIME_NOW)
+		    && tv[0].tv_sec != 0)
+			return -EINVAL;
+		if ((tv[1].tv_nsec == UTIME_OMIT || tv[1].tv_nsec == UTIME_NOW)
+		    && tv[1].tv_sec != 0)
+			return -EINVAL;
+
+		if (tv[0].tv_nsec == UTIME_OMIT && tv[1].tv_nsec == UTIME_OMIT)
+			return 0;
+	}
+	return do_utimes(dfd, filename, t ? tv : NULL);
+}
+
 asmlinkage long compat_sys_futimesat(unsigned int dfd, char __user *filename, struct compat_timeval __user *t)
 {
-	struct timeval tv[2];
+	struct timespec tv[2];
 
 	if (t) {
 		if (get_user(tv[0].tv_sec, &t[0].tv_sec) ||
-		    get_user(tv[0].tv_usec, &t[0].tv_usec) ||
+		    get_user(tv[0].tv_nsec, &t[0].tv_usec) ||
 		    get_user(tv[1].tv_sec, &t[1].tv_sec) ||
-		    get_user(tv[1].tv_usec, &t[1].tv_usec))
+		    get_user(tv[1].tv_nsec, &t[1].tv_usec))
 			return -EFAULT;
+		if (tv[0].tv_nsec > LONG_MAX / 1000
+		    || tv[1].tv_nsec > LONG_MAX / 1000)
+			return -EINVAL;
+		tv[0].tv_nsec *= 1000;
+		tv[1].tv_nsec *= 1000;
 	}
 	return do_utimes(dfd, filename, t ? tv : NULL);
 }
diff --git a/fs/utimes.c b/fs/utimes.c
index 99cf2cb..dc6612e 100644
--- a/fs/utimes.c
+++ b/fs/utimes.c
@@ -3,6 +3,7 @@
 #include <linux/linkage.h>
 #include <linux/namei.h>
 #include <linux/sched.h>
+#include <linux/stat.h>
 #include <linux/utime.h>
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -76,7 +77,7 @@ out:
  * must be owner or have write permission.
  * Else, update from *times, must be owner or super user.
  */
-long do_utimes(int dfd, char __user *filename, struct timeval *times)
+long do_utimes(int dfd, char __user *filename, struct timespec *times)
 {
 	int error;
 	struct nameidata nd;
@@ -100,11 +101,21 @@ long do_utimes(int dfd, char __user *filename, struct timeval *times)
                 if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
                         goto dput_and_out;
 
-		newattrs.ia_atime.tv_sec = times[0].tv_sec;
-		newattrs.ia_atime.tv_nsec = times[0].tv_usec * 1000;
-		newattrs.ia_mtime.tv_sec = times[1].tv_sec;
-		newattrs.ia_mtime.tv_nsec = times[1].tv_usec * 1000;
-		newattrs.ia_valid |= ATTR_ATIME_SET | ATTR_MTIME_SET;
+		if (times[0].tv_nsec == UTIME_OMIT)
+			newattrs.ia_valid &= ~ATTR_ATIME;
+		else if (times[0].tv_nsec != UTIME_NOW) {
+			newattrs.ia_atime.tv_sec = times[0].tv_sec;
+			newattrs.ia_atime.tv_nsec = times[0].tv_nsec;
+			newattrs.ia_valid |= ATTR_ATIME_SET;
+		}
+
+		if (times[1].tv_nsec == UTIME_OMIT)
+			newattrs.ia_valid &= ~ATTR_MTIME;
+		else if (times[1].tv_nsec != UTIME_NOW) {
+			newattrs.ia_mtime.tv_sec = times[1].tv_sec;
+			newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
+			newattrs.ia_valid |= ATTR_MTIME_SET;
+		}
 	} else {
 		error = -EACCES;
                 if (IS_IMMUTABLE(inode))
@@ -123,13 +134,55 @@ out:
 	return error;
 }
 
+asmlinkage long sys_utimensat(int dfd, char __user *filename, struct timespec __user *utimes)
+{
+	struct timespec tstimes[2];
+	if (utimes) {
+		if (copy_from_user(&tstimes, utimes, sizeof(tstimes)))
+			return -EFAULT;
+		if ((tstimes[0].tv_nsec == UTIME_OMIT ||
+		     tstimes[0].tv_nsec == UTIME_NOW) &&
+		    tstimes[0].tv_sec != 0)
+			return -EINVAL;
+		if ((tstimes[1].tv_nsec == UTIME_OMIT ||
+		     tstimes[1].tv_nsec == UTIME_NOW) &&
+		    tstimes[1].tv_sec != 0)
+			return -EINVAL;
+
+		/* Nothing to do, we must not even check the path.  */
+		if (tstimes[0].tv_nsec == UTIME_OMIT &&
+		    tstimes[1].tv_nsec == UTIME_OMIT)
+			return 0;
+	}
+
+	return do_utimes(dfd, filename, utimes ? tstimes : NULL);
+}
+
 asmlinkage long sys_futimesat(int dfd, char __user *filename, struct timeval __user *utimes)
 {
 	struct timeval times[2];
+	struct timespec tstimes[2];
+
+	if (utimes) {
+		if (copy_from_user(&times, utimes, sizeof(times)))
+			return -EFAULT;
+
+		/* This test is needed to catch all invalid values.  If we
+		   would test only in do_utimes we would miss those invalid
+		   values truncated by the multiplication with 1000.  Note
+		   that we also catch UTIME_{NOW,OMIT} here which are only
+		   valid for utimensat.  */
+		if (times[0].tv_usec > LONG_MAX / 1000 ||
+		    times[1].tv_usec > LONG_MAX / 1000)
+			return -EINVAL;
+
+		tstimes[0].tv_sec = times[0].tv_sec;
+		tstimes[0].tv_nsec = 1000 * times[0].tv_usec;
+		tstimes[1].tv_sec = times[1].tv_sec;
+		tstimes[1].tv_nsec = 1000 * times[1].tv_usec;
+	}
 
-	if (utimes && copy_from_user(&times, utimes, sizeof(times)))
-		return -EFAULT;
-	return do_utimes(dfd, filename, utimes ? times : NULL);
+	return do_utimes(dfd, filename, utimes ? tstimes : NULL);
 }
 
 asmlinkage long sys_utimes(char __user *filename, struct timeval __user *utimes)
diff --git a/include/asm-i386/unistd.h b/include/asm-i386/unistd.h
index 833fa17..17a9d5a 100644
--- a/include/asm-i386/unistd.h
+++ b/include/asm-i386/unistd.h
@@ -325,6 +325,7 @@
 #define __NR_move_pages		317
 #define __NR_getcpu		318
 #define __NR_epoll_pwait	319
+#define __NR_utimensat		320
 
 #ifdef __KERNEL__
 
diff --git a/include/asm-x86_64/unistd.h b/include/asm-x86_64/unistd.h
index c5f596e..0aae2ae 100644
--- a/include/asm-x86_64/unistd.h
+++ b/include/asm-x86_64/unistd.h
@@ -619,8 +619,10 @@ __SYSCALL(__NR_sync_file_range, sys_sync_file_range)
 __SYSCALL(__NR_vmsplice, sys_vmsplice)
 #define __NR_move_pages		279
 __SYSCALL(__NR_move_pages, sys_move_pages)
+#define __NR_utimensat		280
+__SYSCALL(__NR_utimensat, sys_utimensat)
 
-#define __NR_syscall_max __NR_move_pages
+#define __NR_syscall_max __NR_utimensat
 
 #ifndef __NO_STUBS
 #define __ARCH_WANT_OLD_READDIR
diff --git a/include/linux/stat.h b/include/linux/stat.h
index 679ef0d..611c398 100644
--- a/include/linux/stat.h
+++ b/include/linux/stat.h
@@ -53,6 +53,9 @@
 #define S_IWUGO		(S_IWUSR|S_IWGRP|S_IWOTH)
 #define S_IXUGO		(S_IXUSR|S_IXGRP|S_IXOTH)
 
+#define UTIME_NOW	((1l << 30) - 1l)
+#define UTIME_OMIT	((1l << 30) - 2l)
+
 #include <linux/types.h>
 #include <linux/time.h>
 
diff --git a/include/linux/time.h b/include/linux/time.h
index 8ea8dea..936b21b 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -109,7 +109,7 @@ extern void do_gettimeofday(struct timeval *tv);
 extern int do_settimeofday(struct timespec *tv);
 extern int do_sys_settimeofday(struct timespec *tv, struct timezone *tz);
 #define do_posix_clock_monotonic_gettime(ts) ktime_get_ts(ts)
-extern long do_utimes(int dfd, char __user *filename, struct timeval *times);
+extern long do_utimes(int dfd, char __user *filename, struct timespec *times);
 struct itimerval;
 extern int do_setitimer(int which, struct itimerval *value,
 			struct itimerval *ovalue);

Re: [PATCH] utimensat implementation

[Andrew Morton]

On Thu, 26 Apr 2007 18:49:05 -0400 Ulrich Drepper <drepper@redhat.com> wrote:

> 
> The next revision of POSIX will support fine-grained filesystem timestamps the way we already support.  struct stat will report nanosecond values.  So far so good.
> 
> During the development one additional problem was found: there is no interface to set the file timestamp with that precision.  utimes only takes a timeval structure which allows only micro-second resolution.

Does the spec say what the OS should do if (ts_nsec => 1e9)?

> This is why the utimensat() interface was created.  It is basically the same as our futimesat() interface but it takes a timespec structure.
> 
> While adding this new interface two more features got added.  Programmers sometimes/often only want to set one value.  This currently requires reading the current value with stat() and then use this value in the futimesat() call.  This is slow and might create even wrong results (the file could have been updated in the meantime).  If the tv_nsec value of either of the elements of the utimes parameter to utimensat() is UTIME_OMIT no update of that respective value is performed.

ITYM "If the value of either of the elements..."

+#define UTIME_NOW	((1l << 30) - 1l)
+#define UTIME_OMIT	((1l << 30) - 2l)

OK, so there's no collision on ts_nsec if unnormalised timespecs are
disallowed.

But there's a potential collision on ts_sec?  Do we know what date that
corresponds to?

> The second extension allows to set one of the values to the correct current time.  Today it is only possible to set both values at the same time.  Once again this is slower and might to lead to incorrect results.  The use of UTIME_NOW for the respective tv_nsec value implements this functionality.
> 
> The resulting patch is attached.  It modifies the do_utimes function which is also used in the compat code.  The callers are adjusted.  Most of the added code are checks for invalid parameters.  In fact, I think one problem where the old code wouldn't recognize certain overflows (if tv_nsec * 1000 overflows).
> 
> I've tested the code on x86-64.

Do you have a testcase app which can be used by arch maintainers?

Re: [PATCH] utimensat implementation

[H. Peter Anvin]

Andrew Morton wrote:
> On Thu, 26 Apr 2007 18:49:05 -0400 Ulrich Drepper <drepper@redhat.com> wrote:
>>
>> If the tv_nsec value of either of the elements of the utimes parameter to utimensat() is UTIME_OMIT no update of that respective value is performed.
> 
> ITYM "If the value of either of the elements..."
> 
> +#define UTIME_NOW	((1l << 30) - 1l)
> +#define UTIME_OMIT	((1l << 30) - 2l)
> 
> OK, so there's no collision on ts_nsec if unnormalised timespecs are
> disallowed.
> 
> But there's a potential collision on ts_sec?  Do we know what date that
> corresponds to?

"If the tv_nsec value" implies that these magic numbers have no impact
on these.

I'm a bit leery of abusing the timespec value like this, though.  A
flags field seem like it would be cleaner.

Something else... if we're dickering with these interfaces, shouldn't we
allow setting atime as well?

	-hpa

Re: [PATCH] utimensat implementation

[Ulrich Drepper]

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

H. Peter Anvin wrote:
> I'm a bit leery of abusing the timespec value like this, though.  A
> flags field seem like it would be cleaner.

It's ugly.  Then you have the parameter, which might have nice valid
values, and they get ignored.  I thought about it when this was
discussed in the working group and thought it's a toss up.


> Something else... if we're dickering with these interfaces, shouldn't we
> allow setting atime as well?

Why?  To allow somebody to hide her/his tracks?

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: [PATCH] utimensat implementation

[H. Peter Anvin]

Ulrich Drepper wrote:
> H. Peter Anvin wrote:
>> I'm a bit leery of abusing the timespec value like this, though.  A
>> flags field seem like it would be cleaner.
> 
> It's ugly.  Then you have the parameter, which might have nice valid
> values, and they get ignored.  I thought about it when this was
> discussed in the working group and thought it's a toss up.

It's pretty ugly either way :-/

>> Something else... if we're dickering with these interfaces, shouldn't we
>> allow setting atime as well?
> 
> Why?  To allow somebody to hide her/his tracks?

Primarily to let a backup program restore the full state of the filesystem.

	-hpa

Re: [PATCH] utimensat implementation

[Ulrich Drepper]

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

H. Peter Anvin wrote:
> Primarily to let a backup program restore the full state of the filesystem.

Is this wanted?  Or needed?  I would think there are good reasons why
this hasn't been done so far.  Intrusion detection is one reason I can
think of.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: [PATCH] utimensat implementation

[H. Peter Anvin]

Ulrich Drepper wrote:
> H. Peter Anvin wrote:
>> Primarily to let a backup program restore the full state of the filesystem.
> 
> Is this wanted?  Or needed?  I would think there are good reasons why
> this hasn't been done so far.  Intrusion detection is one reason I can
> think of.

Well, dump/restore -- the traditional Unix backup solution -- went "the
back way" to restore the times in the filesystem.

The main use of atime seems to be to figure out when something can be
automatically deleted.  Anyone else have other usage scenarios?

	-hpa

Re: [PATCH] utimensat implementation

[David Lang]

On Fri, 27 Apr 2007, H. Peter Anvin wrote:

> The main use of atime seems to be to figure out when something can be
> automatically deleted.  Anyone else have other usage scenarios?

as a varient of this, I use it to help determine what files are actually needed 
when building a chroot sandbox.

David Lang

Re: [PATCH] utimensat implementation

[Ulrich Drepper]

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

H. Peter Anvin wrote:
> The main use of atime seems to be to figure out when something can be
> automatically deleted.  Anyone else have other usage scenarios?

It's not atime which cannot be set, it's ctime.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: [PATCH] utimensat implementation

[H. Peter Anvin]

Ulrich Drepper wrote:
> H. Peter Anvin wrote:
>> The main use of atime seems to be to figure out when something can be
>> automatically deleted.  Anyone else have other usage scenarios?
> 
> It's not atime which cannot be set, it's ctime.

You're right, my brainfart.

	-hpa

Re: [PATCH] utimensat implementation

[Ulrich Drepper]

[-- Attachment #1.1: Type: text/plain, Size: 1377 bytes --]

Andrew Morton wrote:
> Does the spec say what the OS should do if (ts_nsec => 1e9)?

Yes, return EINVAL.  We already do this.  It's just that now we have to
recognize two special values.


> OK, so there's no collision on ts_nsec if unnormalised timespecs are
> disallowed.

Indeed, that's the basis of using the special values.

I chose the values of the constants so that they are a) out of the way
of valid values and b) don't have to be adjusted for 32-bit compat code.


> But there's a potential collision on ts_sec?  Do we know what date that
> corresponds to?

No, there is no collision.  The tv_sec value is relevant.  The
UTIME_OMIT and UTIME_NOW value refers to the atime/mtime respectively,
not just the tv_nsec field of either.  It makes no sense to just set
tv_sec, the tv_nsec value would be basically random.

In my patch I'm testing that tv_sec is zero in case any of the special
values is used in the corresponding tv_nsec field.  That's more than the
standard currently requires but I think it's better and I try to get the
standard proposal changed.  If this doesn't happen I'll make appropriate
changes at userlevel for the "strictly POSIX" mode.


> Do you have a testcase app which can be used by arch maintainers?

Attached here.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: utimensat-test.c --]
[-- Type: text/x-csrc; name="utimensat-test.c", Size: 3044 bytes --]

#include <errno.h>
#include <fcntl.h>
#include <time.h>
#include <sys/time.h>
#include <stddef.h>
#include <syscall.h>

#define UTIME_NOW       ((1l << 30) - 1l)
#define UTIME_OMIT      ((1l << 30) - 2l)


int
main(void)
{
  int status = 0;

  int fd = open("ttt", O_RDWR|O_CREAT|O_EXCL, 0666);
  if (fd == -1)
    error (1, errno, "failed to create test file \"ttt\"");

  struct stat64 st1;
  if (fstat64 (fd, &st1) != 0)
    error (1, errno, "fstat failed");

  struct timespec t[2];
  t[0].tv_sec = 0;
  t[0].tv_nsec = 0;
  t[1].tv_sec = 0;
  t[1].tv_nsec = 0;
  if (syscall(280, AT_FDCWD, "ttt", t) != 0)
    error (1, errno, "utimensat failed");

  struct stat64 st2;
  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");
  
  if (st2.st_atim.tv_sec != 0 || st2.st_atim.tv_nsec != 0)
    {
      puts ("atim not reset to zero");
      status = 1;
    }
  if (st2.st_mtim.tv_sec != 0 || st2.st_mtim.tv_nsec != 0)
    {
      puts ("mtim not reset to zero");
      status = 1;
    }
  if (status != 0)
    goto out;

  t[0] = st1.st_atim;
  t[1].tv_sec = 0;
  t[1].tv_nsec = UTIME_OMIT;
  if (syscall(280, AT_FDCWD, "ttt", t) != 0)
    error (1, errno, "utimensat failed");

  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");
  
  if (st2.st_atim.tv_sec != st1.st_atim.tv_sec
      || st2.st_atim.tv_nsec != st1.st_atim.tv_nsec)
    {
      puts ("atim not set");
      status = 1;
    }
  if (st2.st_mtim.tv_sec != 0 || st2.st_mtim.tv_nsec != 0)
    {
      puts ("mtim changed from zero");
      status = 1;
    }
  if (status != 0)
    goto out;

  t[0].tv_sec = 0;
  t[0].tv_nsec = UTIME_OMIT;
  t[1] = st1.st_mtim;
  if (syscall(280, AT_FDCWD, "ttt", t) != 0)
    error (1, errno, "utimensat failed");

  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");
  
  if (st2.st_atim.tv_sec != st1.st_atim.tv_sec
      || st2.st_atim.tv_nsec != st1.st_atim.tv_nsec)
    {
      puts ("mtim changed from original time");
      status = 1;
    }
  if (st2.st_mtim.tv_sec != st1.st_mtim.tv_sec
      || st2.st_mtim.tv_nsec != st1.st_mtim.tv_nsec)
    {
      puts ("mtim not set");
      status = 1;
    }
  if (status != 0)
    goto out;

  sleep (2);

  t[0].tv_sec = 0;
  t[0].tv_nsec = UTIME_NOW;
  t[1].tv_sec = 0;
  t[1].tv_nsec = UTIME_NOW;
  if (syscall(280, AT_FDCWD, "ttt", t) != 0)
    error (1, errno, "utimensat failed");

  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");

  struct timeval tv;
  gettimeofday(&tv,NULL);

  if (st2.st_atim.tv_sec <= st1.st_atim.tv_sec
      || st2.st_atim.tv_sec > tv.tv_sec)
    {
      puts ("atim not set to NOW");
      status = 1;
    }
  if (st2.st_mtim.tv_sec <= st1.st_mtim.tv_sec
      || st2.st_mtim.tv_sec > tv.tv_sec)
    {
      puts ("mtim not set to NOW");
      status = 1;
    }

  if (status == 0)
     puts ("all OK");

 out:
  close (fd);
  unlink ("ttt");

  return status;
}

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Updated utimensat test program

[Ulrich Drepper]

[-- Attachment #1.1: Type: text/plain, Size: 220 bytes --]

This is the updated test program for the utimensat syscall.  Cleaned up
a bit and extended to cover the lutimesat functionality.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: utimensat-test.c --]
[-- Type: text/x-csrc; name="utimensat-test.c", Size: 3812 bytes --]

#include <errno.h>
#include <fcntl.h>
#include <time.h>
#include <sys/time.h>
#include <stddef.h>
#include <syscall.h>

#define __NR_utimensat 280

#define UTIME_NOW       ((1l << 30) - 1l)
#define UTIME_OMIT      ((1l << 30) - 2l)


int
main(void)
{
  int status = 0;

  int fd = open("ttt", O_RDWR|O_CREAT|O_EXCL, 0666);
  if (fd == -1)
    error (1, errno, "failed to create test file \"ttt\"");

  struct stat64 st1;
  if (fstat64 (fd, &st1) != 0)
    error (1, errno, "fstat failed");

  struct timespec t[2];
  t[0].tv_sec = 0;
  t[0].tv_nsec = 0;
  t[1].tv_sec = 0;
  t[1].tv_nsec = 0;
  if (syscall(__NR_utimensat, AT_FDCWD, "ttt", t, 0) != 0)
    error (1, errno, "utimensat failed");

  struct stat64 st2;
  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");
  
  if (st2.st_atim.tv_sec != 0 || st2.st_atim.tv_nsec != 0)
    {
      puts ("atim not reset to zero");
      status = 1;
    }
  if (st2.st_mtim.tv_sec != 0 || st2.st_mtim.tv_nsec != 0)
    {
      puts ("mtim not reset to zero");
      status = 1;
    }
  if (status != 0)
    goto out;

  t[0] = st1.st_atim;
  t[1].tv_sec = 0;
  t[1].tv_nsec = UTIME_OMIT;
  if (syscall(__NR_utimensat, AT_FDCWD, "ttt", t, 0) != 0)
    error (1, errno, "utimensat failed");

  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");
  
  if (st2.st_atim.tv_sec != st1.st_atim.tv_sec
      || st2.st_atim.tv_nsec != st1.st_atim.tv_nsec)
    {
      puts ("atim not set");
      status = 1;
    }
  if (st2.st_mtim.tv_sec != 0 || st2.st_mtim.tv_nsec != 0)
    {
      puts ("mtim changed from zero");
      status = 1;
    }
  if (status != 0)
    goto out;

  t[0].tv_sec = 0;
  t[0].tv_nsec = UTIME_OMIT;
  t[1] = st1.st_mtim;
  if (syscall(__NR_utimensat, AT_FDCWD, "ttt", t, 0) != 0)
    error (1, errno, "utimensat failed");

  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");
  
  if (st2.st_atim.tv_sec != st1.st_atim.tv_sec
      || st2.st_atim.tv_nsec != st1.st_atim.tv_nsec)
    {
      puts ("mtim changed from original time");
      status = 1;
    }
  if (st2.st_mtim.tv_sec != st1.st_mtim.tv_sec
      || st2.st_mtim.tv_nsec != st1.st_mtim.tv_nsec)
    {
      puts ("mtim not set");
      status = 1;
    }
  if (status != 0)
    goto out;

  sleep (2);

  t[0].tv_sec = 0;
  t[0].tv_nsec = UTIME_NOW;
  t[1].tv_sec = 0;
  t[1].tv_nsec = UTIME_NOW;
  if (syscall(__NR_utimensat, AT_FDCWD, "ttt", t, 0) != 0)
    error (1, errno, "utimensat failed");

  if (fstat64 (fd, &st2) != 0)
    error (1, errno, "fstat failed");

  struct timeval tv;
  gettimeofday(&tv,NULL);

  if (st2.st_atim.tv_sec <= st1.st_atim.tv_sec
      || st2.st_atim.tv_sec > tv.tv_sec)
    {
      puts ("atim not set to NOW");
      status = 1;
    }
  if (st2.st_mtim.tv_sec <= st1.st_mtim.tv_sec
      || st2.st_mtim.tv_sec > tv.tv_sec)
    {
      puts ("mtim not set to NOW");
      status = 1;
    }

  if (symlink ("ttt", "tttsym") != 0)
    error (1, errno, "cannot create symlink");

  t[0].tv_sec = 0;
  t[0].tv_nsec = 0;
  t[1].tv_sec = 0;
  t[1].tv_nsec = 0;
  if (syscall(__NR_utimensat, AT_FDCWD, "tttsym", t, AT_SYMLINK_NOFOLLOW) != 0)
    error (1, errno, "utimensat failed");

  if (lstat64 ("tttsym", &st2) != 0)
    error (1, errno, "lstat failed");

  if (st2.st_atim.tv_sec != 0 || st2.st_atim.tv_nsec != 0)
    {
      puts ("symlink atim not reset to zero");
      status = 1;
    }
  if (st2.st_mtim.tv_sec != 0 || st2.st_mtim.tv_nsec != 0)
    {
      puts ("symlink mtim not reset to zero");
      status = 1;
    }

  if (status == 0)
     puts ("all OK");

 out:
  close (fd);
  unlink ("ttt");
  unlink ("tttsym");

  return status;
}

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: [PATCH] utimensat implementation

[Neil Brown]

On Thursday April 26, drepper@redhat.com wrote:
> The next revision of POSIX will support fine-grained filesystem
> timestamps the way we already support.  struct stat will report
> nanosecond values.  So far so good.  

Does it also specify how to find out what granularity is used by the
filesystem?  I had a need for this just recently and couldn't see any
way to extract it.

[If the mtime of a file matches the current time, then you cannot
cache the contents of the file.  You have to wait until the mtime is
in the past.  Without knowing the granularity, you cannot tell if the
mtime still matches current time or not]

Thanks,
NeilBrown

Re: [PATCH] utimensat implementation

[Ulrich Drepper]

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

Neil Brown wrote:
> Does it also specify how to find out what granularity is used by the
> filesystem?  I had a need for this just recently and couldn't see any
> way to extract it.

That's still on the table.  We might end up with an fpathconf() solution.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: [PATCH] utimensat implementation

[Neil Brown]

On Thursday April 26, drepper@redhat.com wrote:
> Neil Brown wrote:
> > Does it also specify how to find out what granularity is used by the
> > filesystem?  I had a need for this just recently and couldn't see any
> > way to extract it.
> 
> That's still on the table.  We might end up with an fpathconf() solution.

Ok, that makes sense.  Thanks.  Glad it is under consideration.

NeilBrown

Re: [PATCH] utimensat implementation

[Ulrich Drepper]

Ulrich Drepper wrote:
> Neil Brown wrote:
>> Does it also specify how to find out what granularity is used by the
>> filesystem?  I had a need for this just recently and couldn't see any
>> way to extract it.
> 
> That's still on the table.  We might end up with an fpathconf() solution.

OK, the pathconf()-based solution will most probably be in the next 
POSIX spec.

Now, somebody has to provide a way to get to this information.  The 
kernel does not export it so far.  Is it finally time to break down and 
allow pathconf() and fpathconf() syscalls into the kernel?

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

Re: [PATCH] utimensat implementation

[Christoph Hellwig]

On Thu, May 10, 2007 at 11:26:36AM -0700, Ulrich Drepper wrote:
> >That's still on the table.  We might end up with an fpathconf() solution.
> 
> OK, the pathconf()-based solution will most probably be in the next 
> POSIX spec.
> 
> Now, somebody has to provide a way to get to this information.  The 
> kernel does not export it so far.  Is it finally time to break down and 
> allow pathconf() and fpathconf() syscalls into the kernel?

I'd be happy to have them.  While it's not the nicest API in the world
it's in Posix and we have to support it at the library level, so we
should better get it right.

I'd like to avoid having a big swithc statement in every filesystem,
though, instead of we should have a table-driven approach instead
where each filesystem defines one table (or multiple ones when it
supports subtypes with different limits) and just sets a pointer in
the superblock to it.

E.g.

long foofs_pathconf {
	[_PC_LINK_MAX]		= 16384,
	[_PC_2_SYMLINKS]	= 1,
	...
}

and then in fill_super:

	sb->s_pathconf = &foofs_pathconf;

Re: [PATCH] utimensat implementation

[Ulrich Drepper]

Christoph Hellwig wrote:
> E.g.
> 
> long foofs_pathconf {
> 	[_PC_LINK_MAX]		= 16384,
> 	[_PC_2_SYMLINKS]	= 1,

In general I agree.  But what do you do for network filesystems?  Maybe 
we'll have a network filesystem protocol which allows to query the 
remote server about the underlying filesystem.  Then the return value is 
dynamic and it's the maximum (coarsest granularity) of the network 
filesystem itself and the underlying filesystem.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

Re: [PATCH] utimensat implementation

[Christoph Hellwig]

On Thu, May 10, 2007 at 12:44:19PM -0700, Ulrich Drepper wrote:
> Christoph Hellwig wrote:
> >E.g.
> >
> >long foofs_pathconf {
> >	[_PC_LINK_MAX]		= 16384,
> >	[_PC_2_SYMLINKS]	= 1,
> 
> In general I agree.  But what do you do for network filesystems?  Maybe 
> we'll have a network filesystem protocol which allows to query the 
> remote server about the underlying filesystem.  Then the return value is 
> dynamic and it's the maximum (coarsest granularity) of the network 
> filesystem itself and the underlying filesystem.

You're right, we'll probably want a method call for it.  The table
approach might still be a nice helper for the normal case.

Re: [PATCH] utimensat implementation

[H. Peter Anvin]

Christoph Hellwig wrote:
> 
> I'd be happy to have them.  While it's not the nicest API in the world
> it's in Posix and we have to support it at the library level, so we
> should better get it right.
> 
> I'd like to avoid having a big swithc statement in every filesystem,
> though, instead of we should have a table-driven approach instead
> where each filesystem defines one table (or multiple ones when it
> supports subtypes with different limits) and just sets a pointer in
> the superblock to it.
> 

This is starting to sound an awful lot like statfs().  Maybe we could
create a new statfs call which takes a buffer size input (so that we can
add new fields as time goes on) and which returns the necessary information?

	-hpa

Re: [PATCH] utimensat implementation

[Neil Brown]

On Thursday May 10, drepper@redhat.com wrote:
> Ulrich Drepper wrote:
> > Neil Brown wrote:
> >> Does it also specify how to find out what granularity is used by the
> >> filesystem?  I had a need for this just recently and couldn't see any
> >> way to extract it.
> > 
> > That's still on the table.  We might end up with an fpathconf() solution.
> 
> OK, the pathconf()-based solution will most probably be in the next 
> POSIX spec.
> 
> Now, somebody has to provide a way to get to this information.  The 
> kernel does not export it so far.  Is it finally time to break down and 
> allow pathconf() and fpathconf() syscalls into the kernel?

Maybe... certainly we want some way to get at this information.

It has occurred to me a number of times that there is no easy way to
export information about filesystems from the kernel.

One specific example is request statistics for an NFS filesystem.  We
can get system-wide statistics, but to get stats for a single
filesystem isn't possible, and a big reason for this is that there is
no-where to put that information.

Filesystems also have a variety of mount options and they are only
available through "/proc/mounts" and can only be change by "remount"
which is a bit of a clunky interface.

Just about every other kernel object is, or can be, exposed through
sysfs.  But filesystems cannot.  This is presumably because there is
no unique handle for them (what with name spaces and bind mounts and
so forth).
Each filesystem still have a unique device number (->s_dev) so that
could be used. e.g. we could create
  /sysfs/filesystem/00:03/....

which would contain info about the filesystem with device number 0:3.
We could then put time-granularity and other fs-specific info in
there.

I feel that would be more flexible than a specific fpathconfat system
call.  But would it be enough?
The pathconf values can apparently be different for different files in
a filesystem.  Is that important?  If it is, we really would want
some new syscall rather than just sysfs attributes.

So that makes two questions for anyone with opinions:
 1/ Does pathconf have to be per-file, or is per-filesystem OK
 2/ Can we have a way to put attributes for filesystems in sysfs?

NeilBrown