[pnfs] [PATCH 10/11] pnfs: remove error return from nfs_read_rpcsetup

Fred Isaman iisaman at citi.umich.edu
Thu Apr 10 10:05:10 EDT 2008


This is expected to be an async call, in fact nfs_pagein_multi relies
on this.  Further, if my understanding of the code is correct, "passing
the error up" is pointless, since they are eventually ignored.
do_generic_file_read calls nfs_readpages through the readahead code,
ignoring any error there because it is just readahead.  Later, a cache
miss will cause nfs_readpage to be called, but nfs_readpage_async ignores
any errors and always returns 0

Signed-off-by: Fred Isaman <iisaman at citi.umich.edu>
---
 fs/nfs/internal.h |    2 +-
 fs/nfs/read.c     |   42 ++++++++++++++----------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 07eea8e..00afd05 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -203,7 +203,7 @@ extern int nfs4_path_walk(struct nfs_server *server,
 #endif
 
 /* read.c */
-extern int nfs_initiate_read(struct nfs_read_data *data, struct rpc_clnt *clnt,
+extern void nfs_initiate_read(struct nfs_read_data *data, struct rpc_clnt *clnt,
 			     const struct rpc_call_ops *call_ops);
 extern int nfs_read_validate(struct rpc_task *task, void *calldata);
 extern void nfs_readdata_release(void *data);
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 019abb9..7381951 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -159,7 +159,7 @@ static void nfs_readpage_release(struct nfs_page *req)
 	nfs_release_request(req);
 }
 
-int nfs_initiate_read(struct nfs_read_data *data, struct rpc_clnt *clnt,
+void nfs_initiate_read(struct nfs_read_data *data, struct rpc_clnt *clnt,
 		      const struct rpc_call_ops *call_ops)
 {
 	struct inode *inode = data->inode;
@@ -190,22 +190,19 @@ int nfs_initiate_read(struct nfs_read_data *data, struct rpc_clnt *clnt,
 			(unsigned long long)data->args.offset);
 
 	task = rpc_run_task(&task_setup_data);
-	if (unlikely(IS_ERR(task)))
-		return PTR_ERR(task);
-	rpc_put_task(task);
-	return 0;
+	if (!IS_ERR(task))
+		rpc_put_task(task);
 }
 EXPORT_SYMBOL(nfs_initiate_read);
 
 /*
  * Set up the NFS read request struct
  */
-static int nfs_read_rpcsetup(struct nfs_page *req, struct nfs_read_data *data,
+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 = req->wb_context->path.dentry->d_inode;
-	int ret;
 
 	data->req	  = req;
 	data->inode	  = inode;
@@ -223,11 +220,9 @@ static int nfs_read_rpcsetup(struct nfs_page *req, struct nfs_read_data *data,
 	data->res.eof     = 0;
 	nfs_fattr_init(&data->fattr);
 
-	ret = pnfs_try_to_read_data(data, call_ops);
-	if (ret == 0)
-		return data->pnfs_error;
-
-	return nfs_initiate_read(data, NFS_CLIENT(inode), call_ops);
+	if (pnfs_try_to_read_data(data, call_ops) == 0)
+		return;
+	nfs_initiate_read(data, NFS_CLIENT(inode), call_ops);
 }
 
 static void
@@ -263,7 +258,7 @@ static int nfs_pagein_multi(struct inode *inode, struct list_head *head, unsigne
 	struct nfs_read_data *data;
 	size_t rsize = NFS_SERVER(inode)->rsize, nbytes;
 	unsigned int offset;
-	int requests = 0, status = -ENOMEM;
+	int requests = 0;
 	LIST_HEAD(list);
 
 	nfs_list_remove_request(req);
@@ -294,15 +289,8 @@ static int nfs_pagein_multi(struct inode *inode, struct list_head *head, unsigne
 
 		if (nbytes < rsize)
 			rsize = nbytes;
-		status = nfs_read_rpcsetup(req, data, &nfs_read_partial_ops,
-					   rsize, offset);
-		if (status) {
-			/* FIXME - this is broken. Cleanup code assumes
-			 * nothing  outstanding.  Further, if triggered on
-			 * last list entry, req will be double-freed.
-			 */
-			goto out_bad;
-		}
+		nfs_read_rpcsetup(req, data, &nfs_read_partial_ops,
+				  rsize, offset);
 		offset += rsize;
 		nbytes -= rsize;
 	} while (nbytes != 0);
@@ -317,7 +305,7 @@ out_bad:
 	}
 	SetPageError(page);
 	nfs_readpage_release(req);
-	return status;
+	return -ENOMEM;
 }
 
 static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned int npages, size_t count, int flags)
@@ -325,7 +313,6 @@ static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned
 	struct nfs_page		*req;
 	struct page		**pages;
 	struct nfs_read_data	*data;
-	int			status = -ENOMEM;
 
 	data = nfs_readdata_alloc(npages);
 	if (!data)
@@ -343,12 +330,11 @@ static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned
 	}
 	req = nfs_list_entry(data->pages.next);
 
-	status = nfs_read_rpcsetup(req, data, &nfs_read_full_ops, count, 0);
-	if (!status)
-		return 0;
+	nfs_read_rpcsetup(req, data, &nfs_read_full_ops, count, 0);
+	return 0;
 out_bad:
 	nfs_async_read_error(head);
-	return status;
+	return -ENOMEM;
 }
 
 /*
-- 
1.5.3.3



More information about the pNFS mailing list