Skip to content

Commit

Permalink
[PATCH] NFS: Introduce the use of inode->i_lock to protect fields in …
Browse files Browse the repository at this point in the history 
…nfsi

Down the road we want to eliminate the use of the global kernel lock entirely
from the NFS client.  To do this, we need to protect the fields in the
nfs_inode structure adequately.  Start by serializing updates to the
"cache_validity" field.

Note this change addresses an SMP hang found by [email protected], where processes
deadlock because nfs_end_data_update and nfs_revalidate_mapping update the
"cache_validity" field without proper serialization.

Test plan:
 Millions of fsx ops on SMP clients.  Run Nick Wilson's breaknfs program on
 large SMP clients.

Signed-off-by: Chuck Lever <[email protected]>
Cc: Trond Myklebust <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Chuck Lever authored and Linus Torvalds committed Aug 18, 2005
1 parent 412d582 commit dc59250
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 4 deletions.
7 changes: 7 additions & 0 deletions fs/nfs/dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page)
goto error;
}
SetPageUptodate(page);
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ATIME;
spin_unlock(&inode->i_lock);
/* Ensure consistent page alignment of the data.
* Note: assumes we have exclusive access to this mapping either
* through inode->i_sem or some other mechanism.
Expand Down Expand Up @@ -462,7 +464,9 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent,
page,
NFS_SERVER(inode)->dtsize,
desc->plus);
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ATIME;
spin_unlock(&inode->i_lock);
desc->page = page;
desc->ptr = kmap(page); /* matching kunmap in nfs_do_filldir */
if (desc->error >= 0) {
Expand Down Expand Up @@ -1596,7 +1600,10 @@ void nfs_access_add_cache(struct inode *inode, struct nfs_access_entry *set)
put_rpccred(cache->cred);
cache->cred = get_rpccred(set->cred);
}
/* FIXME: replace current access_cache BKL reliance with inode->i_lock */
spin_lock(&inode->i_lock);
nfsi->cache_validity &= ~NFS_INO_INVALID_ACCESS;
spin_unlock(&inode->i_lock);
cache->jiffies = set->jiffies;
cache->mask = set->mask;
}
Expand Down
34 changes: 31 additions & 3 deletions fs/nfs/inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,8 @@ nfs_zap_caches(struct inode *inode)
struct nfs_inode *nfsi = NFS_I(inode);
int mode = inode->i_mode;

spin_lock(&inode->i_lock);

NFS_ATTRTIMEO(inode) = NFS_MINATTRTIMEO(inode);
NFS_ATTRTIMEO_UPDATE(inode) = jiffies;

Expand All @@ -623,6 +625,8 @@ nfs_zap_caches(struct inode *inode)
nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL|NFS_INO_REVAL_PAGECACHE;
else
nfsi->cache_validity |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL|NFS_INO_REVAL_PAGECACHE;

spin_unlock(&inode->i_lock);
}

static void nfs_zap_acl_cache(struct inode *inode)
Expand All @@ -632,7 +636,9 @@ static void nfs_zap_acl_cache(struct inode *inode)
clear_acl_cache = NFS_PROTO(inode)->clear_acl_cache;
if (clear_acl_cache != NULL)
clear_acl_cache(inode);
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_ACL;
spin_unlock(&inode->i_lock);
}

/*
Expand Down Expand Up @@ -841,7 +847,9 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr)
inode->i_uid = attr->ia_uid;
if ((attr->ia_valid & ATTR_GID) != 0)
inode->i_gid = attr->ia_gid;
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
spin_unlock(&inode->i_lock);
}
if ((attr->ia_valid & ATTR_SIZE) != 0) {
inode->i_size = attr->ia_size;
Expand Down Expand Up @@ -1082,6 +1090,7 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
(long long)NFS_FILEID(inode), status);
goto out;
}
spin_lock(&inode->i_lock);
cache_validity = nfsi->cache_validity;
nfsi->cache_validity &= ~NFS_INO_REVAL_PAGECACHE;

Expand All @@ -1091,6 +1100,7 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
*/
if (verifier == nfsi->cache_change_attribute)
nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME);
spin_unlock(&inode->i_lock);

nfs_revalidate_mapping(inode, inode->i_mapping);

Expand Down Expand Up @@ -1149,12 +1159,16 @@ void nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping)
nfs_wb_all(inode);
}
invalidate_inode_pages2(mapping);

