On Mon, 14 Nov 2011 13:19:29 +1100 NeilBrown <neilb@xxxxxxx> wrote:
>
> hi,
> I've run into another issue that seems to related to FS_REVAL_DOT.
>
> The script below makes the details precise, but the essence is that if I 'cd'
> into a directory on the client, then rename it on the server, then it is
> possible that the client will start getting ESTALE when accessing '.' - even
> though the directory still exists.
>
> The ESTALE is generated because nfs_lookup_revalidate fails on the dentry, so
> complete_walk (in fs/namei.c) gets failure from d_revalidate() and so sets the
> status to -ESTALE.
>
> nfs_lookup_revalidate fails because when it repeats the lookup it sees a
> different directory (as you will see the script creates a new directory with
> the old name).
>
> I think it only makes sense to do a ->lookup revalidate of the dentry at the
> end of the path when there was a real non '.' or '..' name leading to the
> dentry. If we were just looking up '.', we want to revalidate the inode, but
> not the dentry.
>
> Unfortunately I cannot see how that distinction could be introduced into the
> current path-walk code.
>
> Any ideas?
>
> Thanks,
> NeilBrown
>
>
> SERVER=eli # name of server. ssh access required.
> DIR=/home # directory on server to mount
> MPOINT=/mnt # location on client to mount it.
> TMP=/neilb/tmp # path to scratch area in $DIR
>
> sudo umount $MPOINT
> sudo mount -o vers=3 $SERVER:$DIR $MPOINT
>
> cd /
> ssh $SERVER "rm -r $DIR$TMP/*dir*"
> ssh $SERVER "mkdir $DIR$TMP/adir"
> while [ ! -d $MPOINT$TMP/adir ];
> do echo -n . ; sleep 2;
> done
> cd $MPOINT$TMP/adir || exit
> echo "Entered directory"
> ls -la > /dev/null
> ssh $SERVER "cd $DIR$TMP; mv adir adir.moved"
> echo "Moved directory on server"
> ls -la > /dev/null
> echo -n "Waiting for move to be visible on client"
> while ls -la $MPOINT$TMP/adir >/dev/null 2>&1
> do echo -n .
> sleep 3
> (cd / ; ssh $SERVER "cd $DIR$TMP; mkdir bdir ; rmdir bdir" )
> done
> echo
> echo "Make replacement directory on server"
> (cd / ; ssh $SERVER "cd $DIR$TMP; mkdir adir")
> ls -la $MPOINT$TMP/adir
> ls -la
>
.. but answer came there none....
I've looked some more at the code and now would like to propose a patch.
This fixes it for me and feels right.
Opinions?
Thanks,
NeilBrown
From 7abb2d77b4c8d8ca340e372447467d8a47241f83 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@xxxxxxx>
Date: Wed, 30 Nov 2011 18:35:13 +1100
Subject: [PATCH] nfs - handle d_revalidate of 'dot' correctly.
When d_revalidate is called on a dentry because FS_REVAL_DOT is set
it isn't really appropriate to revalidate the name.
If the path was simply ".", then the current-working-directory could
have been renamed on the server and should still be accessible as "."
even if it has a new name.
If the path was "/some/long/path/.", then the final component ("path" in
this case) has already been revalidated and there is no particular
need to do it again.
If we change nd->last_type to refer to "the last component looked at"
rather than just "the last component", then these cases can be
detected by "nd->last_type != LAST_NORM".
Signed-off-by: NeilBrown <neilb@xxxxxxx>
---
fs/namei.c | 2 +-
fs/nfs/dir.c | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 5008f01..6a720f7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1434,6 +1434,7 @@ static int link_path_walk(const char *name, struct nameidata *nd)
}
}
+ nd->last_type = type;
/* remove trailing slashes? */
if (!c)
goto last_component;
@@ -1458,7 +1459,6 @@ static int link_path_walk(const char *name, struct nameidata *nd)
last_component:
nd->last = this;
- nd->last_type = type;
return 0;
}
terminate_walk(nd);
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ac28990..f62827a 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1137,6 +1137,15 @@ static int nfs_lookup_revalidate(struct dentry *dentry, struct nameidata *nd)
if (NFS_STALE(inode))
goto out_bad;
+ if (nd->last_type != LAST_NORM) {
+ /* name not relevant, just inode */
+ error = nfs_revalidate_inode(NFS_SERVER(inode), inode);
+ if (error)
+ goto out_bad;
+ else
+ goto out_valid;
+ }
+
error = -ENOMEM;
fhandle = nfs_alloc_fhandle();
fattr = nfs_alloc_fattr();
--
1.7.7.3
Attachment:
signature.asc
Description: PGP signature
