NFS: Cleanup of NFS read code
Trond Myklebust [Mon, 20 Mar 2006 18:44:27 +0000 (13:44 -0500)]
Same callback hierarchy inversion as for the NFS write calls. This patch is
not strictly speaking needed by the O_DIRECT code, but avoids confusing
differences between the asynchronous read and write code.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

fs/nfs/direct.c
fs/nfs/nfs3proc.c
fs/nfs/nfs4proc.c
fs/nfs/proc.c
fs/nfs/read.c
include/linux/nfs_fs.h
include/linux/nfs_xdr.h

index fc07ce4..3f87a72 100644 (file)
@@ -218,14 +218,17 @@ static struct nfs_direct_req *nfs_direct_read_alloc(size_t nbytes, unsigned int
  * until the RPCs complete.  This could be long *after* we are woken up in
  * nfs_direct_read_wait (for instance, if someone hits ^C on a slow server).
  */
-static void nfs_direct_read_result(struct nfs_read_data *data, int status)
+static void nfs_direct_read_result(struct rpc_task *task, void *calldata)
 {
+       struct nfs_read_data *data = calldata;
        struct nfs_direct_req *dreq = (struct nfs_direct_req *) data->req;
 
-       if (likely(status >= 0))
+       if (nfs_readpage_result(task, data) != 0)
+               return;
+       if (likely(task->tk_status >= 0))
                atomic_add(data->res.count, &dreq->count);
        else
-               atomic_set(&dreq->error, status);
+               atomic_set(&dreq->error, task->tk_status);
 
        if (unlikely(atomic_dec_and_test(&dreq->complete))) {
                nfs_free_user_pages(dreq->pages, dreq->npages, 1);
@@ -234,6 +237,11 @@ static void nfs_direct_read_result(struct nfs_read_data *data, int status)
        }
 }
 
+static const struct rpc_call_ops nfs_read_direct_ops = {
+       .rpc_call_done = nfs_direct_read_result,
+       .rpc_release = nfs_readdata_release,
+};
+
 /**
  * nfs_direct_read_schedule - dispatch NFS READ operations for a direct read
  * @dreq: address of nfs_direct_req struct for this request
@@ -280,10 +288,11 @@ static void nfs_direct_read_schedule(struct nfs_direct_req *dreq,
                data->res.eof = 0;
                data->res.count = bytes;
 
+               rpc_init_task(&data->task, NFS_CLIENT(inode), RPC_TASK_ASYNC,
+                               &nfs_read_direct_ops, data);
                NFS_PROTO(inode)->read_setup(data);
 
                data->task.tk_cookie = (unsigned long) inode;
-               data->complete = nfs_direct_read_result;
 
                lock_kernel();
                rpc_execute(&data->task);
index c4f7de8..cf186f0 100644 (file)
@@ -811,29 +811,18 @@ nfs3_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
 
 extern u32 *nfs3_decode_dirent(u32 *, struct nfs_entry *, int);
 
-static void nfs3_read_done(struct rpc_task *task, void *calldata)
+static int nfs3_read_done(struct rpc_task *task, struct nfs_read_data *data)
 {
-       struct nfs_read_data *data = calldata;
-
        if (nfs3_async_handle_jukebox(task, data->inode))
-               return;
+               return -EAGAIN;
        /* Call back common NFS readpage processing */
        if (task->tk_status >= 0)
                nfs_refresh_inode(data->inode, &data->fattr);
-       nfs_readpage_result(task, calldata);
+       return 0;
 }
 
-static const struct rpc_call_ops nfs3_read_ops = {
-       .rpc_call_done = nfs3_read_done,
-       .rpc_release = nfs_readdata_release,
-};
-
-static void
-nfs3_proc_read_setup(struct nfs_read_data *data)
+static void nfs3_proc_read_setup(struct nfs_read_data *data)
 {
-       struct rpc_task         *task = &data->task;
-       struct inode            *inode = data->inode;
-       int                     flags;
        struct rpc_message      msg = {
                .rpc_proc       = &nfs3_procedures[NFS3PROC_READ],
                .rpc_argp       = &data->args,
@@ -841,12 +830,7 @@ nfs3_proc_read_setup(struct nfs_read_data *data)
                .rpc_cred       = data->cred,
        };
 
-       /* N.B. Do we need to test? Never called for swapfile inode */
-       flags = RPC_TASK_ASYNC | (IS_SWAPFILE(inode)? NFS_RPC_SWAPFLAGS : 0);
-
-       /* Finalize the task. */
-       rpc_init_task(task, NFS_CLIENT(inode), flags, &nfs3_read_ops, data);
-       rpc_call_setup(task, &msg, 0);
+       rpc_call_setup(&data->task, &msg, 0);
 }
 
 static int nfs3_write_done(struct rpc_task *task, struct nfs_write_data *data)
@@ -935,6 +919,7 @@ struct nfs_rpc_ops  nfs_v3_clientops = {
        .pathconf       = nfs3_proc_pathconf,
        .decode_dirent  = nfs3_decode_dirent,
        .read_setup     = nfs3_proc_read_setup,
+       .read_done      = nfs3_read_done,
        .write_setup    = nfs3_proc_write_setup,
        .write_done     = nfs3_write_done,
        .commit_setup   = nfs3_proc_commit_setup,
index ef4dc31..bad1eae 100644 (file)
@@ -2345,47 +2345,31 @@ static int nfs4_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
        return err;
 }
 
-static void nfs4_read_done(struct rpc_task *task, void *calldata)
+static int nfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)
 {
-       struct nfs_read_data *data = calldata;
-       struct inode *inode = data->inode;
+       struct nfs_server *server = NFS_SERVER(data->inode);
 
-       if (nfs4_async_handle_error(task, NFS_SERVER(inode)) == -EAGAIN) {
+       if (nfs4_async_handle_error(task, server) == -EAGAIN) {
                rpc_restart_call(task);
-               return;
+               return -EAGAIN;
        }
        if (task->tk_status > 0)
-               renew_lease(NFS_SERVER(inode), data->timestamp);
-       /* Call back common NFS readpage processing */
-       nfs_readpage_result(task, calldata);
+               renew_lease(server, data->timestamp);
+       return 0;
 }
 
-static const struct rpc_call_ops nfs4_read_ops = {
-       .rpc_call_done = nfs4_read_done,
-       .rpc_release = nfs_readdata_release,
-};
-
-static void
-nfs4_proc_read_setup(struct nfs_read_data *data)
+static void nfs4_proc_read_setup(struct nfs_read_data *data)
 {
-       struct rpc_task *task = &data->task;
        struct rpc_message msg = {
                .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ],
                .rpc_argp = &data->args,
                .rpc_resp = &data->res,
                .rpc_cred = data->cred,
        };
-       struct inode *inode = data->inode;
-       int flags;
 
        data->timestamp   = jiffies;
 
-       /* N.B. Do we need to test? Never called for swapfile inode */
-       flags = RPC_TASK_ASYNC | (IS_SWAPFILE(inode)? NFS_RPC_SWAPFLAGS : 0);
-
-       /* Finalize the task. */
-       rpc_init_task(task, NFS_CLIENT(inode), flags, &nfs4_read_ops, data);
-       rpc_call_setup(task, &msg, 0);
+       rpc_call_setup(&data->task, &msg, 0);
 }
 
 static int nfs4_write_done(struct rpc_task *task, struct nfs_write_data *data)
@@ -3617,6 +3601,7 @@ struct nfs_rpc_ops        nfs_v4_clientops = {
        .pathconf       = nfs4_proc_pathconf,
        .decode_dirent  = nfs4_decode_dirent,
        .read_setup     = nfs4_proc_read_setup,
+       .read_done      = nfs4_read_done,
        .write_setup    = nfs4_proc_write_setup,
        .write_done     = nfs4_write_done,
        .commit_setup   = nfs4_proc_commit_setup,
index 608aa59..9dd85ca 100644 (file)
@@ -613,10 +613,8 @@ nfs_proc_pathconf(struct nfs_server *server, struct nfs_fh *fhandle,
 
 extern u32 * nfs_decode_dirent(u32 *, struct nfs_entry *, int);
 
-static void nfs_read_done(struct rpc_task *task, void *calldata)
+static int nfs_read_done(struct rpc_task *task, struct nfs_read_data *data)
 {
-       struct nfs_read_data *data = calldata;
-
        if (task->tk_status >= 0) {
                nfs_refresh_inode(data->inode, data->res.fattr);
                /* Emulate the eof flag, which isn't normally needed in NFSv2
@@ -625,20 +623,11 @@ static void nfs_read_done(struct rpc_task *task, void *calldata)
                if (data->args.offset + data->args.count >= data->res.fattr->size)
                        data->res.eof = 1;
        }
-       nfs_readpage_result(task, calldata);
+       return 0;
 }
 
-static const struct rpc_call_ops nfs_read_ops = {
-       .rpc_call_done = nfs_read_done,
-       .rpc_release = nfs_readdata_release,
-};
-
-static void
-nfs_proc_read_setup(struct nfs_read_data *data)
+static void nfs_proc_read_setup(struct nfs_read_data *data)
 {
-       struct rpc_task         *task = &data->task;
-       struct inode            *inode = data->inode;
-       int                     flags;
        struct rpc_message      msg = {
                .rpc_proc       = &nfs_procedures[NFSPROC_READ],
                .rpc_argp       = &data->args,
@@ -646,12 +635,7 @@ nfs_proc_read_setup(struct nfs_read_data *data)
                .rpc_cred       = data->cred,
        };
 
-       /* N.B. Do we need to test? Never called for swapfile inode */
-       flags = RPC_TASK_ASYNC | (IS_SWAPFILE(inode)? NFS_RPC_SWAPFLAGS : 0);
-
-       /* Finalize the task. */
-       rpc_init_task(task, NFS_CLIENT(inode), flags, &nfs_read_ops, data);
-       rpc_call_setup(task, &msg, 0);
+       rpc_call_setup(&data->task, &msg, 0);
 }
 
 static int nfs_write_done(struct rpc_task *task, struct nfs_write_data *data)
@@ -720,6 +704,7 @@ struct nfs_rpc_ops  nfs_v2_clientops = {
        .pathconf       = nfs_proc_pathconf,
        .decode_dirent  = nfs_decode_dirent,
        .read_setup     = nfs_proc_read_setup,
+       .read_done      = nfs_read_done,
        .write_setup    = nfs_proc_write_setup,
        .write_done     = nfs_write_done,
        .commit_setup   = nfs_proc_commit_setup,
index ae3ddd2..2da255f 100644 (file)
@@ -36,8 +36,8 @@
 #define NFSDBG_FACILITY                NFSDBG_PAGECACHE
 
 static int nfs_pagein_one(struct list_head *, struct inode *);
-static void nfs_readpage_result_partial(struct nfs_read_data *, int);
-static void nfs_readpage_result_full(struct nfs_read_data *, int);
+static const struct rpc_call_ops nfs_read_partial_ops;
+static const struct rpc_call_ops nfs_read_full_ops;
 
 static kmem_cache_t *nfs_rdata_cachep;
 mempool_t *nfs_rdata_mempool;
@@ -200,9 +200,11 @@ static void nfs_readpage_release(struct nfs_page *req)
  * Set up the NFS read request struct
  */
 static void nfs_read_rpcsetup(struct nfs_page *req, struct nfs_read_data *data,
+               const struct rpc_call_ops *call_ops,
                unsigned int count, unsigned int offset)
 {
        struct inode            *inode;
+       int flags;
 
        data->req         = req;
        data->inode       = inode = req->wb_context->dentry->d_inode;
@@ -220,6 +222,9 @@ static void nfs_read_rpcsetup(struct nfs_page *req, struct nfs_read_data *data,
        data->res.eof     = 0;
        nfs_fattr_init(&data->fattr);
 
+       /* Set up the initial task struct. */
+       flags = RPC_TASK_ASYNC | (IS_SWAPFILE(inode)? NFS_RPC_SWAPFLAGS : 0);
+       rpc_init_task(&data->task, NFS_CLIENT(inode), flags, call_ops, data);
        NFS_PROTO(inode)->read_setup(data);
 
        data->task.tk_cookie = (unsigned long)inode;
@@ -307,14 +312,15 @@ static int nfs_pagein_multi(struct list_head *head, struct inode *inode)
                list_del_init(&data->pages);
 
                data->pagevec[0] = page;
-               data->complete = nfs_readpage_result_partial;
 
                if (nbytes > rsize) {
-                       nfs_read_rpcsetup(req, data, rsize, offset);
+                       nfs_read_rpcsetup(req, data, &nfs_read_partial_ops,
+                                       rsize, offset);
                        offset += rsize;
                        nbytes -= rsize;
                } else {
-                       nfs_read_rpcsetup(req, data, nbytes, offset);
+                       nfs_read_rpcsetup(req, data, &nfs_read_partial_ops,
+                                       nbytes, offset);
                        nbytes = 0;
                }
                nfs_execute_read(data);
@@ -360,8 +366,7 @@ static int nfs_pagein_one(struct list_head *head, struct inode *inode)
        }
        req = nfs_list_entry(data->pages.next);
 
-       data->complete = nfs_readpage_result_full;
-       nfs_read_rpcsetup(req, data, count, 0);
+       nfs_read_rpcsetup(req, data, &nfs_read_full_ops, count, 0);
 
        nfs_execute_read(data);
        return 0;
@@ -395,12 +400,15 @@ nfs_pagein_list(struct list_head *head, int rpages)
 /*
  * Handle a read reply that fills part of a page.
  */
-static void nfs_readpage_result_partial(struct nfs_read_data *data, int status)
+static void nfs_readpage_result_partial(struct rpc_task *task, void *calldata)
 {
+       struct nfs_read_data *data = calldata;
        struct nfs_page *req = data->req;
        struct page *page = req->wb_page;
  
-       if (status >= 0) {
+       if (nfs_readpage_result(task, data) != 0)
+               return;
+       if (task->tk_status >= 0) {
                unsigned int request = data->args.count;
                unsigned int result = data->res.count;
 
@@ -419,20 +427,28 @@ static void nfs_readpage_result_partial(struct nfs_read_data *data, int status)
        }
 }
 
+static const struct rpc_call_ops nfs_read_partial_ops = {
+       .rpc_call_done = nfs_readpage_result_partial,
+       .rpc_release = nfs_readdata_release,
+};
+
 /*
  * This is the callback from RPC telling us whether a reply was
  * received or some error occurred (timeout or socket shutdown).
  */
-static void nfs_readpage_result_full(struct nfs_read_data *data, int status)
+static void nfs_readpage_result_full(struct rpc_task *task, void *calldata)
 {
+       struct nfs_read_data *data = calldata;
        unsigned int count = data->res.count;
 
+       if (nfs_readpage_result(task, data) != 0)
+               return;
        while (!list_empty(&data->pages)) {
                struct nfs_page *req = nfs_list_entry(data->pages.next);
                struct page *page = req->wb_page;
                nfs_list_remove_request(req);
 
-               if (status >= 0) {
+               if (task->tk_status >= 0) {
                        if (count < PAGE_CACHE_SIZE) {
                                if (count < req->wb_bytes)
                                        memclear_highpage_flush(page,
@@ -448,19 +464,27 @@ static void nfs_readpage_result_full(struct nfs_read_data *data, int status)
        }
 }
 
+static const struct rpc_call_ops nfs_read_full_ops = {
+       .rpc_call_done = nfs_readpage_result_full,
+       .rpc_release = nfs_readdata_release,
+};
+
 /*
  * This is the callback from RPC telling us whether a reply was
  * received or some error occurred (timeout or socket shutdown).
  */
-void nfs_readpage_result(struct rpc_task *task, void *calldata)
+int nfs_readpage_result(struct rpc_task *task, struct nfs_read_data *data)
 {
-       struct nfs_read_data *data = calldata;
        struct nfs_readargs *argp = &data->args;
        struct nfs_readres *resp = &data->res;
-       int status = task->tk_status;
+       int status;
 
        dprintk("NFS: %4d nfs_readpage_result, (status %d)\n",
-               task->tk_pid, status);
+               task->tk_pid, task->tk_status);
+
+       status = NFS_PROTO(data->inode)->read_done(task, data);
+       if (status != 0)
+               return status;
 
        nfs_add_stats(data->inode, NFSIOS_SERVERREADBYTES, resp->count);
 
@@ -474,14 +498,14 @@ void nfs_readpage_result(struct rpc_task *task, void *calldata)
                        argp->pgbase += resp->count;
                        argp->count -= resp->count;
                        rpc_restart_call(task);
-                       return;
+                       return -EAGAIN;
                }
                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);
+       return 0;
 }
 
 /*
index 782e597..f55827b 100644 (file)
@@ -492,8 +492,8 @@ static inline void nfs_writedata_free(struct nfs_write_data *p)
 extern int  nfs_readpage(struct file *, struct page *);
 extern int  nfs_readpages(struct file *, struct address_space *,
                struct list_head *, unsigned);
-extern void nfs_readpage_result(struct rpc_task *, void *);
-extern void  nfs_readdata_release(void *data);
+extern int  nfs_readpage_result(struct rpc_task *, struct nfs_read_data *);
+extern void nfs_readdata_release(void *data);
 
 
 /*
index 277750c..7fafc4c 100644 (file)
@@ -695,7 +695,6 @@ struct nfs_read_data {
 #ifdef CONFIG_NFS_V4
        unsigned long           timestamp;      /* For lease renewal */
 #endif
-       void (*complete) (struct nfs_read_data *, int);
        struct page             *page_array[NFS_PAGEVEC_SIZE + 1];
 };
 
@@ -768,6 +767,7 @@ struct nfs_rpc_ops {
                             struct nfs_pathconf *);
        u32 *   (*decode_dirent)(u32 *, struct nfs_entry *, int plus);
        void    (*read_setup)   (struct nfs_read_data *);
+       int     (*read_done)  (struct rpc_task *, struct nfs_read_data *);
        void    (*write_setup)  (struct nfs_write_data *, int how);
        int     (*write_done)  (struct rpc_task *, struct nfs_write_data *);
        void    (*commit_setup) (struct nfs_write_data *, int how);