Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug
Bug#: 132673
Alias:
Product:
Component:
Status: RESOLVED
Resolution: FIXED
Assigned To: Daniel Drake <dsd@gentoo.org>
Hardware:
OS:
Version:
Priority:
Severity:
Reporter: Ronald Huizer <rhuizer@math.leidenuniv.nl>
Add CC:
CC:
Remove selected CCs
URL:
Summary:
Status Whiteboard:
Keywords:

Filename Description Type Creator Created Size Actions
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 132673 depends on: Show dependency tree
Bug 132673 blocks:
Votes: 0    Show votes for this bug    Vote for this bug

Additional Comments: (this is where you put emerge --info)


Not eligible to see or edit group visibility for this bug.






View Bug Activity   |   Format For Printing   |   XML   |   Clone This Bug


Description:   Opened: 2006-05-08 05:58 0000
Hi,

'cp -p' in coreutils-5.94-r3 (and most likely others, but this needs
verification) doesn't set the value of mtime correctly on files copied to NFS
mounts.
This problem is likely caused by NFS itself, which seems to update modification
times on files after a chmod operation. I believe POSIX defines that mtime
should be updated on every change of file content, but I'm not sure on this, so
correct me if I'm wrong. Although ultimately NFS might be to blame, the problem
seems to be introduced by the recent behaviour in coreutils copy.c, which is
outlined below.

The behaviour of 'cp' itself is apparent from the following strace output,
where it changes file modes after updating the modification time.

utimes("/proc/self/fd/4", {1147089451, 552945}) = 0
fchmod(4, 0100600)                      = 0

Of course this conflicts with the fact that NFS updates mtimes for chmod
operations.

An easy patch which should be applied to 'src/copy.c' has been provided for an
ebuild compile on coreutils-5.94-r3 at:
http://www.math.leidenuniv.nl/~rhuizer/coreutils-5.94-r3-copy.c.patch

With regards,
  -- Ronald Huizer

------- Comment #1 From SpanKY 2006-05-08 19:55:04 0000 -------
you have to update both copy_reg() and copy_internal() ...

ive e-mailed a patch upstream against latest cvs and i'll see what they have to
say on the topic

------- Comment #2 From SpanKY 2006-05-09 05:03:11 0000 -------
kernel bug which has been fixed already
http://lists.gnu.org/archive/html/bug-coreutils/2006-05/msg00060.html

------- Comment #3 From Ronald Huizer 2006-05-09 07:41:17 0000 -------
Hi again,

I have read the relevant discussion on the link you posted, but am quite
convinced this doesn't solve the problems; although admittedly I haven't tested
the relevant kernel patch, I have my thoughts on it.

This patch seems to resolve issues when caching of NFS writes are involved,
where a write occuring before a utimes call would be cached first, and written
to disk only after the utimes call.

--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -859,11 +859,9 @@ nfs_setattr(struct dentry *dentry, struc

        lock_kernel();
        nfs_begin_data_update(inode);
-       /* Write all dirty data if we're changing file permissions or size */
-       if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID|ATTR_SIZE)) != 0) {
-               filemap_write_and_wait(inode->i_mapping);
-               nfs_wb_all(inode);
-       }
+       /* Write all dirty data */
+       filemap_write_and_wait(inode->i_mapping);
+       nfs_wb_all(inode);
        /*
         * Return any delegations if we're going to change ACLs
         */

Reading this patch or the kernel source code makes clear NFS mode changes were
always flushed out at this point anyhow, so the change shouldn't matter in this
case. What matters is that the fchmod() occurs at a later time in the 'cp'
trace than the utimes() call.

The test case you outlined in your posting to the list was not a bit confusing,
so I have outlined a simple way te verify the problem below:

rhuizer@hoornik ~ $ uname -a # Just for the record
Linux hoornik 2.6.15-gentoo-r1 #1 PREEMPT Wed Mar 8 11:09:24 CET 2006 i686 AMD
Athlon(TM) XP 2000+ GNU/Linux
rhuizer@hoornik ~ $ stat breaknfs.c
  File: `breaknfs.c'
  Size: 803             Blocks: 8          IO Block: 32768  regular file
Device: 12h/18d Inode: 58327432    Links: 1
Access: (0644/-rw-r--r--)  Uid: (  669/ rhuizer)   Gid: (  501/  admins)
Access: 2006-05-06 18:09:56.000000000 +0200
Modify: 2005-09-13 11:34:59.000000000 +0200
Change: 2005-09-13 11:34:59.000000000 +0200
rhuizer@hoornik ~ $ cp -p breaknfs.c new.c
rhuizer@hoornik ~ $ stat new.c
  File: `new.c'
  Size: 803             Blocks: 8          IO Block: 32768  regular file
