[linux-cifs-client] [PATCH 2/8] cifs: overhaul cifs_revalidate and rename to cifs_revalidate_dentry
Jeff Layton
jlayton at redhat.com
Thu Feb 11 12:58:44 MST 2010
cifs_revalidate is renamed to cifs_revalidate_dentry as a later patch
will add a by-filehandle variant.
Add a new "invalid_mapping" flag to the cifsInodeInfo that indicates
that the pagecache is considered invalid. Add a new routine to check
inode attributes whenever they're updated and set that flag if the inode
has changed on the server.
cifs_revalidate_dentry is then changed to just update the attrcache if
needed and then to zap the pagecache if it's not valid.
There are some other behavior changes in here as well. Open files are
now allowed to have their caches invalidated. I see no reason why we'd
want to keep stale data around just because a file is open. Also,
cifs_revalidate_cache uses the server_eof for revalidating the file
size since that should more closely match the size of the file on the
server.
Signed-off-by: Jeff Layton <jlayton at redhat.com>
---
fs/cifs/cifsfs.c | 3 +-
fs/cifs/cifsfs.h | 2 +-
fs/cifs/cifsglob.h | 1 +
fs/cifs/dir.c | 2 +-
fs/cifs/file.c | 2 +-
fs/cifs/inode.c | 211 ++++++++++++++++++++++++----------------------------
6 files changed, 104 insertions(+), 117 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 8c6a036..cf85a41 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -312,6 +312,7 @@ cifs_alloc_inode(struct super_block *sb)
cifs_inode->clientCanCacheRead = false;
cifs_inode->clientCanCacheAll = false;
cifs_inode->delete_pending = false;
+ cifs_inode->invalid_mapping = false;
cifs_inode->vfs_inode.i_blkbits = 14; /* 2**14 = CIFS_MAX_MSGSIZE */
cifs_inode->server_eof = 0;
@@ -638,7 +639,7 @@ static loff_t cifs_llseek(struct file *file, loff_t offset, int origin)
setting the revalidate time to zero */
CIFS_I(file->f_path.dentry->d_inode)->time = 0;
- retval = cifs_revalidate(file->f_path.dentry);
+ retval = cifs_revalidate_dentry(file->f_path.dentry);
if (retval < 0)
return (loff_t)retval;
}
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 78c1b86..2af995c 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -61,7 +61,7 @@ extern int cifs_mkdir(struct inode *, struct dentry *, int);
extern int cifs_rmdir(struct inode *, struct dentry *);
extern int cifs_rename(struct inode *, struct dentry *, struct inode *,
struct dentry *);
-extern int cifs_revalidate(struct dentry *);
+extern int cifs_revalidate_dentry(struct dentry *);
extern int cifs_getattr(struct vfsmount *, struct dentry *, struct kstat *);
extern int cifs_setattr(struct dentry *, struct iattr *);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ed751bb..344488c 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -389,6 +389,7 @@ struct cifsInodeInfo {
bool clientCanCacheRead:1; /* read oplock */
bool clientCanCacheAll:1; /* read and writebehind oplock */
bool delete_pending:1; /* DELETE_ON_CLOSE is set */
+ bool invalid_mapping:1; /* pagecache is invalid */
u64 server_eof; /* current file size on server */
u64 uniqueid; /* server inode number */
struct inode vfs_inode;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 6ccf726..e9f7ecc 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -739,7 +739,7 @@ cifs_d_revalidate(struct dentry *direntry, struct nameidata *nd)
int isValid = 1;
if (direntry->d_inode) {
- if (cifs_revalidate(direntry))
+ if (cifs_revalidate_dentry(direntry))
return 0;
} else {
cFYI(1, ("neg dentry 0x%p name = %s",
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 057e1da..1f5d763 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1894,7 +1894,7 @@ int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
int rc, xid;
xid = GetXid();
- rc = cifs_revalidate(dentry);
+ rc = cifs_revalidate_dentry(dentry);
if (rc) {
cFYI(1, ("Validation prior to mmap failed, error=%d", rc));
FreeXid(xid);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 00f8f33..f7eb88c 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -77,6 +77,41 @@ static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
}
}
+/* check inode attributes against fattr. If they don't match, tag the
+ * inode for cache invalidation
+ */
+static void
+cifs_revalidate_cache(struct inode *inode, struct cifs_fattr *fattr)
+{
+ struct cifsInodeInfo *cifs_i = CIFS_I(inode);
+
+ cFYI(1, ("%s: revalidating inode %llu", __func__, cifs_i->uniqueid));
+
+ if (inode->i_state & I_NEW) {
+ cFYI(1, ("%s: inode %llu is new", __func__, cifs_i->uniqueid));
+ return;
+ }
+
+ /* don't bother with revalidation if we have an oplock */
+ if (cifs_i->clientCanCacheRead) {
+ cFYI(1, ("%s: inode %llu is oplocked", __func__,
+ cifs_i->uniqueid));
+ return;
+ }
+
+ /* revalidate if mtime or size have changed */
+ if (timespec_equal(&inode->i_mtime, &fattr->cf_mtime) &&
+ cifs_i->server_eof == fattr->cf_eof) {
+ cFYI(1, ("%s: inode %llu is unchanged", __func__,
+ cifs_i->uniqueid));
+ return;
+ }
+
+ cFYI(1, ("%s: invalidating inode %llu mapping", __func__,
+ cifs_i->uniqueid));
+ cifs_i->invalid_mapping = true;
+}
+
/* populate an inode with info from a cifs_fattr struct */
void
cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
@@ -85,6 +120,8 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
unsigned long oldtime = cifs_i->time;
+ cifs_revalidate_cache(inode, fattr);
+
inode->i_atime = fattr->cf_atime;
inode->i_mtime = fattr->cf_mtime;
inode->i_ctime = fattr->cf_ctime;
@@ -1389,135 +1426,83 @@ cifs_rename_exit:
return rc;
}
-int cifs_revalidate(struct dentry *direntry)
+static bool
+cifs_inode_needs_reval(struct inode *inode)
{
- int xid;
- int rc = 0, wbrc = 0;
- char *full_path;
- struct cifs_sb_info *cifs_sb;
- struct cifsInodeInfo *cifsInode;
- loff_t local_size;
- struct timespec local_mtime;
- bool invalidate_inode = false;
+ struct cifsInodeInfo *cifs_i = CIFS_I(inode);
- if (direntry->d_inode == NULL)
- return -ENOENT;
+ if (cifs_i->clientCanCacheRead)
+ return false;
- cifsInode = CIFS_I(direntry->d_inode);
+ if (!lookupCacheEnabled)
+ return true;
- if (cifsInode == NULL)
- return -ENOENT;
+ if (cifs_i->time == 0)
+ return true;
- /* no sense revalidating inode info on file that no one can write */
- if (CIFS_I(direntry->d_inode)->clientCanCacheRead)
- return rc;
+ /* FIXME: the actimeo should be tunable */
+ if (time_after_eq(jiffies, cifs_i->time + HZ))
+ return true;
+
+ return false;
+}
+
+/* check invalid_mapping flag and zap the cache if it's set */
+static void
+cifs_invalidate_mapping(struct inode *inode)
+{
+ int rc;
+ struct cifsInodeInfo *cifs_i = CIFS_I(inode);
+
+ cifs_i->invalid_mapping = false;
+
+ /* write back any cached data */
+ if (inode->i_mapping && inode->i_mapping->nrpages != 0) {
+ rc = filemap_write_and_wait(inode->i_mapping);
+ if (rc)
+ cifs_i->write_behind_rc = rc;
+ }
+ invalidate_remote_inode(inode);
+}
+
+/* revalidate a dentry's inode attributes */
+int cifs_revalidate_dentry(struct dentry *dentry)
+{
+ int xid;
+ int rc = 0;
+ char *full_path = NULL;
+ struct inode *inode = dentry->d_inode;
+ struct super_block *sb = dentry->d_sb;
+
+ if (inode == NULL)
+ return -ENOENT;
xid = GetXid();
- cifs_sb = CIFS_SB(direntry->d_sb);
+ if (!cifs_inode_needs_reval(inode))
+ goto check_inval;
/* can not safely grab the rename sem here if rename calls revalidate
since that would deadlock */
- full_path = build_path_from_dentry(direntry);
+ full_path = build_path_from_dentry(dentry);
if (full_path == NULL) {
rc = -ENOMEM;
- FreeXid(xid);
- return rc;
+ goto check_inval;
}
- cFYI(1, ("Revalidate: %s inode 0x%p count %d dentry: 0x%p d_time %ld "
- "jiffies %ld", full_path, direntry->d_inode,
- direntry->d_inode->i_count.counter, direntry,
- direntry->d_time, jiffies));
-
- if (cifsInode->time == 0) {
- /* was set to zero previously to force revalidate */
- } else if (time_before(jiffies, cifsInode->time + HZ) &&
- lookupCacheEnabled) {
- if ((S_ISREG(direntry->d_inode->i_mode) == 0) ||
- (direntry->d_inode->i_nlink == 1)) {
- kfree(full_path);
- FreeXid(xid);
- return rc;
- } else {
- cFYI(1, ("Have to revalidate file due to hardlinks"));
- }
- }
-
- /* save mtime and size */
- local_mtime = direntry->d_inode->i_mtime;
- local_size = direntry->d_inode->i_size;
- if (cifs_sb->tcon->unix_ext) {
- rc = cifs_get_inode_info_unix(&direntry->d_inode, full_path,
- direntry->d_sb, xid);
- if (rc) {
- cFYI(1, ("error on getting revalidate info %d", rc));
-/* if (rc != -ENOENT)
- rc = 0; */ /* BB should we cache info on
- certain errors? */
- }
- } else {
- rc = cifs_get_inode_info(&direntry->d_inode, full_path, NULL,
- direntry->d_sb, xid, NULL);
- if (rc) {
- cFYI(1, ("error on getting revalidate info %d", rc));
-/* if (rc != -ENOENT)
- rc = 0; */ /* BB should we cache info on
- certain errors? */
- }
- }
- /* should we remap certain errors, access denied?, to zero */
-
- /* if not oplocked, we invalidate inode pages if mtime or file size
- had changed on server */
+ cFYI(1, ("Revalidate: %s inode 0x%p count %d dentry: 0x%p d_time %ld "
+ "jiffies %ld", full_path, inode, inode->i_count.counter,
+ dentry, dentry->d_time, jiffies));
- if (timespec_equal(&local_mtime, &direntry->d_inode->i_mtime) &&
- (local_size == direntry->d_inode->i_size)) {
- cFYI(1, ("cifs_revalidate - inode unchanged"));
- } else {
- /* file may have changed on server */
- if (cifsInode->clientCanCacheRead) {
- /* no need to invalidate inode pages since we were the
- only ones who could have modified the file and the
- server copy is staler than ours */
- } else {
- invalidate_inode = true;
- }
- }
+ if (CIFS_SB(sb)->tcon->unix_ext)
+ rc = cifs_get_inode_info_unix(&inode, full_path, sb, xid);
+ else
+ rc = cifs_get_inode_info(&inode, full_path, NULL, sb,
+ xid, NULL);
- /* can not grab this sem since kernel filesys locking documentation
- indicates i_mutex may be taken by the kernel on lookup and rename
- which could deadlock if we grab the i_mutex here as well */
-/* mutex_lock(&direntry->d_inode->i_mutex);*/
- /* need to write out dirty pages here */
- if (direntry->d_inode->i_mapping) {
- /* do we need to lock inode until after invalidate completes
- below? */
- wbrc = filemap_fdatawrite(direntry->d_inode->i_mapping);
- if (wbrc)
- CIFS_I(direntry->d_inode)->write_behind_rc = wbrc;
- }
- if (invalidate_inode) {
- /* shrink_dcache not necessary now that cifs dentry ops
- are exported for negative dentries */
-/* if (S_ISDIR(direntry->d_inode->i_mode))
- shrink_dcache_parent(direntry); */
- if (S_ISREG(direntry->d_inode->i_mode)) {
- if (direntry->d_inode->i_mapping) {
- wbrc = filemap_fdatawait(direntry->d_inode->i_mapping);
- if (wbrc)
- CIFS_I(direntry->d_inode)->write_behind_rc = wbrc;
- }
- /* may eventually have to do this for open files too */
- if (list_empty(&(cifsInode->openFileList))) {
- /* changed on server - flush read ahead pages */
- cFYI(1, ("Invalidating read ahead data on "
- "closed file"));
- invalidate_remote_inode(direntry->d_inode);
- }
- }
- }
-/* mutex_unlock(&direntry->d_inode->i_mutex); */
+check_inval:
+ if (CIFS_I(inode)->invalid_mapping)
+ cifs_invalidate_mapping(inode);
kfree(full_path);
FreeXid(xid);
@@ -1527,7 +1512,7 @@ int cifs_revalidate(struct dentry *direntry)
int cifs_getattr(struct vfsmount *mnt, struct dentry *dentry,
struct kstat *stat)
{
- int err = cifs_revalidate(dentry);
+ int err = cifs_revalidate_dentry(dentry);
if (!err) {
generic_fillattr(dentry->d_inode, stat);
stat->blksize = CIFS_MAX_MSGSIZE;
--
1.6.6
More information about the linux-cifs-client
mailing list