utimensat fails to update ctime

utimensat fails to update ctime

[Eric Blake]

POSIX requires that utimensat/futimens must update ctime in all cases
where any change is made (it only exempts when both atime and mtime were
requested as UTIME_OMIT, where the file must exist but no change is made).
 Unfortunately, when atime is specified and mtime is UTIME_OMIT, the
kernel mistakenly behaves like read(), by updating atime but not ctime.
This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html

Here is a simple program demonstrating the failure:
$ cat foo.c
#include <fcntl.h>
#include <unistd.h>
#include <sys/stat.h>
int
main ()
{
  int fd = creat ("file", 0600);
  struct stat st1, st2;
  struct timespec t[2] = { { 1000000000, 0 }, { 0, UTIME_OMIT } };
  fstat (fd, &st1);
  sleep (1);
  futimens (fd, t);
  fstat (fd, &st2);
  return st1.st_ctime == st2.st_ctime;
}
$ gcc -o foo foo.c -D_GNU_SOURCE
$ ./foo; echo $?
1

The exit status should have been 0.

GNU coreutils will end up working around the bug by calling fstat/[l]stat
prior to futimens/utimensat, and populating the mtime field with the
desired value rather than using UTIME_OMIT.  But this is a pointless stat
call, which could be avoided if the kernel were fixed to comply with POSIX
by updating ctime even when mtime is UTIME_OMIT.

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

Eric Blake             ebb9@byu.net

Re: utimensat fails to update ctime

[OGAWA Hirofumi]

Eric Blake <ebb9@byu.net> writes:

> POSIX requires that utimensat/futimens must update ctime in all cases
> where any change is made (it only exempts when both atime and mtime were
> requested as UTIME_OMIT, where the file must exist but no change is made).
>  Unfortunately, when atime is specified and mtime is UTIME_OMIT, the
> kernel mistakenly behaves like read(), by updating atime but not ctime.
> This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
> http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html
>
> Here is a simple program demonstrating the failure:
> $ cat foo.c
> #include <fcntl.h>
> #include <unistd.h>
> #include <sys/stat.h>
> int
> main ()
> {
>   int fd = creat ("file", 0600);
>   struct stat st1, st2;
>   struct timespec t[2] = { { 1000000000, 0 }, { 0, UTIME_OMIT } };
>   fstat (fd, &st1);
>   sleep (1);
>   futimens (fd, t);
>   fstat (fd, &st2);
>   return st1.st_ctime == st2.st_ctime;
> }
> $ gcc -o foo foo.c -D_GNU_SOURCE
> $ ./foo; echo $?
> 1
>
> The exit status should have been 0.
>
> GNU coreutils will end up working around the bug by calling fstat/[l]stat
> prior to futimens/utimensat, and populating the mtime field with the
> desired value rather than using UTIME_OMIT.  But this is a pointless stat
> call, which could be avoided if the kernel were fixed to comply with POSIX
> by updating ctime even when mtime is UTIME_OMIT.

I couldn't reproduce this with your test program on my machine (latest
linus tree). And that utime path looks like no problem, um..., can you
provide output of strace or something?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: utimensat fails to update ctime

[Eric Blake]

According to OGAWA Hirofumi on 12/21/2009 12:31 AM:
>> This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
>> http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html
>>
>> GNU coreutils will end up working around the bug by calling fstat/[l]stat
>> prior to futimens/utimensat, and populating the mtime field with the
>> desired value rather than using UTIME_OMIT.  But this is a pointless stat
>> call, which could be avoided if the kernel were fixed to comply with POSIX
>> by updating ctime even when mtime is UTIME_OMIT.
> 
> I couldn't reproduce this with your test program on my machine (latest
> linus tree). And that utime path looks like no problem, um..., can you
> provide output of strace or something?