spin_lock(&inode->i_lock);
nfsi->cache_validity &= ~NFS_INO_INVALID_DATA;
if (S_ISDIR(inode->i_mode)) {
memset(nfsi->cookieverf, 0, sizeof(nfsi->cookieverf));
/* This ensures we revalidate child dentries */
nfsi->cache_change_attribute++;
}
spin_unlock(&inode->i_lock);

dfprintk(PAGECACHE, "NFS: (%s/%Ld) data cache invalidated\n",
inode->i_sb->s_id,
(long long)NFS_FILEID(inode));
Expand Down Expand Up @@ -1184,10 +1198,12 @@ void nfs_end_data_update(struct inode *inode)

if (!nfs_have_delegation(inode, FMODE_READ)) {
/* Mark the attribute cache for revalidation */
spin_lock(&inode->i_lock);
nfsi->cache_validity |= NFS_INO_INVALID_ATTR;
/* Directories and symlinks: invalidate page cache too */
if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
nfsi->cache_validity |= NFS_INO_INVALID_DATA;
spin_unlock(&inode->i_lock);
}
nfsi->cache_change_attribute ++;
atomic_dec(&nfsi->data_updates);
Expand All @@ -1212,6 +1228,8 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
if (nfs_have_delegation(inode, FMODE_READ))
return 0;

spin_lock(&inode->i_lock);

/* Are we in the process of updating data on the server? */
data_unstable = nfs_caches_unstable(inode);

Expand All @@ -1226,13 +1244,17 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
}
}

if ((fattr->valid & NFS_ATTR_FATTR) == 0)
if ((fattr->valid & NFS_ATTR_FATTR) == 0) {
spin_unlock(&inode->i_lock);
return 0;
}

/* Has the inode gone and changed behind our back? */
if (nfsi->fileid != fattr->fileid
|| (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT))
|| (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT)) {
spin_unlock(&inode->i_lock);
return -EIO;
}

cur_size = i_size_read(inode);
new_isize = nfs_size_to_loff_t(fattr->size);
Expand Down Expand Up @@ -1271,6 +1293,7 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
nfsi->cache_validity |= NFS_INO_INVALID_ATIME;

nfsi->read_cache_jiffies = fattr->timestamp;
spin_unlock(&inode->i_lock);
return 0;
}

Expand Down Expand Up @@ -1309,11 +1332,15 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign
goto out_err;
}

spin_lock(&inode->i_lock);

/*
* Make sure the inode's type hasn't changed.
*/
if ((inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT))
if ((inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT)) {
spin_unlock(&inode->i_lock);
goto out_changed;
}

/*
* Update the read time so we don't revalidate too often.
Expand Down Expand Up @@ -1406,6 +1433,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign
if (!nfs_have_delegation(inode, FMODE_READ))
nfsi->cache_validity |= invalid;

spin_unlock(&inode->i_lock);
return 0;
out_changed:
/*
Expand Down
2 changes: 2 additions & 0 deletions fs/nfs/nfs3acl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,9 @@ static int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
nfs_begin_data_update(inode);
status = rpc_call(server->client_acl, ACLPROC3_SETACL,
&args, &fattr, 0);
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ACCESS;
spin_unlock(&inode->i_lock);
nfs_end_data_update(inode);
dprintk("NFS reply setacl: %d\n", status);

Expand Down
4 changes: 4 additions & 0 deletions fs/nfs/read.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ static int nfs_readpage_sync(struct nfs_open_context *ctx, struct inode *inode,
if (rdata->res.eof != 0 || result == 0)
break;
} while (count);
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ATIME;
spin_unlock(&inode->i_lock);

if (count)
memclear_highpage_flush(page, rdata->args.pgbase, count);
Expand Down Expand Up @@ -473,7 +475,9 @@ void nfs_readpage_result(struct rpc_task *task)
}
task->tk_status = -EIO;
}
spin_lock(&data->inode->i_lock);
NFS_I(data->inode)->cache_validity |= NFS_INO_INVALID_ATIME;
spin_unlock(&data->inode->i_lock);
data->complete(data, status);
}

Expand Down
5 changes: 4 additions & 1 deletion include/linux/nfs_fs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,11 @@ static inline int nfs_caches_unstable(struct inode *inode)

static inline void NFS_CACHEINV(struct inode *inode)
{
if (!nfs_caches_unstable(inode))
if (!nfs_caches_unstable(inode)) {
spin_lock(&inode->i_lock);
NFS_I(inode)->cache_validity |= NFS_INO_INVALID_ATTR | NFS_INO_INVALID_ACCESS;
spin_unlock(&inode->i_lock);
}
}

static inline int nfs_server_capable(struct inode *inode, int cap)
Expand Down

0 comments on commit dc59250

Please sign in to comment.