[PATCH] NFS: Introduce the use of inode->i_lock to protect fields in 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 njw@osdl.org, 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 <cel@netapp.com>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 27cf5577f239f04206a44ff8bfc8113c328f58b9..147cbf9261cedba976acade07391e43d4af19eb3 100644 (file)
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -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.
@@ -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) {
@@ -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;
 }

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index ee27578277f3ebf8c17607744791ed56b73e85db..541b418327c8b18b8f30d294beca9c10d3ca732e 100644 (file)
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -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;
 
@@ -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)
@@ -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);
 }
 
 /*
@@ -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;
@@ -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;
 
@@ -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);
 
@@ -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));
@@ -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);
@@ -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);
 
@@ -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);
@@ -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;
 }
 
@@ -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.
@@ -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:
        /*

diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index a020e650ffc29657d812fc1f635368a725bbd630..6a5bbc0ae941aa690e1f73137cbce4af38902092 100644 (file)
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -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);
 

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 90df0500ca1b1f89b7e08f6b91c7e9c49c91b292..6ceb1d471f2064952d8b4727a28711e821f5aba9 100644 (file)
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -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);
@@ -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);
 }
 

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index deef9567788aa7ed9a78a39117b472c045b97e1e..9a6047ff1b25b198a869678463ed0561cbf417cc 100644 (file)
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -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)