$ uname -a
Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
x86_64 GNU/Linux
$ strace ./foo
execve("./foo", ["./foo"], [/* 19 vars */]) = 0
brk(0)                                  = 0x16fc000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94eb000
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or
directory)
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94e9000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or
directory)
open("/etc/ld.so.cache", O_RDONLY)      = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=42235, ...}) = 0
mmap(NULL, 42235, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fa7d94de000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or
directory)
open("/lib/libc.so.6", O_RDONLY)        = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\340\342"..., 832)
= 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1436976, ...}) = 0
mmap(NULL, 3543672, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0)
= 0x7fa7d8f6d000
mprotect(0x7fa7d90c5000, 2097152, PROT_NONE) = 0
mmap(0x7fa7d92c5000, 20480, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x158000) = 0x7fa7d92c5000
mmap(0x7fa7d92ca000, 17016, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fa7d92ca000
close(3)                                = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94dd000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94dc000
arch_prctl(ARCH_SET_FS, 0x7fa7d94dc6e0) = 0
mprotect(0x7fa7d92c5000, 12288, PROT_READ) = 0
munmap(0x7fa7d94de000, 42235)           = 0
creat("file", 0600)                     = 3
fstat(3, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
rt_sigaction(SIGCHLD, NULL, {SIG_DFL}, 8) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
nanosleep({1, 0}, {1, 0})               = 0
syscall_280(0x3, 0, 0x7fff707fb660, 0, 0, 0x7fff707fb360, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) = 0
fstat(3, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
exit_group(1)                           = ?
Process 13846 detached
$ stat file
  File: `file'
  Size: 0         	Blocks: 0          IO Block: 4096   regular empty file
Device: 811h/2065d	Inode: 1769273205  Links: 1
Access: (0600/-rw-------)  Uid: ( 1267/   ericb)   Gid: ( 1267/   ericb)
Access: 2001-09-08 21:46:40.000000000 -0400
Modify: 2009-12-21 08:06:03.425326531 -0500
Change: 2009-12-21 08:06:03.425326531 -0500
$

Note that ctime is still stuck at the same second as mtime, even though it
should be at least one second later since there was a sleep(1) in between
the file creation and the utimensat that adjusted the atime.

However, when I tried on a different machine, I did not see the failure:

$ uname -a
Linux vladim 2.6.31.6-166.fc12.i686 #1 SMP Wed Dec 9 11:14:59 EST 2009
i686 GNU/Linux

So it may be architecture-dependent.  The coreutils report was against:

> Host triplet: i686-pc-linux-gnu
> coreutils: 8.2
> kernel:    2.6.32.1

but I don't have access to a machine running that new of a kernel at the
moment.

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

Eric Blake             ebb9@byu.net

Re: utimensat fails to update ctime

[Eric Blake]

According to Eric Blake on 12/21/2009 6:12 AM:

It may also be file-system dependent.  On the machine where I saw the
original failure:
> $ uname -a
> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
> x86_64 GNU/Linux

$ df -T .
Filesystem    Type   1K-blocks      Used Available Use% Mounted on
/dev/sdb1      xfs   419299328 269018656 150280672  65% /srv/data

But on the machine where the test case is working:
> $ uname -a
> Linux vladim 2.6.31.6-166.fc12.i686 #1 SMP Wed Dec 9 11:14:59 EST 2009
> i686 GNU/Linux

$ df -T .
Filesystem    Type   1K-blocks      Used Available Use% Mounted on
/dev/sda3     ext3    34123236  25894932   6494892  80% /

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

Eric Blake             ebb9@byu.net

Re: utimensat fails to update ctime

[OGAWA Hirofumi]

Eric Blake <ebb9@byu.net> writes:

According to OGAWA Hirofumi on 12/21/2009 12:31 AM:
>>> This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
>>> http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html
>>>
>>> GNU coreutils will end up working around the bug by calling fstat/[l]stat
>>> prior to futimens/utimensat, and populating the mtime field with the
>>> desired value rather than using UTIME_OMIT.  But this is a pointless stat
>>> call, which could be avoided if the kernel were fixed to comply with POSIX
>>> by updating ctime even when mtime is UTIME_OMIT.
>> 
>> I couldn't reproduce this with your test program on my machine (latest
>> linus tree). And that utime path looks like no problem, um..., can you
>> provide output of strace or something?

> According to Eric Blake on 12/21/2009 6:12 AM:
>
> It may also be file-system dependent.  On the machine where I saw the
> original failure:
>> $ uname -a
>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
>> x86_64 GNU/Linux
>
> $ df -T .
> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
> /dev/sdb1      xfs   419299328 269018656 150280672  65% /srv/data

Thanks.

This is good point. This would be xfs issue or design. xfs seems to have
own special handling of ctime.

Cc: to xfs peoples. Any idea?

> But on the machine where the test case is working:
>> $ uname -a
>> Linux vladim 2.6.31.6-166.fc12.i686 #1 SMP Wed Dec 9 11:14:59 EST 2009
>> i686 GNU/Linux
>
> $ df -T .
> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
> /dev/sda3     ext3    34123236  25894932   6494892  80% /

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: utimensat fails to update ctime

[Eric Blake]

According to OGAWA Hirofumi on 12/21/2009 8:05 AM:
>> It may also be file-system dependent.  On the machine where I saw the
>> original failure:
>>> $ uname -a
>>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
>>> x86_64 GNU/Linux
>> $ df -T .
>> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
>> /dev/sdb1      xfs   419299328 269018656 150280672  65% /srv/data
> 
> Thanks.
> 
> This is good point. This would be xfs issue or design. xfs seems to have
> own special handling of ctime.

Here's another report, this time about an mtime update not happening on
ntfs-3g.  http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/19336

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

Eric Blake             ebb9@byu.net

Re: utimensat fails to update ctime

[OGAWA Hirofumi]

Eric Blake <ebb9@byu.net> writes:

> According to OGAWA Hirofumi on 12/21/2009 8:05 AM:
>>> It may also be file-system dependent.  On the machine where I saw the
>>> original failure:
>>>> $ uname -a
>>>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
>>>> x86_64 GNU/Linux
>>> $ df -T .
>>> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
>>> /dev/sdb1      xfs   419299328 269018656 150280672  65% /srv/data
>> 
>> Thanks.
>> 
>> This is good point. This would be xfs issue or design. xfs seems to have
>> own special handling of ctime.
>
> Here's another report, this time about an mtime update not happening on
> ntfs-3g.  http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/19336

It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
people at all. So, for now, just Cc: to fuse people.

> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0

>From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
request would pass to userland via fuse of kernel part, then it will be
handled by libfuse.

>From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
using "struct fuse_operations", not "struct fuse_lowlevel_ops".

So, fuse_lib_setattr() would be the handler for it in libfuse. And, from
the following part, it seems to require the both of MTIME and ATIME to
call ntfs-3g's ->utime handler.

I don't know whether it is limitation or bug in fuse_operations.
Any ideas?

		if (!err &&
		    (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
		    (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
			struct timespec tv[2];
			tv[0].tv_sec = attr->st_atime;
			tv[0].tv_nsec = ST_ATIM_NSEC(attr);
			tv[1].tv_sec = attr->st_mtime;
			tv[1].tv_nsec = ST_MTIM_NSEC(attr);
			err = fuse_fs_utimens(f->fs, path, tv);
		}

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [fuse-devel] utimensat fails to update ctime

[Jean-Pierre André]

Hi all,


OGAWA Hirofumi wrote:
> Eric Blake<ebb9@byu.net>  writes:
>
>    
> It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
> people at all. So, for now, just Cc: to fuse people.
>    

Which ntfs-3g version are you using ?

>> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0
>>      
>    

Currently ntfs-3g does not set sub-second precision.

There is also a slight problem in the fuse interface :
the time buffer is never passed as NULL, consequently
in some circumstances ntfs-3g cannot decide correctly
over permissions. A permissive action is taken in this
situation.

> > From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
> request would pass to userland via fuse of kernel part, then it will be
> handled by libfuse.
>
> > From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
> using "struct fuse_operations", not "struct fuse_lowlevel_ops".
>    

With the latest ntfs-3g, currently as a release candidate,
you can (optionally) use the low level fuse interface
http://pagesperso-orange.fr/b.andre/advanced-ntfs-3g.html
use the "lowntfs-3g" driver instead of "ntfs-3g"

Hope this helps

Regards

Jean-Pierre

Re: [fuse-devel] utimensat fails to update ctime

[OGAWA Hirofumi]

Jean-Pierre André <jean-pierre.andre@wanadoo.fr> writes:

> Hi all,

Hi,

> OGAWA Hirofumi wrote:
>> Eric Blake<ebb9@byu.net>  writes:
>>
>>    
>> It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
>> people at all. So, for now, just Cc: to fuse people.
>>    
>
> Which ntfs-3g version are you using ?

I don't know which version is used by actual user. But, well, I've got
source by "apt-get source", and the version was 1:2009.4.4-1.

Now, I've got ntfs-3g-2009.11.14AC.2.tgz from specified url.

>>> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0
>
> Currently ntfs-3g does not set sub-second precision.
>
> There is also a slight problem in the fuse interface :
> the time buffer is never passed as NULL, consequently
> in some circumstances ntfs-3g cannot decide correctly
> over permissions. A permissive action is taken in this
> situation.
>
>> > From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
>> request would pass to userland via fuse of kernel part, then it will be
>> handled by libfuse.
>>
>> > From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
>> using "struct fuse_operations", not "struct fuse_lowlevel_ops".
>>    
>
> With the latest ntfs-3g, currently as a release candidate,
> you can (optionally) use the low level fuse interface
> http://pagesperso-orange.fr/b.andre/advanced-ntfs-3g.html
> use the "lowntfs-3g" driver instead of "ntfs-3g"

Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
(lowlevel op too).

The both functions is requiring "ATIME | MTIME". Doesn't it mean the
ntfs-3g can't set only MTIME like above utimensat()?

In fuse_lib_setattr(),

        if (!err && (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
            (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {

In ntfs_fuse_setattr(),
	case FUSE_SET_ATTR_ATIME + FUSE_SET_ATTR_MTIME :
		res = ntfs_fuse_utime(&security, ino, attr, &stbuf);

Or I'm missing something?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [fuse-devel] utimensat fails to update ctime

[Jean-Pierre André]

Hi again,

OGAWA Hirofumi wrote:
> Jean-Pierre André<jean-pierre.andre@wanadoo.fr>  writes:
>
>    
>> Hi all,
>>      
> Hi,
>
>    
>> OGAWA Hirofumi wrote:
>>      
>>> Eric Blake<ebb9@byu.net>   writes:
>>>
>>>
>>> It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
>>> people at all. So, for now, just Cc: to fuse people.
>>>
>>>        
>> Which ntfs-3g version are you using ?
>>      
> I don't know which version is used by actual user. But, well, I've got
> source by "apt-get source", and the version was 1:2009.4.4-1.
>    
> Now, I've got ntfs-3g-2009.11.14AC.2.tgz from specified url.
>
>    
>>>> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0
>>>>          
>> Currently ntfs-3g does not set sub-second precision.
>>
>> There is also a slight problem in the fuse interface :
>> the time buffer is never passed as NULL, consequently
>> in some circumstances ntfs-3g cannot decide correctly
>> over permissions. A permissive action is taken in this
>> situation.
>>
>>      
>>>>  From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
>>>>          
>>> request would pass to userland via fuse of kernel part, then it will be
>>> handled by libfuse.
>>>
>>>        
>>>>  From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
>>>>          
>>> using "struct fuse_operations", not "struct fuse_lowlevel_ops".
>>>
>>>        
>> With the latest ntfs-3g, currently as a release candidate,
>> you can (optionally) use the low level fuse interface
>> http://pagesperso-orange.fr/b.andre/advanced-ntfs-3g.html
>> use the "lowntfs-3g" driver instead of "ntfs-3g"
>>      
> Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
> (lowlevel op too).
>
> The both functions is requiring "ATIME | MTIME". Doesn't it mean the
> ntfs-3g can't set only MTIME like above utimensat()?
>    

With ntfs-3g this is not directly possible, because
the interface does not provide flags telling which
timestamps should be updated. The only way would
be fuse feeding both values (even though unchanged)
before calling ntfs-3g. This is true for all versions of
ntfs-3g.

With lowntfs-3g (release candidate only), this could
be possible.... but this is not implemented, as the
case was never found up to now. I can provide you
with a patch,... if fuse can feed in the flags selectively.

> In fuse_lib_setattr(),
>
>          if (!err&&  (valid&  (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
>              (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
>
> In ntfs_fuse_setattr(),
> 	case FUSE_SET_ATTR_ATIME + FUSE_SET_ATTR_MTIME :
> 		res = ntfs_fuse_utime(&security, ino, attr,&stbuf);
>
> Or I'm missing something?
>
> Thanks.
>    

Regards

Jean-Pierre

Re: [fuse-devel] utimensat fails to update ctime

[Miklos Szeredi]

On Tue, 22 Dec 2009, Jean-Pierre André wrote:
> With ntfs-3g this is not directly possible, because
> the interface does not provide flags telling which
> timestamps should be updated. The only way would
> be fuse feeding both values (even though unchanged)
> before calling ntfs-3g. This is true for all versions of
> ntfs-3g.

Here's a patch for the high level lib.  If the flag_utime_omit_ok flag
is set then the ->utimes() method will be supplied with arguments that
correspond to utimensat (i.e. it can have UTIME_NOW and UTIME_OMIT
values).

Thanks,
Miklos

Index: fuse/configure.in
===================================================================
--- fuse.orig/configure.in	2009-12-22 12:46:44.000000000 +0100
+++ fuse/configure.in	2009-12-22 13:16:07.000000000 +0100
@@ -53,7 +53,7 @@ fi
 if test "$enable_mtab" = "no"; then
 	AC_DEFINE(IGNORE_MTAB, 1, [Don't update /etc/mtab])
 fi
-AC_CHECK_FUNCS([fork setxattr fdatasync])
+AC_CHECK_FUNCS([fork setxattr fdatasync utimensat])
 AC_CHECK_MEMBERS([struct stat.st_atim])
 AC_CHECK_MEMBERS([struct stat.st_atimespec])
 
Index: fuse/include/fuse.h
===================================================================
--- fuse.orig/include/fuse.h	2009-12-22 12:46:44.000000000 +0100
+++ fuse/include/fuse.h	2009-12-22 13:16:07.000000000 +0100
@@ -458,9 +458,15 @@ struct fuse_operations {
 	unsigned int flag_nullpath_ok : 1;
 
 	/**
+	 * Flag indicating that the filesystem accepts special
+	 * UTIME_NOW and UTIME_OMIT values in its utimens operation.
+	 */
+	unsigned int flag_utime_omit_ok : 1;
+
+	/**
 	 * Reserved flags, don't set
 	 */
-	unsigned int flag_reserved : 31;
+	unsigned int flag_reserved : 30;
 
 	/**
 	 * Ioctl
Index: fuse/lib/fuse.c
===================================================================
--- fuse.orig/lib/fuse.c	2009-12-22 12:46:44.000000000 +0100
+++ fuse/lib/fuse.c	2009-12-22 13:16:07.000000000 +0100
@@ -97,6 +97,7 @@ struct fuse {
 	int intr_installed;
 	struct fuse_fs *fs;
 	int nullpath_ok;
+	int utime_omit_ok;
 	int curr_ticket;
 	struct lock_queue_element *lockq;
 };
@@ -2119,6 +2120,29 @@ static void fuse_lib_setattr(fuse_req_t
 				err = fuse_fs_truncate(f->fs, path,
 						       attr->st_size);
 		}
+#ifdef HAVE_UTIMENSAT
+		if (!err && f->utime_omit_ok &&
+		    (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME))) {
+			struct timespec tv[2];
+
+			tv[0].tv_sec = 0;
+			tv[1].tv_sec = 0;
+			tv[0].tv_nsec = UTIME_OMIT;
+			tv[1].tv_nsec = UTIME_OMIT;
+
+			if (valid & FUSE_SET_ATTR_ATIME_NOW)
+				tv[0].tv_nsec = UTIME_NOW;
+			else if (valid & FUSE_SET_ATTR_ATIME)
+				tv[0] = attr->st_atim;
+
+			if (valid & FUSE_SET_ATTR_MTIME_NOW)
+				tv[1].tv_nsec = UTIME_NOW;
+			else if (valid & FUSE_SET_ATTR_MTIME)
+				tv[1] = attr->st_mtim;
+
+			err = fuse_fs_utimens(f->fs, path, tv);
+		} else
+#endif
 		if (!err &&
 		    (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
 		    (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
@@ -3637,6 +3661,7 @@ static int fuse_push_module(struct fuse
 	newfs->m = m;
 	f->fs = newfs;
 	f->nullpath_ok = newfs->op.flag_nullpath_ok && f->nullpath_ok;
+	f->utime_omit_ok = newfs->op.flag_utime_omit_ok && f->utime_omit_ok;
 	return 0;
 }
 
@@ -3687,6 +3712,7 @@ struct fuse *fuse_new_common(struct fuse
 	fs->compat = compat;
 	f->fs = fs;
 	f->nullpath_ok = fs->op.flag_nullpath_ok;
+	f->utime_omit_ok = fs->op.flag_utime_omit_ok;
 
 	/* Oh f**k, this is ugly! */
 	if (!fs->op.lock) {
@@ -3743,8 +3769,10 @@ struct fuse *fuse_new_common(struct fuse
 
 	fuse_session_add_chan(f->se, ch);
 
-	if (f->conf.debug)
+	if (f->conf.debug) {
 		fprintf(stderr, "nullpath_ok: %i\n", f->nullpath_ok);
+		fprintf(stderr, "utime_omit_ok: %i\n", f->utime_omit_ok);
+	}
 
 	/* Trace topmost layer by default */
 	f->fs->debug = f->conf.debug;
Index: fuse/example/fusexmp_fh.c
===================================================================
--- fuse.orig/example/fusexmp_fh.c	2009-06-19 12:24:25.000000000 +0200
+++ fuse/example/fusexmp_fh.c	2009-12-22 13:55:04.000000000 +0100
@@ -283,6 +283,9 @@ static int xmp_ftruncate(const char *pat
 static int xmp_utimens(const char *path, const struct timespec ts[2])
 {
 	int res;
+#ifdef HAVE_UTIMENSAT
+	res = utimensat(AT_FDCWD, path, ts, AT_SYMLINK_NOFOLLOW);
+#else
 	struct timeval tv[2];
 
 	tv[0].tv_sec = ts[0].tv_sec;
@@ -291,6 +294,7 @@ static int xmp_utimens(const char *path,
 	tv[1].tv_usec = ts[1].tv_nsec / 1000;
 
 	res = utimes(path, tv);
+#endif
 	if (res == -1)
 		return -errno;
 
@@ -486,6 +490,9 @@ static struct fuse_operations xmp_oper =
 	.lock		= xmp_lock,
 
 	.flag_nullpath_ok = 1,
+#if HAVE_UTIMENSAT
+	.flag_utime_omit_ok = 1,
+#endif
 };
 
 int main(int argc, char *argv[])

Re: [fuse-devel] utimensat fails to update ctime

[OGAWA Hirofumi]

Jean-Pierre André <jean-pierre.andre@wanadoo.fr> writes:

> Hi again,

Hi,

>> Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
>> (lowlevel op too).
>>
>> The both functions is requiring "ATIME | MTIME". Doesn't it mean the
>> ntfs-3g can't set only MTIME like above utimensat()?
>>    
>
> With ntfs-3g this is not directly possible, because
> the interface does not provide flags telling which
> timestamps should be updated. The only way would
> be fuse feeding both values (even though unchanged)
> before calling ntfs-3g. This is true for all versions of
> ntfs-3g.

Yes, with fuse_operations. It is why I'm saying the issue is libfuse or
ntfs-3g.

But I noticed ntfs-3g is including libfuse-lite sources and use it with
static link (I might be wrong here, because I just looked ntfs-3g source
slightly). AFAIK, the fuse of kernel part is passing the flags of some
sort of detail always.

[BTW, the code of that part in kernel may be the following,

fs/fuse/dir.c:iattr_to_fattr(),

	if (ivalid & ATTR_ATIME) {
		arg->valid |= FATTR_ATIME;
		arg->atime = iattr->ia_atime.tv_sec;
		arg->atimensec = iattr->ia_atime.tv_nsec;
		if (!(ivalid & ATTR_ATIME_SET))
			arg->valid |= FATTR_ATIME_NOW;
	}
	if ((ivalid & ATTR_MTIME) && update_mtime(ivalid)) {
		arg->valid |= FATTR_MTIME;
		arg->mtime = iattr->ia_mtime.tv_sec;
		arg->mtimensec = iattr->ia_mtime.tv_nsec;
		if (!(ivalid & ATTR_MTIME_SET))
			arg->valid |= FATTR_MTIME_NOW;
	}
]

So, if libfuse-lite was fixed to supported that update request, it would
be able to do even if fuse_operations.  I.e. in libfuse-lite, emulate
"ATIME | MTIME" request if "MTIME" only (pass unchanged original atime),
then call ->utime() callback. (or adds new utime2 callback with flags, or
something other solutions)

Or, if that request is known limitation of fuse_operations, I think it
would be clear state and ok. The fs needs to use lowlevel op to support
that.

> With lowntfs-3g (release candidate only), this could
> be possible.... but this is not implemented, as the
> case was never found up to now. I can provide you
> with a patch,... if fuse can feed in the flags selectively.

Yes.  AFAIK, fuse of kernel part is passing FATTR_MTIME without
FATTR_ATIME to userland (i.e. FUSE_SET_ATTR_ATIME and
FUSE_SET_ATTR_MTIME in libfuse).

I think it's good to implement if it's not design decision of ntfs-3g.

[BTW, just my guess though, it would be good to use "if (vaild &
ATTR_XXX)" style, not "switch()" to support various combinations of
flags]

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [fuse-devel] utimensat fails to update ctime

[Jean-Pierre André]

Hi again,

OGAWA Hirofumi wrote:
> Jean-Pierre André<jean-pierre.andre@wanadoo.fr>  writes:
>
>    
>> Hi again,
>>      
> Hi,
>
>    
>>> Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
>>> (lowlevel op too).
>>>
>>> The both functions is requiring "ATIME | MTIME". Doesn't it mean the
>>> ntfs-3g can't set only MTIME like above utimensat()?
>>>
>>>        
>> With ntfs-3g this is not directly possible, because
>> the interface does not provide flags telling which
>> timestamps should be updated. The only way would
>> be fuse feeding both values (even though unchanged)
>> before calling ntfs-3g. This is true for all versions of
>> ntfs-3g.
>>      
> Yes, with fuse_operations. It is why I'm saying the issue is libfuse or
> ntfs-3g.
>
> But I noticed ntfs-3g is including libfuse-lite sources and use it with
> static link (I might be wrong here, because I just looked ntfs-3g source
> slightly). AFAIK, the fuse of kernel part is passing the flags of some
> sort of detail always.
>    

Use of fuse-lite is optional, and static linking also
(both depend on your configure options),

> [BTW, the code of that part in kernel may be the following,
>
> fs/fuse/dir.c:iattr_to_fattr(),
>
>    
[...]
> So, if libfuse-lite was fixed to supported that update request, it would
> be able to do even if fuse_operations.  I.e. in libfuse-lite, emulate
> "ATIME | MTIME" request if "MTIME" only (pass unchanged original atime),
> then call ->utime() callback. (or adds new utime2 callback with flags, or
> something other solutions)
>    

I will port to fuse-lite the patch which Miklos has
just sent.

> Or, if that request is known limitation of fuse_operations, I think it
> would be clear state and ok. The fs needs to use lowlevel op to support
> that.
>
>    
>> With lowntfs-3g (release candidate only), this could
>> be possible.... but this is not implemented, as the
>> case was never found up to now. I can provide you
>> with a patch,... if fuse can feed in the flags selectively.
>>      
> Yes.  AFAIK, fuse of kernel part is passing FATTR_MTIME without
> FATTR_ATIME to userland (i.e. FUSE_SET_ATTR_ATIME and
> FUSE_SET_ATTR_MTIME in libfuse).
>
> I think it's good to implement if it's not design decision of ntfs-3g.
>
> [BTW, just my guess though, it would be good to use "if (vaild&
> ATTR_XXX)" style, not "switch()" to support various combinations of
> flags]
>    

Might be better, ... or not. Setting both mtime
and atime is much simpler than setting each one
individually. So both methods will end up having
to process three different situations.

I suggest I port Miklos patch to fuse-lite soon,
and delay the low-level case (and microsecond
precision) until January. Does that suit your needs ?

Also : there is a common case of mis-configuration
in which gid is set in the mount options and uid
is not. As a consequence files appear as owned
by root and utime() is forbidden to plain users.

So you should first check your mount options.
And since ntfs-2009.11.14 standard ownership
and permissions are fully supported provided a
proper configuration file has been set.

Regards

Jean-Pierre



> Thanks.
>    

-- 
JP André
email jean-pierre.andre@wanadoo.fr

Re: [fuse-devel] utimensat fails to update ctime

[OGAWA Hirofumi]

Jean-Pierre André <jean-pierre.andre@wanadoo.fr> writes:

> Hi again,

Hi,

>> Yes.  AFAIK, fuse of kernel part is passing FATTR_MTIME without
>> FATTR_ATIME to userland (i.e. FUSE_SET_ATTR_ATIME and
>> FUSE_SET_ATTR_MTIME in libfuse).
>>
>> I think it's good to implement if it's not design decision of ntfs-3g.
>>
>> [BTW, just my guess though, it would be good to use "if (vaild&
>> ATTR_XXX)" style, not "switch()" to support various combinations of
>> flags]
>>    
>
> Might be better, ... or not. Setting both mtime
> and atime is much simpler than setting each one
> individually. So both methods will end up having
> to process three different situations.

Well, I don't care the implementation detail. However, the combination
might not be only three. E.g. if fs was exported as network fs's
backend, so many combinations are possible. So, specified known
combination can be fragile.

(e.g. ATTR_MTIME | ATTR_SIZE, etc, etc.)

> I suggest I port Miklos patch to fuse-lite soon,
> and delay the low-level case (and microsecond
> precision) until January. Does that suit your needs ?

Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
bug report on lkml to others. Eric?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [fuse-devel] utimensat fails to update ctime

[Jean-Pierre André]

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

Hi,

OGAWA Hirofumi wrote:
>> I suggest I port Miklos patch to fuse-lite soon,
>> and delay the low-level case (and microsecond
>> precision) until January. Does that suit your needs ?
>>      
> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
> bug report on lkml to others. Eric?

Attached is a proposed patch for selective update
of timestamps in ntfs-3g (based on the release
candidate version ntfs-3g-2009.11.14AC.2).

It includes the backport of Miklos's patch posted
yesterday to fuse-lite. As a matter of fact fuse-lite
is needed, because I do not know how to make this
compile with unpatched fuse.

Note : this does not yet include  sub-second precision.

Regards

Jean-Pierre




[-- Attachment #2: utimensat.patch --]
[-- Type: text/plain, Size: 6233 bytes --]

--- ntfs-3g-2009.11.14AC.2/configure.ac	2009-12-16 08:33:26.000000000 +0100
+++ ntfslow/ntfs-3g/configure.ac	2009-12-23 10:24:29.000000000 +0100
@@ -314,7 +314,7 @@
 	atexit basename daemon dup2 fdatasync ffs getopt_long hasmntopt \
 	mbsinit memmove memset realpath regcomp setlocale setxattr \
 	strcasecmp strchr strdup strerror strnlen strsep strtol strtoul \
-	sysconf utime fork \
+	sysconf utime utimensat fork \
 ])
 AC_SYS_LARGEFILE
 
--- ntfs-3g-2009.11.14AC.2/include/fuse-lite/fuse.h	2009-07-30 15:11:50.000000000 +0200
+++ ntfslow/ntfs-3g/include/fuse-lite/fuse.h	2009-12-23 08:22:10.000000000 +0100
@@ -67,7 +67,23 @@
  * Changed in fuse 2.8.0 (regardless of API version)
  * Previously, paths were limited to a length of PATH_MAX.
  */
+
+#define HAS_UTIME_OMIT_OK 1
+
 struct fuse_operations {
+ 	unsigned int flag_nullpath_ok : 1;
+ 
+ 	/**
+	 * Flag indicating that the filesystem accepts special
+	 * UTIME_NOW and UTIME_OMIT values in its utimens operation.
+	 */
+	unsigned int flag_utime_omit_ok : 1;
+
+	/**
+ 	 * Reserved flags, don't set
+ 	 */
+	unsigned int flag_reserved : 30;
+ 
 	/** Get file attributes.
 	 *
 	 * Similar to stat().  The 'st_dev' and 'st_blksize' fields are
--- ntfs-3g-2009.11.14AC.2/include/fuse-lite/fuse_lowlevel.h	2009-07-30 15:11:50.000000000 +0200
+++ ntfslow/ntfs-3g/include/fuse-lite/fuse_lowlevel.h	2009-12-22 18:24:54.000000000 +0100
@@ -114,6 +114,8 @@
 #define FUSE_SET_ATTR_SIZE	(1 << 3)
 #define FUSE_SET_ATTR_ATIME	(1 << 4)
 #define FUSE_SET_ATTR_MTIME	(1 << 5)
+#define FUSE_SET_ATTR_ATIME_NOW	(1 << 7)
+#define FUSE_SET_ATTR_MTIME_NOW	(1 << 8)
 
 /* ----------------------------------------------------------- *
  * Request methods and replies				       *
--- ntfs-3g-2009.11.14AC.2/libfuse-lite/fuse.c	2009-07-30 15:11:50.000000000 +0200
+++ ntfslow/ntfs-3g/libfuse-lite/fuse.c	2009-12-23 10:06:50.000000000 +0100
@@ -75,6 +75,7 @@
     struct fuse_config conf;
     int intr_installed;
     struct fuse_fs *fs;
+    int utime_omit_ok;
 };
 
 struct lock {
@@ -1187,6 +1188,29 @@
             else
                 err = fuse_fs_truncate(f->fs, path, attr->st_size);
         }
+#ifdef HAVE_UTIMENSAT
+        if (!err && f->utime_omit_ok &&
+            (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME))) {
+            struct timespec tv[2];
+
+            tv[0].tv_sec = 0;
+            tv[1].tv_sec = 0;
+            tv[0].tv_nsec = UTIME_OMIT;
+            tv[1].tv_nsec = UTIME_OMIT;
+
+            if (valid & FUSE_SET_ATTR_ATIME_NOW)
+                tv[0].tv_nsec = UTIME_NOW;
+            else if (valid & FUSE_SET_ATTR_ATIME)
+                tv[0] = attr->st_atim;
+
+            if (valid & FUSE_SET_ATTR_MTIME_NOW)
+                tv[1].tv_nsec = UTIME_NOW;
+            else if (valid & FUSE_SET_ATTR_MTIME)
+                tv[1] = attr->st_mtim;
+
+            err = fuse_fs_utimens(f->fs, path, tv);
+        } else
+#endif
         if (!err && (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
             (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
             struct timespec tv[2];
@@ -2606,6 +2630,7 @@
         goto out_free;
 
     f->fs = fs;
+    f->utime_omit_ok = fs->op.flag_utime_omit_ok;
 
     /* Oh f**k, this is ugly! */
     if (!fs->op.lock) {
@@ -2639,6 +2664,8 @@
 
     fuse_session_add_chan(f->se, ch);
 
+    if (f->conf.debug)
+        fprintf(stderr, "utime_omit_ok: %i\n", f->utime_omit_ok);
     f->ctr = 0;
     f->generation = 0;
     /* FIXME: Dynamic hash table */
--- ntfs-3g-2009.11.14AC.2/src/ntfs-3g.c	2009-12-16 08:33:25.000000000 +0100
+++ ntfslow/ntfs-3g/src/ntfs-3g.c	2009-12-23 10:28:15.000000000 +0100
@@ -2171,6 +2171,63 @@
 	return ntfs_fuse_rm(path);
 }
 
+#ifdef HAVE_UTIMENSAT
+
+static int ntfs_fuse_utimens(const char *path, const struct timespec tv[2])
+{
+	ntfs_inode *ni;
+	int res = 0;
+#if !KERNELPERMS | (POSIXACLS & !KERNELACLS)
+	struct SECURITY_CONTEXT security;
+#endif
+
+	if (ntfs_fuse_is_named_data_stream(path))
+		return -EINVAL; /* n/a for named data streams. */
+#if !KERNELPERMS | (POSIXACLS & !KERNELACLS)
+		   /* parent directory must be executable */
+	if (ntfs_fuse_fill_security_context(&security)
+	    && !ntfs_allowed_dir_access(&security,path,
+			(ntfs_inode*)NULL,(ntfs_inode*)NULL,S_IEXEC)) {
+		return (-errno);
+	}
+#endif
+	ni = ntfs_pathname_to_inode(ctx->vol, NULL, path);
+	if (!ni)
+		return -errno;
+
+			/* no check or update if both UTIME_OMIT */
+	if ((tv[0].tv_nsec != UTIME_OMIT) || (tv[1].tv_nsec != UTIME_OMIT)) {
+#if !KERNELPERMS | (POSIXACLS & !KERNELACLS)
+		if (ntfs_allowed_access(&security, ni, S_IWRITE)
+		    || ((tv[0].tv_nsec == UTIME_NOW)
+			&& (tv[0].tv_nsec == UTIME_NOW)
+			&& ntfs_allowed_as_owner(&security, ni))) {
+#endif
+			ntfs_time_update_flags mask = NTFS_UPDATE_CTIME;
+
+			if (tv[0].tv_nsec == UTIME_NOW)
+				mask |= NTFS_UPDATE_ATIME;
+			else
+				if (tv[0].tv_nsec != UTIME_OMIT)
+					ni->last_access_time = tv[0].tv_sec;
+			if (tv[1].tv_nsec == UTIME_NOW)
+				mask |= NTFS_UPDATE_MTIME;
+			else
+				if (tv[1].tv_nsec != UTIME_OMIT)
+					ni->last_data_change_time = tv[1].tv_sec;
+			ntfs_inode_update_times(ni, mask);
+#if !KERNELPERMS | (POSIXACLS & !KERNELACLS)
+		} else
+			res = -errno;
+#endif
+	}
+	if (ntfs_inode_close(ni))
+		set_fuse_error(&res);
+	return res;
+}
+
+#else /* HAVE_UTIMENSAT */
+
 static int ntfs_fuse_utime(const char *path, struct utimbuf *buf)
 {
 	ntfs_inode *ni;
@@ -2238,6 +2295,8 @@
 	return res;
 }
 
+#endif /* HAVE_UTIMENSAT */
+
 static int ntfs_fuse_bmap(const char *path, size_t blocksize, uint64_t *idx)
 {
 	ntfs_inode *ni;
@@ -3372,6 +3431,9 @@
 }
 
 static struct fuse_operations ntfs_3g_ops = {
+#if defined(HAVE_UTIMENSAT) && HAS_UTIME_OMIT_OK
+	.flag_utime_omit_ok = 1, /* accept UTIME_NOW and UTIME_OMIT in utimens */
+#endif
 	.getattr	= ntfs_fuse_getattr,
 	.readlink	= ntfs_fuse_readlink,
 	.readdir	= ntfs_fuse_readdir,
@@ -3392,7 +3454,11 @@
 	.rename		= ntfs_fuse_rename,
 	.mkdir		= ntfs_fuse_mkdir,
 	.rmdir		= ntfs_fuse_rmdir,
+#ifdef HAVE_UTIMENSAT
+	.utimens	= ntfs_fuse_utimens,
+#else
 	.utime		= ntfs_fuse_utime,
+#endif
 	.bmap		= ntfs_fuse_bmap,
 	.destroy        = ntfs_fuse_destroy2,
 #if !KERNELPERMS | (POSIXACLS & !KERNELACLS)

Re: [fuse-devel] utimensat fails to update ctime

[OGAWA Hirofumi]

Jean-Pierre André <jean-pierre.andre@wanadoo.fr> writes:

> Hi,

Hi,

> OGAWA Hirofumi wrote:
>>> I suggest I port Miklos patch to fuse-lite soon,
>>> and delay the low-level case (and microsecond
>>> precision) until January. Does that suit your needs ?
>>>      
>> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
>> bug report on lkml to others. Eric?
>
> Attached is a proposed patch for selective update
> of timestamps in ntfs-3g (based on the release
> candidate version ntfs-3g-2009.11.14AC.2).
>
> It includes the backport of Miklos's patch posted
> yesterday to fuse-lite. As a matter of fact fuse-lite
> is needed, because I do not know how to make this
> compile with unpatched fuse.

Thanks for fixing this.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [fuse-devel] utimensat fails to update ctime

[Eric Blake]

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

According to OGAWA Hirofumi on 12/22/2009 10:58 AM:
>> I suggest I port Miklos patch to fuse-lite soon,
>> and delay the low-level case (and microsecond
>> precision) until January. Does that suit your needs ?
> 
> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
> bug report on lkml to others. Eric?

I'm also bridging the report from a coreutils user (now cc'd).  Since I
also don't use ntfs-3g, I'm hoping that ctrn3e8 will be able to help test
whether the latest patch to ntfs-3g makes a difference in properly setting
times.  To me, delaying precision while fixing UTIME_OMIT semantics is a
reasonable approach.

By the way, is there any reliable way, other than uname() and checking for
a minimum kernel version, to tell if all file systems will properly
support UTIME_OMIT?  For coreutils 8.3, we will be inserting a workaround
where instead of using UTIME_OMIT, we call fstatat() in advance of
utimensat() and pass the original timestamp down.  But it would be nice to
avoid the penalty of the extra stat if there were a reliable way to ensure
that, regardless of file system, the use of UTIME_OMIT will be honored.
After all, coreutils wants touch(1) to work regardless of how old the
user's kernel and file system drivers are.

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

Eric Blake             ebb9@byu.net
-----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/

iEYEARECAAYFAksyEu0ACgkQ84KuGfSFAYCrzACgirIjqmS7vFOBcI8xau6jHEa0
4L0AnAjJkje+tSMF/FZkTbkohg/fhQ+i
=ngx0
-----END PGP SIGNATURE-----

Re: [fuse-devel] utimensat fails to update ctime

[OGAWA Hirofumi]

Eric Blake <ebb9@byu.net> writes:

> By the way, is there any reliable way, other than uname() and checking for
> a minimum kernel version, to tell if all file systems will properly
> support UTIME_OMIT?

Um... sorry, I don't know. And it might be hard to detect efficiently if
the workaround is enough efficient like one fstat() syscall (Pass fd to
kernel.  I.e. just read from cached inode).

> For coreutils 8.3, we will be inserting a workaround where instead of
> using UTIME_OMIT, we call fstatat() in advance of utimensat() and pass
> the original timestamp down.  But it would be nice to avoid the
> penalty of the extra stat if there were a reliable way to ensure that,
> regardless of file system, the use of UTIME_OMIT will be honored.
> After all, coreutils wants touch(1) to work regardless of how old the
> user's kernel and file system drivers are.

Or it would depend on coreutils policy though, personally I think it's
ok that it ignores the bug as known fs bug, otherwise coreutils would
need to collect workarounds on several filesystems of several OSes.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [fuse-devel] utimensat fails to update ctime

[Eric Blake]

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

According to ctrn3e8 on 12/23/2009 5:17 PM:
> The strace has the following function call  (and it may be because I am
> looking at the trace rather than the actual source):

> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0

> The two don't seem to match.  Is this just because of the way the trace is printed?

Yes.  When the tv_nsec field is UTIME_OMIT or UTIME_NOW, the tv_sec field
is irrelevant.  Therefore, to save on space, strace omits the tv_sec field
in its output.  But rest assured that the kernel has read access to all
four 32-bit words located at the timespec pointer passed in the syscall.

> No mention of ntfs-3g support for nanosecond time stamping.

Read the rest of the thread on lkml - that is a known issue, which will
probably not be solved any sooner than January (all the patches this week
only dealt with mishandling of UTIME_OMIT).

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

Eric Blake             ebb9@byu.net
-----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/

iEYEARECAAYFAksyurwACgkQ84KuGfSFAYDJ2gCgv9YMVwl7HL//ThRvQKJH5hSR
S/EAn0WzRr7FrFbkDHUtEfRdtXDdkqxT
=YpCl
-----END PGP SIGNATURE-----

Re: [fuse-devel] utimensat fails to update ctime

[Jean-Pierre André]

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

Hi,

OGAWA Hirofumi wrote:
>> I suggest I port Miklos patch to fuse-lite soon,
>> and delay the low-level case (and microsecond
>> precision) until January. Does that suit your needs ?
>>      
> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
> bug report on lkml to others. Eric?

As my Christmas present to any real non-bridging
user, here is, ahead of schedule, the patch for the
low level fuse interface of ntfs-3g.

The good news is that for this interface, no update
of fuse is needed. So there is no compilation issue with
unpatched versions of fuse, and the attached patch
is independent of the one for the high-level interface
posted previously, except for the configuration.ac file
for which I duplicate the patch.

Regards

Jean-Pierre




[-- Attachment #2: lowutimensat.patch --]
[-- Type: text/plain, Size: 5838 bytes --]

--- ntfs-3g-2009.11.14AC.2/configure.ac	2009-12-16 08:33:26.000000000 +0100
+++ ntfslow/configure.ac	2009-12-23 10:24:29.000000000 +0100
@@ -314,7 +314,7 @@
 	atexit basename daemon dup2 fdatasync ffs getopt_long hasmntopt \
 	mbsinit memmove memset realpath regcomp setlocale setxattr \
 	strcasecmp strchr strdup strerror strnlen strsep strtol strtoul \
-	sysconf utime fork \
+	sysconf utime utimensat fork \
 ])
 AC_SYS_LARGEFILE
 
--- ntfs-3g-2009.11.14AC.2/src/lowntfs-3g.c	2009-12-19 09:58:22.000000000 +0100
+++ ntfslow/src/lowntfs-3g.c	2009-12-23 15:06:14.000000000 +0100
@@ -1527,8 +1527,54 @@
 	return res;
 }
 
+#ifdef HAVE_UTIMENSAT
+
+static int ntfs_fuse_utimens(struct SECURITY_CONTEXT *scx, fuse_ino_t ino,
+		struct stat *stin, struct stat *stbuf, int to_set)
+{
+	ntfs_inode *ni;
+	int res = 0;
+
+	ni = ntfs_inode_open(ctx->vol, INODE(ino));
+	if (!ni)
+		return -errno;
+
+			/* no check or update if both UTIME_OMIT */
+	if (to_set & (FUSE_SET_ATTR_ATIME + FUSE_SET_ATTR_MTIME)) {
+#if !KERNELPERMS | (POSIXACLS & !KERNELACLS)
+		if (ntfs_allowed_access(scx, ni, S_IWRITE)
+		    || ((to_set & FUSE_SET_ATTR_ATIME_NOW)
+			&& (to_set & FUSE_SET_ATTR_MTIME_NOW)
+			&& ntfs_allowed_as_owner(scx, ni))) {
+#endif
+			ntfs_time_update_flags mask = NTFS_UPDATE_CTIME;
+
+			if (to_set & FUSE_SET_ATTR_ATIME_NOW)
+				mask |= NTFS_UPDATE_ATIME;
+			else
+				if (to_set & FUSE_SET_ATTR_ATIME)
+					ni->last_access_time = stin->st_atime;
+			if (to_set & FUSE_SET_ATTR_MTIME_NOW)
+				mask |= NTFS_UPDATE_MTIME;
+			else
+				if (to_set & FUSE_SET_ATTR_MTIME)
+					ni->last_data_change_time = stin->st_mtime;;
+			ntfs_inode_update_times(ni, mask);
+#if !KERNELPERMS | (POSIXACLS & !KERNELACLS)
+		} else
+			res = -errno;
+#endif
+	}
+	res = ntfs_fuse_getstat(scx, ni, stbuf);
+	if (ntfs_inode_close(ni))
+		set_fuse_error(&res);
+	return res;
+}
+
+#else /* HAVE_UTIMENSAT */
+
 static int ntfs_fuse_utime(struct SECURITY_CONTEXT *scx, fuse_ino_t ino,
-		struct stat *stin, struct stat *stbuf)
+		struct stat *stin, struct stat *stbuf, int to_set)
 {
 	ntfs_inode *ni;
 	int res = 0;
@@ -1586,6 +1632,8 @@
 	return res;
 }
 
+#endif /* HAVE_UTIMENSAT */
+
 static void ntfs_fuse_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
 			 int to_set, struct fuse_file_info *fi __attribute__((unused)))
 {
@@ -1594,13 +1642,14 @@
 	int res;
 	struct SECURITY_CONTEXT security;
 
+	res = 0;
 	ntfs_fuse_fill_security_context(req, &security);
-	switch (to_set
+						/* no flags */
+	if (!(to_set
 		    & (FUSE_SET_ATTR_MODE
 			| FUSE_SET_ATTR_UID | FUSE_SET_ATTR_GID
 			| FUSE_SET_ATTR_SIZE
-			| FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
-	case 0 :
+			| FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME))) {
 		ni = ntfs_inode_open(ctx->vol, INODE(ino));
 		if (!ni)
 			res = -errno;
@@ -1609,46 +1658,60 @@
 			if (ntfs_inode_close(ni))
 				set_fuse_error(&res);
 		}
-		break;
-	case FUSE_SET_ATTR_MODE :
-		res = ntfs_fuse_chmod(&security, ino, attr->st_mode & 07777,
-					&stbuf);
-		break;
-	case FUSE_SET_ATTR_UID :
-		res = ntfs_fuse_chown(&security, ino, attr->st_uid,
-					(gid_t)-1, &stbuf);
-		break;
-	case FUSE_SET_ATTR_GID :
-		res = ntfs_fuse_chown(&security, ino, (uid_t)-1,
-					attr->st_gid, &stbuf);
-		break;
-	case FUSE_SET_ATTR_UID + FUSE_SET_ATTR_GID :
-		res = ntfs_fuse_chown(&security, ino, attr->st_uid,
-					attr->st_gid, &stbuf);
-		break;
-	case FUSE_SET_ATTR_UID + FUSE_SET_ATTR_MODE:
-		res = ntfs_fuse_chownmod(&security, ino, attr->st_uid,
-					(gid_t)-1,attr->st_mode, &stbuf);
-		break;
-	case FUSE_SET_ATTR_GID + FUSE_SET_ATTR_MODE:
-		res = ntfs_fuse_chownmod(&security, ino, (uid_t)-1,
-					attr->st_gid,attr->st_mode, &stbuf);
-		break;
-	case FUSE_SET_ATTR_UID + FUSE_SET_ATTR_GID + FUSE_SET_ATTR_MODE:
-		res = ntfs_fuse_chownmod(&security, ino, attr->st_uid,
+	}
+						/* some set of uid/gid/mode */
+	if (to_set
+		    & (FUSE_SET_ATTR_MODE
+			| FUSE_SET_ATTR_UID | FUSE_SET_ATTR_GID)) {
+		switch (to_set
+			    & (FUSE_SET_ATTR_MODE
+				| FUSE_SET_ATTR_UID | FUSE_SET_ATTR_GID)) {
+		case FUSE_SET_ATTR_MODE :
+			res = ntfs_fuse_chmod(&security, ino,
+						attr->st_mode & 07777, &stbuf);
+			break;
+		case FUSE_SET_ATTR_UID :
+			res = ntfs_fuse_chown(&security, ino, attr->st_uid,
+						(gid_t)-1, &stbuf);
+			break;
+		case FUSE_SET_ATTR_GID :
+			res = ntfs_fuse_chown(&security, ino, (uid_t)-1,
+						attr->st_gid, &stbuf);
+			break;
+		case FUSE_SET_ATTR_UID + FUSE_SET_ATTR_GID :
+			res = ntfs_fuse_chown(&security, ino, attr->st_uid,
+						attr->st_gid, &stbuf);
+			break;
+		case FUSE_SET_ATTR_UID + FUSE_SET_ATTR_MODE:
+			res = ntfs_fuse_chownmod(&security, ino, attr->st_uid,
+						(gid_t)-1,attr->st_mode,
+						&stbuf);
+			break;
+		case FUSE_SET_ATTR_GID + FUSE_SET_ATTR_MODE:
+			res = ntfs_fuse_chownmod(&security, ino, (uid_t)-1,
+						attr->st_gid,attr->st_mode,
+						&stbuf);
+			break;
+		case FUSE_SET_ATTR_UID + FUSE_SET_ATTR_GID + FUSE_SET_ATTR_MODE:
+			res = ntfs_fuse_chownmod(&security, ino, attr->st_uid,
 					attr->st_gid,attr->st_mode, &stbuf);
-		break;
-	case FUSE_SET_ATTR_SIZE :
+			break;
+		default :
+			break;
+		}
+	}
+						/* size */
+	if (!res && (to_set & FUSE_SET_ATTR_SIZE)) {
 		res = ntfs_fuse_trunc(&security, ino, attr->st_size,
 					!fi, &stbuf);
-		break;
-	case FUSE_SET_ATTR_ATIME + FUSE_SET_ATTR_MTIME :
+	}
+						/* some set of atime/mtime */
+	if (!res && (to_set & (FUSE_SET_ATTR_ATIME + FUSE_SET_ATTR_MTIME))) {
+#ifdef HAVE_UTIMENSAT
+		res = ntfs_fuse_utimens(&security, ino, attr, &stbuf, to_set);
+#else /* HAVE_UTIMENSAT */
 		res = ntfs_fuse_utime(&security, ino, attr, &stbuf);
-		break;
-	default:
-		ntfs_log_error("Unsupported setattr mode 0x%x\n",(int)to_set);
-		res = -EOPNOTSUPP;
-		break;
+#endif /* HAVE_UTIMENSAT */
 	}
 	if (res)
 		fuse_reply_err(req, -res);

Re: utimensat fails to update ctime

[Dave Chinner]

On Mon, Dec 21, 2009 at 09:37:24PM -0700, Eric Blake wrote:
> According to OGAWA Hirofumi on 12/21/2009 8:05 AM:
> >> It may also be file-system dependent.  On the machine where I saw the
> >> original failure:
> >>> $ uname -a
> >>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
> >>> x86_64 GNU/Linux
> >> $ df -T .
> >> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
> >> /dev/sdb1      xfs   419299328 269018656 150280672  65% /srv/data
> > 
> > Thanks.
> > 
> > This is good point. This would be xfs issue or design. xfs seems to have
> > own special handling of ctime.

Yeah, it looks like the change to utimesat() back in 2.6.26 for
posix conformance made ATTR_CTIME appear outside inode truncation
and XFS wasn't updated for this change in behaviour at the VFS level.
Looks simple to fix, but I'm worried about introducing other
unintended ctime modifications - is there a test suite that checks
posix compliant atime/mtime/ctime behaviour around anywhere?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: utimensat fails to update ctime

[Eric Blake]

According to Dave Chinner on 12/22/2009 5:34 AM:
> Yeah, it looks like the change to utimesat() back in 2.6.26 for
> posix conformance made ATTR_CTIME appear outside inode truncation
> and XFS wasn't updated for this change in behaviour at the VFS level.
> Looks simple to fix, but I'm worried about introducing other
> unintended ctime modifications - is there a test suite that checks
> posix compliant atime/mtime/ctime behaviour around anywhere?

Yes - the gnulib unit test, consisting of:

http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/nap.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens-common.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-lutimens.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.c
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimensat.c

Taken together, these files produce the executables test-futimens and
test-utimensat which can demonstrate compliance with POSIX.  They are also
bundled as part of GNU coreutils (the version bundled with coreutils 8.2
didn't test for ctime compliance, and coreutils 8.3 hasn't been released
yet) if you use 'make -C gnulib-tests check'.

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

Eric Blake             ebb9@byu.net

Re: utimensat fails to update ctime

[Christoph Hellwig]

On Tue, Dec 22, 2009 at 05:42:49AM -0700, Eric Blake wrote:
> According to Dave Chinner on 12/22/2009 5:34 AM:
> > Yeah, it looks like the change to utimesat() back in 2.6.26 for
> > posix conformance made ATTR_CTIME appear outside inode truncation
> > and XFS wasn't updated for this change in behaviour at the VFS level.
> > Looks simple to fix, but I'm worried about introducing other
> > unintended ctime modifications - is there a test suite that checks
> > posix compliant atime/mtime/ctime behaviour around anywhere?
> 
> Yes - the gnulib unit test, consisting of:
> 
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/nap.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens-common.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-lutimens.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.c
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimensat.c
> 
> Taken together, these files produce the executables test-futimens and
> test-utimensat which can demonstrate compliance with POSIX.  They are also
> bundled as part of GNU coreutils (the version bundled with coreutils 8.2
> didn't test for ctime compliance, and coreutils 8.3 hasn't been released
> yet) if you use 'make -C gnulib-tests check'.

Ok, I'll see if I can add the last GPLv2 licensed version into xfstests.

Re: utimensat fails to update ctime

[Christoph Hellwig]

This patch which I had in my QA queue for a while should fix all the
timestamp update issues in XFS:


Index: linux-2.6/fs/xfs/xfs_vnodeops.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_vnodeops.c	2009-12-21 16:24:28.470990760 +0100
+++ linux-2.6/fs/xfs/xfs_vnodeops.c	2009-12-21 16:32:41.348990760 +0100
@@ -70,7 +70,6 @@ xfs_setattr(
 	uint			commit_flags=0;
 	uid_t			uid=0, iuid=0;
 	gid_t			gid=0, igid=0;
-	int			timeflags = 0;
 	struct xfs_dquot	*udqp, *gdqp, *olddquot1, *olddquot2;
 	int			need_iolock = 1;
 
@@ -135,16 +134,13 @@ xfs_setattr(
 	if (flags & XFS_ATTR_NOLOCK)
 		need_iolock = 0;
 	if (!(mask & ATTR_SIZE)) {
-		if ((mask != (ATTR_CTIME|ATTR_ATIME|ATTR_MTIME)) ||
-		    (mp->m_flags & XFS_MOUNT_WSYNC)) {
-			tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
-			commit_flags = 0;
-			if ((code = xfs_trans_reserve(tp, 0,
-						     XFS_ICHANGE_LOG_RES(mp), 0,
-						     0, 0))) {
-				lock_flags = 0;
-				goto error_return;
-			}
+		tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
+		commit_flags = 0;
+		code = xfs_trans_reserve(tp, 0, XFS_ICHANGE_LOG_RES(mp),
+					 0, 0, 0);
+		if (code) {
+			lock_flags = 0;
+			goto error_return;
 		}
 	} else {
 		if (DM_EVENT_ENABLED(ip, DM_EVENT_TRUNCATE) &&
@@ -295,15 +291,23 @@ xfs_setattr(
 		 * or we are explicitly asked to change it. This handles
 		 * the semantic difference between truncate() and ftruncate()
 		 * as implemented in the VFS.
-		 */
-		if (iattr->ia_size != ip->i_size || (mask & ATTR_CTIME))
-			timeflags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
+		 *
+		 * The regular truncate() case without ATTR_CTIME and ATTR_MTIME
+		 * is a special case where we need to update the times despite
+		 * not having these flags set.  For all other operations the
+		 * VFS set these flags explicitly if it wants a timestamp
+		 * update.
+		 */
+		if (iattr->ia_size != ip->i_size &&
+		    (!(mask & (ATTR_CTIME | ATTR_MTIME)))) {
+			iattr->ia_ctime = iattr->ia_mtime =
+				current_fs_time(inode->i_sb);
+			mask |= ATTR_CTIME | ATTR_MTIME;
+		}
 
 		if (iattr->ia_size > ip->i_size) {
 			ip->i_d.di_size = iattr->ia_size;
 			ip->i_size = iattr->ia_size;
-			if (!(flags & XFS_ATTR_DMI))
-				xfs_ichgtime(ip, XFS_ICHGTIME_CHG);
 			xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 		} else if (iattr->ia_size <= ip->i_size ||
 			   (iattr->ia_size == 0 && ip->i_d.di_nextents)) {
@@ -374,9 +378,6 @@ xfs_setattr(
 			ip->i_d.di_gid = gid;
 			inode->i_gid = gid;
 		}
-
-		xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
-		timeflags |= XFS_ICHGTIME_CHG;
 	}
 
 	/*
@@ -393,51 +394,37 @@ xfs_setattr(
 
 		inode->i_mode &= S_IFMT;
 		inode->i_mode |= mode & ~S_IFMT;
-
-		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-		timeflags |= XFS_ICHGTIME_CHG;
 	}
 
 	/*
 	 * Change file access or modified times.
 	 */
-	if (mask & (ATTR_ATIME|ATTR_MTIME)) {
-		if (mask & ATTR_ATIME) {
-			inode->i_atime = iattr->ia_atime;
-			ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
-			ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
-			ip->i_update_core = 1;
-		}
-		if (mask & ATTR_MTIME) {
-			inode->i_mtime = iattr->ia_mtime;
-			ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
-			ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
-			timeflags &= ~XFS_ICHGTIME_MOD;
-			timeflags |= XFS_ICHGTIME_CHG;
-		}
-		if (tp && (mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)))
-			xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
+	if (mask & ATTR_ATIME) {
+		inode->i_atime = iattr->ia_atime;
+		ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
+		ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
+		ip->i_update_core = 1;
 	}
-
-	/*
-	 * Change file inode change time only if ATTR_CTIME set
-	 * AND we have been called by a DMI function.
-	 */
-
-	if ((flags & XFS_ATTR_DMI) && (mask & ATTR_CTIME)) {
+	if (mask & ATTR_CTIME) {
 		inode->i_ctime = iattr->ia_ctime;
 		ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec;
 		ip->i_d.di_ctime.t_nsec = iattr->ia_ctime.tv_nsec;
 		ip->i_update_core = 1;
-		timeflags &= ~XFS_ICHGTIME_CHG;
+	}
+	if (mask & ATTR_MTIME) {
+		inode->i_mtime = iattr->ia_mtime;
+		ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
+		ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
+		ip->i_update_core = 1;
 	}
 
 	/*
-	 * Send out timestamp changes that need to be set to the
-	 * current time.  Not done when called by a DMI function.
+	 * And finally, log the inode core if any attribute in it
+	 * has been changed.
 	 */
-	if (timeflags && !(flags & XFS_ATTR_DMI))
-		xfs_ichgtime(ip, timeflags);
+	if (mask & (ATTR_UID|ATTR_GID|ATTR_MODE|
+		    ATTR_ATIME|ATTR_CTIME|ATTR_MTIME))
+		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
 	XFS_STATS_INC(xs_ig_attrchg);
 
@@ -452,12 +439,10 @@ xfs_setattr(
 	 * mix so this probably isn't worth the trouble to optimize.
 	 */
 	code = 0;
-	if (tp) {
-		if (mp->m_flags & XFS_MOUNT_WSYNC)
-			xfs_trans_set_sync(tp);
+	if (mp->m_flags & XFS_MOUNT_WSYNC)
+		xfs_trans_set_sync(tp);
 
-		code = xfs_trans_commit(tp, commit_flags);
-	}
+	code = xfs_trans_commit(tp, commit_flags);
 
 	xfs_iunlock(ip, lock_flags);
 
Index: linux-2.6/fs/xfs/linux-2.6/xfs_acl.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_acl.c	2009-12-21 16:31:51.838990759 +0100
+++ linux-2.6/fs/xfs/linux-2.6/xfs_acl.c	2009-12-21 16:32:17.471990759 +0100
@@ -251,8 +251,9 @@ xfs_set_mode(struct inode *inode, mode_t
 	if (mode != inode->i_mode) {
 		struct iattr iattr;
 
-		iattr.ia_valid = ATTR_MODE;
+		iattr.ia_valid = ATTR_MODE | ATTR_CTIME;
 		iattr.ia_mode = mode;
+		iattr.ia_ctime = current_fs_time(inode->i_sb);
 
 		error = -xfs_setattr(XFS_I(inode), &iattr, XFS_ATTR_NOACL);
 	}

Re: utimensat fails to update ctime

[OGAWA Hirofumi]

Christoph Hellwig <hch@lst.de> writes:

> This patch which I had in my QA queue for a while should fix all the
> timestamp update issues in XFS:

Thanks for fixing this.

> Index: linux-2.6/fs/xfs/xfs_vnodeops.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/xfs_vnodeops.c	2009-12-21 16:24:28.470990760 +0100
> +++ linux-2.6/fs/xfs/xfs_vnodeops.c	2009-12-21 16:32:41.348990760 +0100
> @@ -70,7 +70,6 @@ xfs_setattr(
>  	uint			commit_flags=0;
>  	uid_t			uid=0, iuid=0;
>  	gid_t			gid=0, igid=0;
> -	int			timeflags = 0;
>  	struct xfs_dquot	*udqp, *gdqp, *olddquot1, *olddquot2;
>  	int			need_iolock = 1;
>  
> @@ -135,16 +134,13 @@ xfs_setattr(
>  	if (flags & XFS_ATTR_NOLOCK)
>  		need_iolock = 0;
>  	if (!(mask & ATTR_SIZE)) {
> -		if ((mask != (ATTR_CTIME|ATTR_ATIME|ATTR_MTIME)) ||
> -		    (mp->m_flags & XFS_MOUNT_WSYNC)) {
> -			tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
> -			commit_flags = 0;
> -			if ((code = xfs_trans_reserve(tp, 0,
> -						     XFS_ICHANGE_LOG_RES(mp), 0,
> -						     0, 0))) {
> -				lock_flags = 0;
> -				goto error_return;
> -			}
> +		tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
> +		commit_flags = 0;
> +		code = xfs_trans_reserve(tp, 0, XFS_ICHANGE_LOG_RES(mp),
> +					 0, 0, 0);
> +		if (code) {
> +			lock_flags = 0;
> +			goto error_return;
>  		}
>  	} else {
>  		if (DM_EVENT_ENABLED(ip, DM_EVENT_TRUNCATE) &&
> @@ -295,15 +291,23 @@ xfs_setattr(
>  		 * or we are explicitly asked to change it. This handles
>  		 * the semantic difference between truncate() and ftruncate()
>  		 * as implemented in the VFS.
> -		 */
> -		if (iattr->ia_size != ip->i_size || (mask & ATTR_CTIME))
> -			timeflags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
> +		 *
> +		 * The regular truncate() case without ATTR_CTIME and ATTR_MTIME
> +		 * is a special case where we need to update the times despite
> +		 * not having these flags set.  For all other operations the
> +		 * VFS set these flags explicitly if it wants a timestamp
> +		 * update.
> +		 */
> +		if (iattr->ia_size != ip->i_size &&
> +		    (!(mask & (ATTR_CTIME | ATTR_MTIME)))) {
> +			iattr->ia_ctime = iattr->ia_mtime =
> +				current_fs_time(inode->i_sb);
> +			mask |= ATTR_CTIME | ATTR_MTIME;
> +		}
>  
>  		if (iattr->ia_size > ip->i_size) {
>  			ip->i_d.di_size = iattr->ia_size;
>  			ip->i_size = iattr->ia_size;
> -			if (!(flags & XFS_ATTR_DMI))
> -				xfs_ichgtime(ip, XFS_ICHGTIME_CHG);
>  			xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>  		} else if (iattr->ia_size <= ip->i_size ||
>  			   (iattr->ia_size == 0 && ip->i_d.di_nextents)) {
> @@ -374,9 +378,6 @@ xfs_setattr(
>  			ip->i_d.di_gid = gid;
>  			inode->i_gid = gid;
>  		}
> -
> -		xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
> -		timeflags |= XFS_ICHGTIME_CHG;
>  	}
>  
>  	/*
> @@ -393,51 +394,37 @@ xfs_setattr(
>  
>  		inode->i_mode &= S_IFMT;
>  		inode->i_mode |= mode & ~S_IFMT;
> -
> -		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> -		timeflags |= XFS_ICHGTIME_CHG;
>  	}
>  
>  	/*
>  	 * Change file access or modified times.
>  	 */
> -	if (mask & (ATTR_ATIME|ATTR_MTIME)) {
> -		if (mask & ATTR_ATIME) {
> -			inode->i_atime = iattr->ia_atime;
> -			ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
> -			ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
> -			ip->i_update_core = 1;
> -		}
> -		if (mask & ATTR_MTIME) {
> -			inode->i_mtime = iattr->ia_mtime;
> -			ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
> -			ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
> -			timeflags &= ~XFS_ICHGTIME_MOD;
> -			timeflags |= XFS_ICHGTIME_CHG;
> -		}
> -		if (tp && (mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)))
> -			xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
> +	if (mask & ATTR_ATIME) {
> +		inode->i_atime = iattr->ia_atime;
> +		ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
> +		ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
> +		ip->i_update_core = 1;
>  	}
> -
> -	/*
> -	 * Change file inode change time only if ATTR_CTIME set
> -	 * AND we have been called by a DMI function.
> -	 */
> -
> -	if ((flags & XFS_ATTR_DMI) && (mask & ATTR_CTIME)) {
> +	if (mask & ATTR_CTIME) {
>  		inode->i_ctime = iattr->ia_ctime;
>  		ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec;
>  		ip->i_d.di_ctime.t_nsec = iattr->ia_ctime.tv_nsec;
>  		ip->i_update_core = 1;
> -		timeflags &= ~XFS_ICHGTIME_CHG;
> +	}
> +	if (mask & ATTR_MTIME) {
> +		inode->i_mtime = iattr->ia_mtime;
> +		ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
> +		ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
> +		ip->i_update_core = 1;
>  	}
>  
>  	/*
> -	 * Send out timestamp changes that need to be set to the
> -	 * current time.  Not done when called by a DMI function.
> +	 * And finally, log the inode core if any attribute in it
> +	 * has been changed.
>  	 */
> -	if (timeflags && !(flags & XFS_ATTR_DMI))
> -		xfs_ichgtime(ip, timeflags);
> +	if (mask & (ATTR_UID|ATTR_GID|ATTR_MODE|
> +		    ATTR_ATIME|ATTR_CTIME|ATTR_MTIME))
> +		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>  
>  	XFS_STATS_INC(xs_ig_attrchg);
>  
> @@ -452,12 +439,10 @@ xfs_setattr(
>  	 * mix so this probably isn't worth the trouble to optimize.
>  	 */
>  	code = 0;
> -	if (tp) {
> -		if (mp->m_flags & XFS_MOUNT_WSYNC)
> -			xfs_trans_set_sync(tp);
> +	if (mp->m_flags & XFS_MOUNT_WSYNC)
> +		xfs_trans_set_sync(tp);
>  
> -		code = xfs_trans_commit(tp, commit_flags);
> -	}
> +	code = xfs_trans_commit(tp, commit_flags);
>  
>  	xfs_iunlock(ip, lock_flags);
>  
> Index: linux-2.6/fs/xfs/linux-2.6/xfs_acl.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_acl.c	2009-12-21 16:31:51.838990759 +0100
> +++ linux-2.6/fs/xfs/linux-2.6/xfs_acl.c	2009-12-21 16:32:17.471990759 +0100
> @@ -251,8 +251,9 @@ xfs_set_mode(struct inode *inode, mode_t
>  	if (mode != inode->i_mode) {
>  		struct iattr iattr;
>  
> -		iattr.ia_valid = ATTR_MODE;
> +		iattr.ia_valid = ATTR_MODE | ATTR_CTIME;
>  		iattr.ia_mode = mode;
> +		iattr.ia_ctime = current_fs_time(inode->i_sb);
>  
>  		error = -xfs_setattr(XFS_I(inode), &iattr, XFS_ATTR_NOACL);
>  	}
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>