public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Jesper Krogh <jesper-Q2TZfHgGEy4@public.gmane.org>
Cc: linux-nfs@vger.kernel.org
Subject: Re: Client cache updates missing? (2.6.31.5)
Date: Mon, 30 Nov 2009 13:40:35 -0500	[thread overview]
Message-ID: <20091130184035.GD6348@fieldses.org> (raw)
In-Reply-To: <4B140F5F.6050107-Q2TZfHgGEy4@public.gmane.org>

On Mon, Nov 30, 2009 at 07:30:55PM +0100, Jesper Krogh wrote:
> J. Bruce Fields wrote:
> >> Wether or not it has anything to do. The file has been written to the
> >> NFS-server from another NFS-client. The server is running 2.6.31.5 and
> >> the client that above was run on is 2.6.24-24 (Ubuntu Jaunty), the
> >> client that wrote the file was running 2.6.29.1.
> > 
> > I this v3 or v4?  What's the exported filesystem?  (ext3?)
> 
> v3 and ext3
> 
> > It's probably a timestamp resolution problem; if the directory was
> > modified twice in the same second, the later change won't change the
> > timestamp, and so the client may assume its cache is still good.
> 
> That's not nice..  but given the situation is may quite well be the
> problem.
> 
> > Recent clients try a little harder to work around this. 
> 
> How recent and how much harder?

There's the following.  Looks like it was first included in 2.6.30.  I
thought I remembered one or two other related changes, but perhaps the
others didn't make it in.

--b.

commit 37d9d76d8b3a2ac5817e1fa3263cfe0fdb439e51
Author: NeilBrown <neilb@suse.de>
Date:   Wed Mar 11 14:10:23 2009 -0400

    NFS: flush cached directory information slightly more readily.
    
    If cached directory contents becomes incorrect, there is no way to
    flush the contents.  This contrasts with files where file locking is
    the recommended way to ensure cache consistency between multiple
    applications (a read-lock always flushes the cache).
    
    Also while changes to files often change the size of the file (thus
    triggering a cache flush), changes to directories often do not change
    the apparent size (as the size is often rounded to a block size).
    
    So it is particularly important with directories to avoid the
    possibility of an incorrect cache wherever possible.
    
    When the link count on a directory changes it implies a change in the
    number of child directories, and so a change in the contents of this
    directory.  So use that as a trigger to flush cached contents.
    
    When the ctime changes but the mtime does not, there are two possible
    reasons.
     1/ The owner/mode information has been changed.
     2/ utimes has been used to set the mtime backwards.
    
    In the first case, a data-cache flush is not required.
    In the second case it is.
    
    So on the basis that correctness trumps performance, flush the
    directory contents cache in this case also.
    
    Signed-off-by: NeilBrown <neilb@suse.de>
    Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index acaaa7c..268ce3a 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1113,8 +1113,16 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 				nfs_force_lookup_revalidate(inode);
 		}
 		/* If ctime has changed we should definitely clear access+acl caches */
-		if (!timespec_equal(&inode->i_ctime, &fattr->ctime))
+		if (!timespec_equal(&inode->i_ctime, &fattr->ctime)) {
 			invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
+			/* and probably clear data for a directory too as utimes can cause
+			 * havoc with our cache.
+			 */
+			if (S_ISDIR(inode->i_mode)) {
+				invalid |= NFS_INO_INVALID_DATA;
+				nfs_force_lookup_revalidate(inode);
+			}
+		}
 	} else if (nfsi->change_attr != fattr->change_attr) {
 		dprintk("NFS: change_attr change on server for file %s/%ld\n",
 				inode->i_sb->s_id, inode->i_ino);
@@ -1148,8 +1156,11 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	    inode->i_gid != fattr->gid)
 		invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
 
-	if (inode->i_nlink != fattr->nlink)
+	if (inode->i_nlink != fattr->nlink) {
 		invalid |= NFS_INO_INVALID_ATTR;
+		if (S_ISDIR(inode->i_mode))
+			invalid |= NFS_INO_INVALID_DATA;
+	}
 
 	inode->i_mode = fattr->mode;
 	inode->i_nlink = fattr->nlink;

  parent reply	other threads:[~2009-11-30 18:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-29  8:24 Client cache updates missing? (2.6.31.5) Jesper Krogh
     [not found] ` <4B122FB2.7040505-Q2TZfHgGEy4@public.gmane.org>
2009-11-30 18:26   ` J. Bruce Fields
2009-11-30 18:30     ` Jesper Krogh
     [not found]       ` <4B140F5F.6050107-Q2TZfHgGEy4@public.gmane.org>
2009-11-30 18:40         ` J. Bruce Fields [this message]
2009-11-30 18:53           ` Trond Myklebust

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091130184035.GD6348@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=jesper-Q2TZfHgGEy4@public.gmane.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox