nfs: remove extraneous and problematic calls to nfs_clear_request

When a nfs_page is freed, nfs_free_request is called which also calls
nfs_clear_request to clean out the lock and open contexts and free the
pagecache page.

However, a couple of places in the nfs code call nfs_clear_request
themselves. What happens here if the refcount on the request is still high?
We'll be releasing contexts and freeing pointers while the request is
possibly still in use.

Remove those bare calls to nfs_clear_context. That should only be done when
the request is being freed.

Note that when doing this, we need to watch out for tests of req->wb_page.
Previously, nfs_set_page_tag_locked() and nfs_clear_page_tag_locked()
would check the value of req->wb_page to figure out if the page is mapped
into the nfsi->nfs_page_tree. We now indicate the page is mapped using
the new bit PG_MAPPED in req->wb_flags .

Reported-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 137b549e63dbcfa3971aac43091eb91a29a7b69f..b68536cc9046ec937b9b4ccddb98b2acf468f1db 100644 (file)
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -115,7 +115,7 @@ int nfs_set_page_tag_locked(struct nfs_page *req)
 {
        if (!nfs_lock_request_dontget(req))
                return 0;
-       if (req->wb_page != NULL)
+       if (test_bit(PG_MAPPED, &req->wb_flags))
                radix_tree_tag_set(&NFS_I(req->wb_context->path.dentry->d_inode)->nfs_page_tree, req->wb_index, NFS_PAGE_TAG_LOCKED);
        return 1;
 }
@@ -125,7 +125,7 @@ int nfs_set_page_tag_locked(struct nfs_page *req)
  */
 void nfs_clear_page_tag_locked(struct nfs_page *req)
 {
-       if (req->wb_page != NULL) {
+       if (test_bit(PG_MAPPED, &req->wb_flags)) {
                struct inode *inode = req->wb_context->path.dentry->d_inode;
                struct nfs_inode *nfsi = NFS_I(inode);
 

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index e4b62c6f5a6e9eb721eda53d836c41055ab2e1b7..aedcaa7f291fbe4405f2b68c13f407c7de6be9ba 100644 (file)
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -152,7 +152,6 @@ static void nfs_readpage_release(struct nfs_page *req)
                        (long long)NFS_FILEID(req->wb_context->path.dentry->d_inode),
                        req->wb_bytes,
                        (long long)req_offset(req));
-       nfs_clear_request(req);
        nfs_release_request(req);
 }
 

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 4c14c17a5276a410bb383e3650f106ab56cf47d6..10d648ea128bf6345df70da0e15ee8ce443ab8e6 100644 (file)
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -390,6 +390,7 @@ static int nfs_inode_add_request(struct inode *inode, struct nfs_page *req)
                if (nfs_have_delegation(inode, FMODE_WRITE))
                        nfsi->change_attr++;
        }
+       set_bit(PG_MAPPED, &req->wb_flags);
        SetPagePrivate(req->wb_page);
        set_page_private(req->wb_page, (unsigned long)req);
        nfsi->npages++;
@@ -415,6 +416,7 @@ static void nfs_inode_remove_request(struct nfs_page *req)
        spin_lock(&inode->i_lock);
        set_page_private(req->wb_page, 0);
        ClearPagePrivate(req->wb_page);
+       clear_bit(PG_MAPPED, &req->wb_flags);
        radix_tree_delete(&nfsi->nfs_page_tree, req->wb_index);
        nfsi->npages--;
        if (!nfsi->npages) {
@@ -422,7 +424,6 @@ static void nfs_inode_remove_request(struct nfs_page *req)
                iput(inode);
        } else
                spin_unlock(&inode->i_lock);
-       nfs_clear_request(req);
        nfs_release_request(req);
 }
 

diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
index f8b60e7f4c44d9b652a94bca2ab8945130bdb061..d55cee73f63477a326b73cf7eb15e3e277567b14 100644 (file)
--- a/include/linux/nfs_page.h
+++ b/include/linux/nfs_page.h
@@ -29,6 +29,7 @@
  */
 enum {
        PG_BUSY = 0,
+       PG_MAPPED,
        PG_CLEAN,
        PG_NEED_COMMIT,
        PG_NEED_RESCHED,