Skip to content

Commit

Permalink
NFS: Set meaningful value for fattr->time_start in readdirplus results.
Browse files Browse the repository at this point in the history
Don't use uninitialsed value for fattr->time_start in readdirplus results.

The 'fattr' structure filled in by nfs3_decode_direct does not get a
value for ->time_start set.
Thus if an entry is for an inode that we already have in cache,
when nfs_readdir_lookup calls nfs_fhget, it will call nfs_refresh_inode
and may update the inode with out-of-date information.

Directories are read a page at a time, so each page could have a
different timestamp that "should" be used to set the time_start for
the fattr for info in that page.  However storing the timestamp per
page is awkward.  (We could stick in the first 4 bytes and only read 4092
bytes, but that is a bigger code change than I am interested it).

This patch ignores the readdir_plus attributes if a readdir finds the
information already in cache, and otherwise sets ->time_start to the time
the readdir request was sent to the server.

It might be nice to store - in the directory inode - the time stamp for
the earliest readdir request that is still in the page cache, so that we
don't ignore attribute data that we don't have to.  This patch doesn't do
that.

Signed-off-by: Neil Brown <neilb@suse.de>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
  • Loading branch information
Neil Brown authored and Trond Myklebust committed May 1, 2007
1 parent 74dd34e commit 1f4eab7
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions fs/nfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ typedef struct {
decode_dirent_t decode;
int plus;
int error;
unsigned long timestamp;
int timestamp_valid;
} nfs_readdir_descriptor_t;

/* Now we cache directories properly, by stuffing the dirent
Expand Down Expand Up @@ -195,6 +197,8 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page)
}
goto error;
}
desc->timestamp = timestamp;
desc->timestamp_valid = 1;
SetPageUptodate(page);
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ATIME;
Expand Down Expand Up @@ -225,6 +229,10 @@ int dir_decode(nfs_readdir_descriptor_t *desc)
if (IS_ERR(p))
return PTR_ERR(p);
desc->ptr = p;
if (desc->timestamp_valid)
desc->entry->fattr->time_start = desc->timestamp;
else
desc->entry->fattr->valid &= ~NFS_ATTR_FATTR;
return 0;
}

Expand Down Expand Up @@ -316,6 +324,10 @@ int find_dirent_page(nfs_readdir_descriptor_t *desc)
__FUNCTION__, desc->page_index,
(long long) *desc->dir_cookie);

/* If we find the page in the page_cache, we cannot be sure
* how fresh the data is, so we will ignore readdir_plus attributes.
*/
desc->timestamp_valid = 0;
page = read_cache_page(inode->i_mapping, desc->page_index,
(filler_t *)nfs_readdir_filler, desc);
if (IS_ERR(page)) {
Expand Down Expand Up @@ -468,6 +480,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent,
struct rpc_cred *cred = nfs_file_cred(file);
struct page *page = NULL;
int status;
unsigned long timestamp;

dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n",
(unsigned long long)*desc->dir_cookie);
Expand All @@ -477,6 +490,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent,
status = -ENOMEM;
goto out;
}
timestamp = jiffies;
desc->error = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, *desc->dir_cookie,
page,
NFS_SERVER(inode)->dtsize,
Expand All @@ -487,6 +501,8 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent,
desc->page = page;
desc->ptr = kmap(page); /* matching kunmap in nfs_do_filldir */
if (desc->error >= 0) {
desc->timestamp = timestamp;
desc->timestamp_valid = 1;
if ((status = dir_decode(desc)) == 0)
desc->entry->prev_cookie = *desc->dir_cookie;
} else
Expand Down

0 comments on commit 1f4eab7

Please sign in to comment.