Commit 0d86f2ec authored by Trond Myklebust's avatar Trond Myklebust

[PATCH] NFS lookup code rewrite w/o open(".") fix...

  This is a resend of the NFS lookup code rewrite, but with the open(".")
VFS fix removed. (I'll resend the 'uses d_revalidate()' version
separately after a suitable delay to allow for comments.)

  Issues fixed by this patch:

 - Use the directory mtime in order to give us a hint when we should
   check for namespace changes.

 - Add support for the 'nocto' flag, in order to turn off the strict
   attribute cache revalidation on file open().

 - Simplify inode lookup. Don't check the 'fsid' field (which appears
   to be buggy in too many servers in order to be reliable). Instead
   we only rely on the inode number (a.k.a. 'fileid') and the
   (supposedly unique) filehandle.
parent a5c9b326
...@@ -401,6 +401,21 @@ static int nfs_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -401,6 +401,21 @@ static int nfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
return 0; return 0;
} }
/*
* A check for whether or not the parent directory has changed.
* In the case it has, we assume that the dentries are untrustworthy
* and may need to be looked up again.
*/
static inline
int nfs_check_verifier(struct inode *dir, struct dentry *dentry)
{
if (IS_ROOT(dentry))
return 1;
if (nfs_revalidate_inode(NFS_SERVER(dir), dir))
return 0;
return time_after(dentry->d_time, NFS_MTIME_UPDATE(dir));
}
/* /*
* Whenever an NFS operation succeeds, we know that the dentry * Whenever an NFS operation succeeds, we know that the dentry
* is valid, so we update the revalidation timestamp. * is valid, so we update the revalidation timestamp.
...@@ -410,48 +425,31 @@ static inline void nfs_renew_times(struct dentry * dentry) ...@@ -410,48 +425,31 @@ static inline void nfs_renew_times(struct dentry * dentry)
dentry->d_time = jiffies; dentry->d_time = jiffies;
} }
static inline int nfs_dentry_force_reval(struct dentry *dentry, int flags) static inline
int nfs_lookup_verify_inode(struct inode *inode, int flags)
{ {
struct inode *inode = dentry->d_inode; struct nfs_server *server = NFS_SERVER(inode);
unsigned long timeout = NFS_ATTRTIMEO(inode);
/* /*
* If it's the last lookup in a series, we use a stricter * If we're interested in close-to-open cache consistency,
* cache consistency check by looking at the parent mtime. * then we revalidate the inode upon lookup.
*
* If it's been modified in the last hour, be really strict.
* (This still means that we can avoid doing unnecessary
* work on directories like /usr/share/bin etc which basically
* never change).
*/ */
if (!(flags & LOOKUP_CONTINUE)) { if (!(server->flags & NFS_MOUNT_NOCTO) && !(flags & LOOKUP_CONTINUE))
long diff = CURRENT_TIME - dentry->d_parent->d_inode->i_mtime; NFS_CACHEINV(inode);
return nfs_revalidate_inode(server, inode);
if (diff < 15*60)
timeout = 0;
}
return time_after(jiffies,dentry->d_time + timeout);
} }
/* /*
* We judge how long we want to trust negative * We judge how long we want to trust negative
* dentries by looking at the parent inode mtime. * dentries by looking at the parent inode mtime.
* *
* If mtime is close to present time, we revalidate * If parent mtime has changed, we revalidate, else we wait for a
* more often. * period corresponding to the parent's attribute cache timeout value.
*/ */
#define NFS_REVALIDATE_NEGATIVE (1 * HZ) static inline int nfs_neg_need_reval(struct inode *dir, struct dentry *dentry)
static inline int nfs_neg_need_reval(struct dentry *dentry)
{ {
struct inode *dir = dentry->d_parent->d_inode; if (!nfs_check_verifier(dir, dentry))
unsigned long timeout = NFS_ATTRTIMEO(dir); return 1;
long diff = CURRENT_TIME - dir->i_mtime; return time_after(jiffies, dentry->d_time + NFS_ATTRTIMEO(dir));
if (diff < 5*60 && timeout > NFS_REVALIDATE_NEGATIVE)
timeout = NFS_REVALIDATE_NEGATIVE;
return time_after(jiffies, dentry->d_time + timeout);
} }
/* /*
...@@ -462,9 +460,8 @@ static inline int nfs_neg_need_reval(struct dentry *dentry) ...@@ -462,9 +460,8 @@ static inline int nfs_neg_need_reval(struct dentry *dentry)
* NOTE! The hit can be a negative hit too, don't assume * NOTE! The hit can be a negative hit too, don't assume
* we have an inode! * we have an inode!
* *
* If the dentry is older than the revalidation interval, * If the parent directory is seen to have changed, we throw out the
* we do a new lookup and verify that the dentry is still * cached dentry and do a new lookup.
* correct.
*/ */
static int nfs_lookup_revalidate(struct dentry * dentry, int flags) static int nfs_lookup_revalidate(struct dentry * dentry, int flags)
{ {
...@@ -477,13 +474,9 @@ static int nfs_lookup_revalidate(struct dentry * dentry, int flags) ...@@ -477,13 +474,9 @@ static int nfs_lookup_revalidate(struct dentry * dentry, int flags)
lock_kernel(); lock_kernel();
dir = dentry->d_parent->d_inode; dir = dentry->d_parent->d_inode;
inode = dentry->d_inode; inode = dentry->d_inode;
/*
* If we don't have an inode, let's look at the parent
* directory mtime to get a hint about how often we
* should validate things..
*/
if (!inode) { if (!inode) {
if (nfs_neg_need_reval(dentry)) if (nfs_neg_need_reval(dir, dentry))
goto out_bad; goto out_bad;
goto out_valid; goto out_valid;
} }
...@@ -494,48 +487,39 @@ static int nfs_lookup_revalidate(struct dentry * dentry, int flags) ...@@ -494,48 +487,39 @@ static int nfs_lookup_revalidate(struct dentry * dentry, int flags)
goto out_bad; goto out_bad;
} }
if (!nfs_dentry_force_reval(dentry, flags)) /* Force a full look up iff the parent directory has changed */
if (nfs_check_verifier(dir, dentry)) {
if (nfs_lookup_verify_inode(inode, flags))
goto out_bad;
goto out_valid; goto out_valid;
if (IS_ROOT(dentry)) {
__nfs_revalidate_inode(NFS_SERVER(inode), inode);
goto out_valid_renew;
} }
/* if (NFS_STALE(inode))
* Do a new lookup and check the dentry attributes. goto out_bad;
*/
error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr); error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr);
if (error) if (error)
goto out_bad; goto out_bad;
if (memcmp(NFS_FH(inode), &fhandle, sizeof(struct nfs_fh))!= 0)
/* Inode number matches? */
if (!(fattr.valid & NFS_ATTR_FATTR) ||
NFS_FSID(inode) != fattr.fsid ||
NFS_FILEID(inode) != fattr.fileid)
goto out_bad; goto out_bad;
if ((error = nfs_refresh_inode(inode, &fattr)) != 0)
/* Ok, remember that we successfully checked it.. */
nfs_refresh_inode(inode, &fattr);
if (nfs_inode_is_stale(inode, &fhandle, &fattr))
goto out_bad; goto out_bad;
out_valid_renew:
nfs_renew_times(dentry); nfs_renew_times(dentry);
out_valid: out_valid:
unlock_kernel(); unlock_kernel();
return 1; return 1;
out_bad: out_bad:
shrink_dcache_parent(dentry); NFS_CACHEINV(dir);
if (inode && S_ISDIR(inode->i_mode)) {
/* Purge readdir caches. */
nfs_zap_caches(inode);
/* If we have submounts, don't unhash ! */ /* If we have submounts, don't unhash ! */
if (have_submounts(dentry)) if (have_submounts(dentry))
goto out_valid; goto out_valid;
shrink_dcache_parent(dentry);
}
d_drop(dentry); d_drop(dentry);
/* Purge readdir caches. */
nfs_zap_caches(dir);
if (inode && S_ISDIR(inode->i_mode))
nfs_zap_caches(inode);
unlock_kernel(); unlock_kernel();
return 0; return 0;
} }
...@@ -565,6 +549,7 @@ static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode) ...@@ -565,6 +549,7 @@ static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
{ {
if (dentry->d_flags & DCACHE_NFSFS_RENAMED) { if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
lock_kernel(); lock_kernel();
inode->i_nlink--;
nfs_complete_unlink(dentry); nfs_complete_unlink(dentry);
unlock_kernel(); unlock_kernel();
} }
...@@ -604,9 +589,9 @@ static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry) ...@@ -604,9 +589,9 @@ static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry)
if (inode) { if (inode) {
no_entry: no_entry:
d_add(dentry, inode); d_add(dentry, inode);
nfs_renew_times(dentry);
error = 0; error = 0;
} }
nfs_renew_times(dentry);
} }
out: out:
return ERR_PTR(error); return ERR_PTR(error);
......
...@@ -630,39 +630,18 @@ nfs_find_actor(struct inode *inode, unsigned long ino, void *opaque) ...@@ -630,39 +630,18 @@ nfs_find_actor(struct inode *inode, unsigned long ino, void *opaque)
struct nfs_fh *fh = desc->fh; struct nfs_fh *fh = desc->fh;
struct nfs_fattr *fattr = desc->fattr; struct nfs_fattr *fattr = desc->fattr;
if (NFS_FSID(inode) != fattr->fsid)
return 0;
if (NFS_FILEID(inode) != fattr->fileid) if (NFS_FILEID(inode) != fattr->fileid)
return 0; return 0;
if (memcmp(NFS_FH(inode), fh, sizeof(struct nfs_fh)) != 0) if (memcmp(NFS_FH(inode), fh, sizeof(struct nfs_fh)) != 0)
return 0; return 0;
if (is_bad_inode(inode))
return 0;
/* Force an attribute cache update if inode->i_count == 0 */ /* Force an attribute cache update if inode->i_count == 0 */
if (!atomic_read(&inode->i_count)) if (!atomic_read(&inode->i_count))
NFS_CACHEINV(inode); NFS_CACHEINV(inode);
return 1; return 1;
} }
int
nfs_inode_is_stale(struct inode *inode, struct nfs_fh *fh, struct nfs_fattr *fattr)
{
/* Empty inodes are not stale */
if (!inode->i_mode)
return 0;
if ((fattr->mode & S_IFMT) != (inode->i_mode & S_IFMT))
return 1;
if (is_bad_inode(inode) || NFS_STALE(inode))
return 1;
/* Has the filehandle changed? If so is the old one stale? */
if (memcmp(NFS_FH(inode), fh, sizeof(struct nfs_fh)) != 0 &&
__nfs_revalidate_inode(NFS_SERVER(inode),inode) == -ESTALE)
return 1;
return 0;
}
/* /*
* This is our own version of iget that looks up inodes by file handle * This is our own version of iget that looks up inodes by file handle
* instead of inode number. We use this technique instead of using * instead of inode number. We use this technique instead of using
...@@ -713,7 +692,6 @@ __nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr) ...@@ -713,7 +692,6 @@ __nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
/* We can't support UPDATE_ATIME(), since the server will reset it */ /* We can't support UPDATE_ATIME(), since the server will reset it */
NFS_FLAGS(inode) &= ~NFS_INO_NEW; NFS_FLAGS(inode) &= ~NFS_INO_NEW;
NFS_FILEID(inode) = fattr->fileid; NFS_FILEID(inode) = fattr->fileid;
NFS_FSID(inode) = fattr->fsid;
memcpy(NFS_FH(inode), fh, sizeof(struct nfs_fh)); memcpy(NFS_FH(inode), fh, sizeof(struct nfs_fh));
inode->i_flags |= S_NOATIME; inode->i_flags |= S_NOATIME;
inode->i_mode = fattr->mode; inode->i_mode = fattr->mode;
...@@ -787,7 +765,7 @@ nfs_notify_change(struct dentry *dentry, struct iattr *attr) ...@@ -787,7 +765,7 @@ nfs_notify_change(struct dentry *dentry, struct iattr *attr)
/* /*
* Make sure the inode is up-to-date. * Make sure the inode is up-to-date.
*/ */
error = nfs_revalidate(dentry); error = nfs_revalidate_inode(NFS_SERVER(inode),inode);
if (error) { if (error) {
#ifdef NFS_PARANOIA #ifdef NFS_PARANOIA
printk("nfs_notify_change: revalidate failed, error=%d\n", error); printk("nfs_notify_change: revalidate failed, error=%d\n", error);
...@@ -1025,12 +1003,11 @@ __nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) ...@@ -1025,12 +1003,11 @@ __nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
inode->i_sb->s_id, inode->i_ino, inode->i_sb->s_id, inode->i_ino,
atomic_read(&inode->i_count), fattr->valid); atomic_read(&inode->i_count), fattr->valid);
if (NFS_FSID(inode) != fattr->fsid || if (NFS_FILEID(inode) != fattr->fileid) {
NFS_FILEID(inode) != fattr->fileid) {
printk(KERN_ERR "nfs_refresh_inode: inode number mismatch\n" printk(KERN_ERR "nfs_refresh_inode: inode number mismatch\n"
"expected (0x%Lx/0x%Lx), got (0x%Lx/0x%Lx)\n", "expected (%s/0x%Lx), got (%s/0x%Lx)\n",
(long long)NFS_FSID(inode), (long long)NFS_FILEID(inode), inode->i_sb->s_id, (long long)NFS_FILEID(inode),
(long long)fattr->fsid, (long long)fattr->fileid); inode->i_sb->s_id, (long long)fattr->fileid);
goto out_err; goto out_err;
} }
...@@ -1100,8 +1077,11 @@ __nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) ...@@ -1100,8 +1077,11 @@ __nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
inode->i_atime = new_atime; inode->i_atime = new_atime;
if (NFS_CACHE_MTIME(inode) != new_mtime) {
NFS_MTIME_UPDATE(inode) = jiffies;
NFS_CACHE_MTIME(inode) = new_mtime; NFS_CACHE_MTIME(inode) = new_mtime;
inode->i_mtime = nfs_time_to_secs(new_mtime); inode->i_mtime = nfs_time_to_secs(new_mtime);
}
NFS_CACHE_ISIZE(inode) = new_size; NFS_CACHE_ISIZE(inode) = new_size;
inode->i_size = new_isize; inode->i_size = new_isize;
...@@ -1175,10 +1155,6 @@ static struct inode *nfs_alloc_inode(struct super_block *sb) ...@@ -1175,10 +1155,6 @@ static struct inode *nfs_alloc_inode(struct super_block *sb)
if (!nfsi) if (!nfsi)
return NULL; return NULL;
nfsi->flags = NFS_INO_NEW; nfsi->flags = NFS_INO_NEW;
/* do we need the next 4 lines? */
nfsi->hash_next = NULL;
nfsi->hash_prev = NULL;
nfsi->nextscan = 0;
nfsi->mm_cred = NULL; nfsi->mm_cred = NULL;
return &nfsi->vfs_inode; return &nfsi->vfs_inode;
} }
......
...@@ -80,7 +80,8 @@ nfs3_proc_lookup(struct inode *dir, struct qstr *name, ...@@ -80,7 +80,8 @@ nfs3_proc_lookup(struct inode *dir, struct qstr *name,
status = rpc_call(NFS_CLIENT(dir), NFS3PROC_GETATTR, status = rpc_call(NFS_CLIENT(dir), NFS3PROC_GETATTR,
fhandle, fattr, 0); fhandle, fattr, 0);
dprintk("NFS reply lookup: %d\n", status); dprintk("NFS reply lookup: %d\n", status);
nfs_refresh_inode(dir, &dir_attr); if (status >= 0)
status = nfs_refresh_inode(dir, &dir_attr);
return status; return status;
} }
......
...@@ -92,7 +92,6 @@ struct nfs_inode { ...@@ -92,7 +92,6 @@ struct nfs_inode {
/* /*
* The 64bit 'inode number' * The 64bit 'inode number'
*/ */
__u64 fsid;
__u64 fileid; __u64 fileid;
/* /*
...@@ -129,6 +128,12 @@ struct nfs_inode { ...@@ -129,6 +128,12 @@ struct nfs_inode {
unsigned long attrtimeo; unsigned long attrtimeo;
unsigned long attrtimeo_timestamp; unsigned long attrtimeo_timestamp;
/*
* Timestamp that dates the change made to read_cache_mtime.
* This is of use for dentry revalidation
*/
unsigned long cache_mtime_jiffies;
/* /*
* This is the cookie verifier used for NFSv3 readdir * This is the cookie verifier used for NFSv3 readdir
* operations * operations
...@@ -148,11 +153,6 @@ struct nfs_inode { ...@@ -148,11 +153,6 @@ struct nfs_inode {
ncommit, ncommit,
npages; npages;
/* Flush daemon info */
struct inode *hash_next,
*hash_prev;
unsigned long nextscan;
/* Credentials for shared mmap */ /* Credentials for shared mmap */
struct rpc_cred *mm_cred; struct rpc_cred *mm_cred;
...@@ -183,6 +183,7 @@ static inline struct nfs_inode *NFS_I(struct inode *inode) ...@@ -183,6 +183,7 @@ static inline struct nfs_inode *NFS_I(struct inode *inode)
#define NFS_CONGESTED(inode) (RPC_CONGESTED(NFS_CLIENT(inode))) #define NFS_CONGESTED(inode) (RPC_CONGESTED(NFS_CLIENT(inode)))
#define NFS_COOKIEVERF(inode) (NFS_I(inode)->cookieverf) #define NFS_COOKIEVERF(inode) (NFS_I(inode)->cookieverf)
#define NFS_READTIME(inode) (NFS_I(inode)->read_cache_jiffies) #define NFS_READTIME(inode) (NFS_I(inode)->read_cache_jiffies)
#define NFS_MTIME_UPDATE(inode) (NFS_I(inode)->cache_mtime_jiffies)
#define NFS_CACHE_CTIME(inode) (NFS_I(inode)->read_cache_ctime) #define NFS_CACHE_CTIME(inode) (NFS_I(inode)->read_cache_ctime)
#define NFS_CACHE_MTIME(inode) (NFS_I(inode)->read_cache_mtime) #define NFS_CACHE_MTIME(inode) (NFS_I(inode)->read_cache_mtime)
#define NFS_CACHE_ISIZE(inode) (NFS_I(inode)->read_cache_isize) #define NFS_CACHE_ISIZE(inode) (NFS_I(inode)->read_cache_isize)
...@@ -206,7 +207,6 @@ do { \ ...@@ -206,7 +207,6 @@ do { \
#define NFS_NEW(inode) (NFS_FLAGS(inode) & NFS_INO_NEW) #define NFS_NEW(inode) (NFS_FLAGS(inode) & NFS_INO_NEW)
#define NFS_FILEID(inode) (NFS_I(inode)->fileid) #define NFS_FILEID(inode) (NFS_I(inode)->fileid)
#define NFS_FSID(inode) (NFS_I(inode)->fsid)
/* Inode Flags */ /* Inode Flags */
#define NFS_USE_READDIRPLUS(inode) ((NFS_FLAGS(inode) & NFS_INO_ADVISE_RDPLUS) ? 1 : 0) #define NFS_USE_READDIRPLUS(inode) ((NFS_FLAGS(inode) & NFS_INO_ADVISE_RDPLUS) ? 1 : 0)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment