2001-02-28 23:53:29

by NeilBrown

[permalink] [raw]
Subject: Re: Stale NFS handles on 2.4.2

On February 28, [email protected] wrote:
> >>>>> " " == Neil Brown <[email protected]> writes:
>
> > So... you can access things under /home/david, but you cannot
> > access /home/david itself? So, supposing that "fred" were some
> > file that you happen to know is in /home/david, then
>
> > ls /home/david fails with ESTALE and does not cause
> > any traffic to the server and
>
> This is normal. Once an inode gets flagged as being stale, then it
> remains stale. After all it would be a bug too if a filehandle were
> stale one moment, and then not the next.
>
I don't think that is necessarily a bug.
If I mis-configured the server to deny access to a particular client,
the client would start getting NFSERR_STALE responses. I notice the
problem and reconfigure the server and I would expect the ESTALE
errors to go away. But apparently they don't. Or atleast they don't
as long as the inode stays in the cache. Once it gets flushed from
the cache, a lookup will cause everything to work again.
But if an object below a "STALE" directory is being held open, the
inode will never get flushed and so the inode stays stale.

What is really odd about this situation is that whenever David tries
to access his home directory (/home/david) nfs_lookup_revalidate will
be called which will (if the cache isn't fresh enough) do a "lookup"
which returns the filehandle and attributes of /home/david. This
should be enough to convince the client that the filehandle isn't
stale anymore. However nfs_refresh_inode doesn't use this information
to clear the NFS_INO_STALE flag. Maybe it should.

In short, I really don't think that NFS_INO_STALE (or any other item
if information received from the server) should be considered to be
permanent and never rechecked.

NeilBrown


> The question here is therefore really why did the server tell us that
> the filehandle was stale in the first place.
>
> Cheers,
> Trond


2001-03-01 01:14:20

by Trond Myklebust

[permalink] [raw]
Subject: Re: Stale NFS handles on 2.4.2

>>>>> " " == Neil Brown <[email protected]> writes:

> In short, I really don't think that NFS_INO_STALE (or any other
> item if information received from the server) should be
> considered to be permanent and never rechecked.

There are 2 problems of inode corruption if you allow inodes to die
and then reappear at will:

1) is that several servers, including the userspace nfsd, have
problems with filehandle reuse. You do not want to fall victim
either to corruption either of the inode cache or (worse) the
file itself.

In the same vein, consider all these horror stories about people
sharing CDROMs over NFS, mounting/umounting them at will...

2) Even on servers that strictly respect the uniqueness of
filehandles you can risk cache/file corruption due to inode
aliasing when for instance you are using shared mmap between 2
processes.

For instance: under a lookup() operation, the client notices that
the inode is stale, creates a new one (after unhashing the old
one of course but otherwise not invalidating it).
If the old inode is then magically declared to be OK, you
suddenly have 2 inodes with 2 different caches that are
supposed to represent the same file.

Of course, the second problem is not really relevant to the root
directory inode (which should never get aliased anyway). Perhaps the
correct thing to do would be to allow root inodes to clear the
NFS_INO_STALE flag? Something like the following...

Cheers,
Trond

--- linux-2.4.2/fs/nfs/inode.c.orig Wed Feb 14 01:14:28 2001
+++ linux-2.4.2/fs/nfs/inode.c Thu Mar 1 02:08:59 2001
@@ -819,24 +819,22 @@
int
__nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
{
- int status = 0;
+ int status = -ESTALE;
struct nfs_fattr fattr;

dfprintk(PAGECACHE, "NFS: revalidating (%x/%Ld)\n",
inode->i_dev, (long long)NFS_FILEID(inode));

lock_kernel();
- if (!inode || is_bad_inode(inode) || NFS_STALE(inode)) {
- unlock_kernel();
- return -ESTALE;
- }
+ if (!inode || is_bad_inode(inode))
+ goto out_nowait;
+ if (NFS_STALE(inode) && inode != inode->i_sb->s_root->d_inode)
+ goto out_nowait;

while (NFS_REVALIDATING(inode)) {
status = nfs_wait_on_inode(inode, NFS_INO_REVALIDATING);
- if (status < 0) {
- unlock_kernel();
- return status;
- }
+ if (status < 0)
+ goto out_nowait;
if (time_before(jiffies,NFS_READTIME(inode)+NFS_ATTRTIMEO(inode))) {
status = NFS_STALE(inode) ? -ESTALE : 0;
goto out_nowait;
@@ -850,7 +848,8 @@
inode->i_dev, (long long)NFS_FILEID(inode), status);
if (status == -ESTALE) {
NFS_FLAGS(inode) |= NFS_INO_STALE;
- remove_inode_hash(inode);
+ if (inode != inode->i_sb->s_root->d_inode)
+ remove_inode_hash(inode);
}
goto out;
}
@@ -863,6 +862,8 @@
}
dfprintk(PAGECACHE, "NFS: (%x/%Ld) revalidation complete\n",
inode->i_dev, (long long)NFS_FILEID(inode));
+
+ NFS_FLAGS(inode) &= ~NFS_INO_STALE;
out:
NFS_FLAGS(inode) &= ~NFS_INO_REVALIDATING;
wake_up(&inode->i_wait);