On Fri, 2005-04-08 at 16:54, Linda Dunaphant wrote:

>Do you think it would be better for nfs_refresh_inode() to check the mtime,
>perform the mtime update if needed, and not set the NFS_INO_INVALID_ATTR
>flag if the data_unstable flag is set? This is how nfs_update_inode()
>handles its mtime check.

Hi again Trond,

I updated my first patch to nfs_refresh_inode() to be similar to the way
nfs_update_inode() does the check and update of the mtime. nfs_refresh_inode()
now checks to see if the mtime changed, and if so, it does the update of
the mtime. It only sets NFS_INO_INVALID_ATTR if data_unstable is not set.

I temporarily added some printk's to selected functions to monitor some of
the functions called after the data write to the server occurs. With this
latest patch, the sequence that I see with the test program is now the
same as it was originally without any patch - except the mtime is has been
updated:
        nfs3_xdr_writeargs
        xdr_decode_fattr                <--- new mtime from server
        nfs_refresh_inode               <--- updates mtime in inode
        nfs_attribute_timeout
        nfs_attribute_timeout
        xdr_decode_fattr  
        nfs_refresh_inode

With the first patch I proposed this sequence was:
        nfs3_xdr_writeargs
        xdr_decode_fattr                <--- new mtime from server
        nfs_refresh_inode               <--- NFS_INO_INVALID_ATTR set
        xdr_decode_fattr  
        nfs_update_inode                <--- updates mtime in inode
        nfs_attribute_timeout
        xdr_decode_fattr  

Thank you for pointing out that there may be other consequences if the
NFS_INO_INVALID_ATTR is always set by nfs_refresh_inode() when the mtime
values are different. I believe this second patch fixes the original
problem without affecting any other behaviour.

Cheers,
Linda

diff -ura base/fs/nfs/inode.c new/fs/nfs/inode.c
--- base/fs/nfs/inode.c 2005-04-07 16:04:40.000000000 -0400
+++ new/fs/nfs/inode.c  2005-04-08 19:23:44.151698674 -0400
@@ -1176,9 +1176,17 @@
        }
 
        /* Verify a few of the more important attributes */
+       if (!timespec_equal(&inode->i_mtime, &fattr->mtime)) {
+               memcpy(&inode->i_mtime, &fattr->mtime, sizeof(inode->i_mtime));
+#ifdef NFS_DEBUG_VERBOSE
+               printk(KERN_DEBUG "NFS: mtime change on %s/%ld\n", 
inode->i_sb->s_id, inode->i_ino);
+#endif
+               if (!data_unstable)
+                       nfsi->flags |= NFS_INO_INVALID_ATTR;
+       }
+
        if (!data_unstable) {
-               if (!timespec_equal(&inode->i_mtime, &fattr->mtime)
-                               || cur_size != new_isize)
+               if (cur_size != new_isize)
                        nfsi->flags |= NFS_INO_INVALID_ATTR;
        } else if (S_ISREG(inode->i_mode) && new_isize > cur_size)
                        nfsi->flags |= NFS_INO_INVALID_ATTR;


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to