linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Neil Brown <neilb@suse.de>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>,
	Chuck Lever <chuck.lever@oracle.com>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] VFS: fix recent breakage of FS_REVAL_DOT
Date: Mon, 24 May 2010 16:57:56 +1000	[thread overview]
Message-ID: <20100524165756.2cfa54c4@notabene.brown> (raw)


Commit 1f36f774b22a0ceb7dd33eca626746c81a97b6a5 broke FS_REVAL_DOT semantics.

In particular, before this patch, the command
   ls -l
in an NFS mounted directory would always check if the directory on the server
had changed and if so would flush and refill the pagecache for the dir.
After this patch, the same "ls -l" will repeatedly return stale date until
the cached attributes for the directory time out.

The following patch fixes this by ensuring the d_revalidate is called by
do_last when "." is being looked-up.
link_path_walk has already called d_revalidate, but in that case LOOKUP_OPEN
is not set so nfs_lookup_verify_inode chooses not to do any validation.

The following patch restores the original behaviour.

Cc: stable@kernel.org
Signed-off-by: NeilBrown <neilb@suse.de>

---
As an aside the whole "FS_REVAL_DOT" concept seems deeply flawed to me.
The apparent purpose for d_revalidate is to ensure that the given name in the
given directory still points to the given inode.  For '.' this is pointless
as '.' in a given directory has a very well defined and unchangeable value.

It seems that d_revalidate is also used to update the cached attributes for
the target as well.  While that is reasonable side effect it seems wrong to
expect it as is the basis for FS_REVAL_DOT.

Surely ->open should perform any final validation of cached attributes.

NeilBrown

(Yes, I have posted this before but I didn't seem to be getting any real
progress, so I split this more obviously correct patch out from the other
less obvious one relating to mountpoints.  Once I succeed with this I'll try
again with the other).

diff --git a/fs/namei.c b/fs/namei.c
index 48e1f60..868d0cb 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1621,6 +1621,7 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	case LAST_DOTDOT:
 		follow_dotdot(nd);
 		dir = nd->path.dentry;
+	case LAST_DOT:
 		if (nd->path.mnt->mnt_sb->s_type->fs_flags & FS_REVAL_DOT) {
 			if (!dir->d_op->d_revalidate(dir, nd)) {
 				error = -ESTALE;
@@ -1628,7 +1629,6 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 			}
 		}
 		/* fallthrough */
-	case LAST_DOT:
 	case LAST_ROOT:
 		if (open_flag & O_CREAT)
 			goto exit;

             reply	other threads:[~2010-05-24  6:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-24  6:57 Neil Brown [this message]
2010-05-24 11:59 ` [PATCH] VFS: fix recent breakage of FS_REVAL_DOT Al Viro
2010-05-24 15:50   ` Al Viro
2010-05-24 16:21     ` Trond Myklebust
2010-05-24 16:47       ` Al Viro
2010-05-24 17:06         ` Trond Myklebust
2010-05-24 19:08           ` Al Viro
2010-05-24 21:13             ` Trond Myklebust
2010-05-24 23:01               ` Al Viro
2010-05-24 23:44                 ` Al Viro
2010-05-25 13:04                   ` Trond Myklebust
2010-05-25 12:57                 ` Trond Myklebust
2010-05-25  1:35         ` Neil Brown
2010-05-25  1:14   ` Neil Brown
2010-05-25  1:58     ` Al Viro
2010-05-26  2:52       ` Neil Brown

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=20100524165756.2cfa54c4@notabene.brown \
    --to=neilb@suse.de \
    --cc=chuck.lever@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@fys.uio.no \
    --cc=viro@ZenIV.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).