Device: 12h/18d Inode: 39370836    Links: 1
Access: (0644/-rw-r--r--)  Uid: (  669/ rhuizer)   Gid: (  501/  admins)
Access: 2006-05-06 18:09:56.000000000 +0200
Modify: 2006-05-09 16:33:18.233727766 +0200
Change: 2006-05-09 16:33:18.237727944 +0200

From this it should be evident that the resolution order of 'cp -p' conflicts
with NFS updating timestamps on fchmod().

The remark that cp should be able to provide support for ownership transfers
might be appropriate, but does that argument hold true for only the modes of a
file? It might be possible to place the utimes() call after the fchmod() and
before the chown().

With regards,
  -- Ronald Huizer

------- Comment #4 From SpanKY 2006-05-09 17:01:46 0000 -------
not a bug in coreutils

------- Comment #5 From Daniel Drake 2006-05-10 08:02:19 0000 -------
Please confirm that this bug exists in the latest development kernel, currently
2.6.17-rc3

------- Comment #6 From SpanKY 2006-05-10 20:18:01 0000 -------
it exists in 2.6.16.9 for sure

------- Comment #7 From Ronald Huizer 2006-05-23 02:41:54 0000 -------
Trying this on 2.6.17-rc4 seems a whole lot better; at least the modification
and access times are now shown correctly (disregarding microsecond truncation).
Seems to be resolved for me.

With regards,
  -- Ronald Huizer

rhuizer@hoornik ~ $ uname -a
Linux hoornik 2.6.17-rc4 #1 PREEMPT Tue May 23 09:43:42 CEST 2006 i686 AMD
Athlon(TM) XP 2000+ GNU/Linux
rhuizer@hoornik ~ $ stat breaknfs.c
  File: `breaknfs.c'
  Size: 803             Blocks: 8          IO Block: 32768  regular file
Device: 12h/18d Inode: 58327432    Links: 1
Access: (0644/-rw-r--r--)  Uid: (  669/ rhuizer)   Gid: (  501/  admins)
Access: 2006-05-23 10:50:12.779180516 +0200
Modify: 2005-09-13 11:34:59.000000000 +0200
Change: 2005-09-13 11:34:59.000000000 +0200
rhuizer@hoornik ~ $ cp -p breaknfs.c foo.c
rhuizer@hoornik ~ $ stat foo.c
  File: `foo.c'
  Size: 803             Blocks: 8          IO Block: 32768  regular file
Device: 12h/18d Inode: 58327333    Links: 1
Access: (0644/-rw-r--r--)  Uid: (  669/ rhuizer)   Gid: (  501/  admins)
Access: 2006-05-23 10:50:12.779180000 +0200
Modify: 2005-09-13 11:34:59.000000000 +0200
Change: 2006-05-23 10:52:47.612671133 +0200

------- Comment #8 From Daniel Drake 2006-05-29 05:38:35 0000 -------
I'd like to backport the fix, but I can't see which patch in 2.6.17 solves
this. The only real suspect is this one:

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff_plain;h=755c1e20cd2ad56e5c567fa05769eb98a3eef72b;hp=7bab377fcb495ee2e5a1cd69d235f8d84c76e3af

Would you mind patching 2.6.16 with that and seeing if that fixes it, just in
case your understanding is incorrect?

------- Comment #9 From Ronald Huizer 2006-05-29 07:03:08 0000 -------
> I'd like to backport the fix, but I can't see which patch in 2.6.17 solves
> this. The only real suspect is this one:

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff_plain;h=755c1e20cd2ad56e5c567fa05769eb98a3eef72b;hp=7bab377fcb495ee2e5a1cd69d235f8d84c76e3af

You are right, thanks for asking this. I didn't misunderstand the patch, I
misunderstood the nature of the bug, afaics now NFS never explicitly updated
mtimes on fchown(), but didn't sync out the blocks after setting [am]time
attributes immediatly.

I was puzzled at first why this would trigger with coreutils before patching it
and not after, but it seems similar to the following:

In the first situation of "utimes() / fchown() / close()" we have fchown()
synching out disk blocks after the utimes attribute has been set, thus killing
mtime, and in the second situation of "fchown() / utimes() / close()" we have
fchown() flushing out all the pending data, updating mtime, then utimes()
setting it explicitly, then close() not flushing pending blocks since there is
nothing to flush.

Anyway, this should (hopefully) settle and clarify things and allow you to
backport the patch easily. Sorry for the additional trouble.

With kind regards,
 -- Ronald Huizer

------- Comment #10 From Daniel Drake 2006-06-02 11:17:00 0000 -------
Fixed in gentoo-sources-2.6.16-gentoo-r9 / genpatches-2.6.16-11

Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug