This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: MVFS results


Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:

> Just to be sure.  This does not occur with cp -p on other filesystems
> than MVFS, right?  I don't see that problem on NTFS or FAT.

I can confirm that with both NTFS and FAT, the NtSetInformationFile in 
utimens_fs is immediate (no fd has to be closed).  There may be other buggy fs 
out there, but we'll deal with them as reports are issued.

> > Or do we teach fchmod to honor pending 
> > timestamp changes?
> 
> The problem is that the calls are stateless.  The fchmod call doesn't
> know about a former utimens call and vice versa.  Worse, to do that
> really correct you would not only have to keep track of the timestamp as
> set by utimens, you would also have to keep track of them in case of
> write.  That's a can of worms if you ask me.

Agreed.  Forcing the utimes to disk (as already happens for decent fs) seems 
like the best approach for a fix.

> Maybe(!) a FlushFileBuffer call will also flush metadata changes:

Nice thought, but no dice: the time never changed.

> Other than that, I can only suggest a change like this one:
> 
> Index: fhandler_disk_file.cc
> ===================================================================
> -  NTSTATUS status = NtSetInformationFile (get_handle (), &io, &fbi, sizeof 
fbi,
> -					  FileBasicInformation);
> +  if (pc.fs_is_mvfs () && !closeit)
> +    {

As written, that one didn't quite do it either:

$ cp -p bar bar1
cp: preserving times for `bar1': Bad file descriptor

But I see why - you are using fh uninitialized.

> +      HANDLE fh;
> +
> +      InitializeObjectAttributes (&attr, &ro_u_empty, pc.objcaseinsensitive 
(),
> +				  fh, NULL);

Yet the obvious fix of using get_handle() instead of fh doesn't help either.  
Before this patch, writable files were able to preserve times (because there 
was no fchmod in the loop), but after the patch, both writable and read-only 
are losing the timestamp update.  The duplicated handle does just fine, but 
then closing the original handle undoes the change:

Breakpoint 1, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34)
    at ../../../../winsup/cygwin/fhandler_disk_file.cc:1315
1315      if (pc.fs_is_mvfs () && !closeit)
Current language:  auto; currently c++
(gdb) shell ls -l bar*  # just created, prior to fchmod
-r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar
-rw-r--r-- 1 eblake Domain Users 3 Jul 20 10:31 bar1
(gdb) n
1320          InitializeObjectAttributes (&attr, &ro_u_empty, 
pc.objcaseinsensitive (),
(gdb) 
1323                               FILE_SHARE_VALID_FLAGS, 
FILE_OPEN_FOR_BACKUP_INTENT);
(gdb) 

Breakpoint 2, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34)
    at ../../../../winsup/cygwin/fhandler_disk_file.cc:1324
1324          if (NT_SUCCESS (status))
(gdb) p status
$1 = 0
(gdb) n
1327                                             FileBasicInformation);
(gdb) 

Breakpoint 3, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34)
    at ../../../../winsup/cygwin/fhandler_disk_file.cc:1328
1328              NtClose (fh);
(gdb) shell ls -l bar*  # changed, but fd not closed, so don't see it yet
-r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar
-rw-r--r-- 1 eblake Domain Users 3 Jul 20 10:31 bar1
(gdb) p status
$2 = 0
(gdb) n

Breakpoint 4, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34)
    at ../../../../winsup/cygwin/fhandler_disk_file.cc:1334
1334      if (closeit)
(gdb) p status
$3 = 0
(gdb) shell ls -l bar*  # all right - the time made it through
-r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar
-rw-r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar1
(gdb) b 848
Breakpoint 5 at 0x6103e89c: 
file ../../../../winsup/cygwin/fhandler_disk_file.cc, line 848.
(gdb) b 850
Breakpoint 6 at 0x6103e8db: 
file ../../../../winsup/cygwin/fhandler_disk_file.cc, line 850.
(gdb) c
Continuing.

Breakpoint 5, fhandler_disk_file::fchmod (this=0x6128f004, mode=292)
    at ../../../../winsup/cygwin/fhandler_disk_file.cc:848
848       status = NtSetAttributesFile (get_handle (), pc.file_attributes ());
(gdb) shell ls -l bar*  # now ready to do the fchmod
-r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar
-rw-r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar1
(gdb) n

Breakpoint 6, fhandler_disk_file::fchmod (this=0x6128f004, mode=292)
    at ../../../../winsup/cygwin/fhandler_disk_file.cc:850
850       if (!pc.has_acls ())
(gdb) shell ls -l bar*  # good: the w bit is lost, but timestamps don't change
-r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar
-r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar1
(gdb) c
Continuing.

Program exited normally.
(gdb) shell ls -l bar*  # oh dear - closing the original fd changed time
-r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar
-r--r--r-- 1 eblake Domain Users 3 Jul 20 10:37 bar1


So, setting the timestamp via closing an alternative handle works, but then 
we're back to the problem that closing the original handle corrupts the 
timestamp (and worse, it set it to 10:37 of the handle close, instead of the 
10:31 of the file create; whereas before the patch it least left the create 
time intact).

I noticed that http://msdn.microsoft.com/en-us/library/dd852055.aspx states 
that "However, a driver or application can request that the file system not 
update one or more of these members for I/O operations that are performed on 
the caller's file handle by setting the appropriate members to â1."  Maybe it 
would work to change the appropriate fields of the original handle to -1 at the 
same time we close the duplicate handle?  Or will that inhibit correct 
timestamp modifications from subsequent reads on the open fd?

Another thought - since we are able to see the timestamps updated as soon as we 
close the duplicated handle, and the fchmod didn't corrupt state (only the 
subsequent close), maybe we just need to special case close_fs for MVFS to also 
play games with handles, so that the last handle closed always has the desired 
times?

Or maybe this is a case where we need a FlushFileBuffer call during the 
utimens_fs after all, in addition to closing the duplicate descriptor, so that 
the original handle no longer has any state that needs flushing which might 
inadvertently change timestamps.

I guess my next step will be looking at procmon output, to see if I can make 
sense of why timestamps are changing as MVFS forwards operations onto the 
backing store fs.

-- 
Eric Blake



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]