Linux NFS development
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Trond Myklebust <trond.myklebust@primarydata.com>,
	Anna Schumaker <anna.schumaker@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH 7/8] NFS: remove error handling for callers of rpc_new_task()
Date: Fri, 18 Aug 2017 17:12:52 +1000	[thread overview]
Message-ID: <150304037206.30218.6010632417893362395.stgit@noble> (raw)
In-Reply-To: <150304014011.30218.1636255532744321171.stgit@noble>

Since commit 62b2417e84ba ("sunrpc: don't check for failure from mempool_alloc()")

rpc_new_task() cannot fail, so functions which fail only when it failed
also cannot fail.
This means we no longer need to check for errors from:

  rpc_run_task()
  rpc_run_bc_task()
  __nlm_async_call()
  nfs4_do_unlck()
  _nfs41_proc_sequence()
  _nfs41_free_stateid()
  nfs_async_rename()
  rpc_call_null()
  rpc_call_null_helper()
  rpcb_call_async()

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: NeilBrown <neilb@suse.com>


 Hi,
  if you think there might ever be a need to allow
  rpc_new_task() to fail in the future, we might not
  want to take this approach.


Signed-off-by: NeilBrown <neilb@suse.com>
---
 fs/lockd/clntproc.c            |    4 ---
 fs/nfs/nfs42proc.c             |    2 -
 fs/nfs/nfs4proc.c              |   62 ++++------------------------------------
 fs/nfs/pagelist.c              |    5 ---
 fs/nfs/unlink.c                |    9 +-----
 fs/nfs/write.c                 |    2 -
 net/sunrpc/auth_gss/auth_gss.c |    3 +-
 net/sunrpc/clnt.c              |   10 ------
 net/sunrpc/rpcb_clnt.c         |    6 ----
 net/sunrpc/svc.c               |    6 ----
 10 files changed, 8 insertions(+), 101 deletions(-)

diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c
index 066ac313ae5c..fefed4a8d9c8 100644
--- a/fs/lockd/clntproc.c
+++ b/fs/lockd/clntproc.c
@@ -367,8 +367,6 @@ static int nlm_do_async_call(struct nlm_rqst *req, u32 proc, struct rpc_message
 	struct rpc_task *task;
 
 	task = __nlm_async_call(req, proc, msg, tk_ops);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	rpc_put_task(task);
 	return 0;
 }
@@ -412,8 +410,6 @@ static int nlmclnt_async_call(struct rpc_cred *cred, struct nlm_rqst *req, u32 p
 	int err;
 
 	task = __nlm_async_call(req, proc, &msg, tk_ops);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	err = rpc_wait_for_completion_task(task);
 	rpc_put_task(task);
 	return err;
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 6c2db51e67a7..0062a73973ee 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -463,8 +463,6 @@ int nfs42_proc_layoutstats_generic(struct nfs_server *server,
 	}
 	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 0);
 	task = rpc_run_task(&task_setup);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	rpc_put_task(task);
 	return 0;
 }
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index d90132642340..df4a8724e85d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -977,12 +977,8 @@ static int nfs4_call_sync_sequence(struct rpc_clnt *clnt,
 	};
 
 	task = rpc_run_task(&task_setup);
-	if (IS_ERR(task))
-		ret = PTR_ERR(task);
-	else {
-		ret = task->tk_status;
-		rpc_put_task(task);
-	}
+	ret = task->tk_status;
+	rpc_put_task(task);
 	return ret;
 }
 
@@ -2024,8 +2020,6 @@ static int _nfs4_proc_open_confirm(struct nfs4_opendata *data)
 	if (data->is_recover)
 		nfs4_set_sequence_privileged(&data->c_arg.seq_args);
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	status = rpc_wait_for_completion_task(task);
 	if (status != 0) {
 		data->cancelled = true;
@@ -2191,8 +2185,6 @@ static int nfs4_run_open_task(struct nfs4_opendata *data, int isrecover)
 		data->is_recover = true;
 	}
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	status = rpc_wait_for_completion_task(task);
 	if (status != 0) {
 		data->cancelled = true;
@@ -3213,8 +3205,6 @@ int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait)
 	msg.rpc_resp = &calldata->res;
 	task_setup_data.callback_data = calldata;
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	status = 0;
 	if (wait)
 		status = rpc_wait_for_completion_task(task);
@@ -5536,10 +5526,6 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
 		clp->cl_owner_id);
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task)) {
-		status = PTR_ERR(task);
-		goto out;
-	}
 	status = task->tk_status;
 	if (setclientid.sc_cred) {
 		clp->cl_acceptor = rpcauth_stringify_acceptor(setclientid.sc_cred);
@@ -5755,8 +5741,6 @@ static int _nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, co
 	msg.rpc_argp = &data->args;
 	msg.rpc_resp = &data->res;
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	if (!issync)
 		goto out;
 	status = rpc_wait_for_completion_task(task);
@@ -6035,9 +6019,6 @@ static int nfs4_proc_unlck(struct nfs4_state *state, int cmd, struct file_lock *
 	if (IS_ERR(seqid))
 		goto out;
 	task = nfs4_do_unlck(request, nfs_file_open_context(request->fl_file), lsp, seqid);
-	status = PTR_ERR(task);
-	if (IS_ERR(task))
-		goto out;
 	status = rpc_wait_for_completion_task(task);
 	rpc_put_task(task);
 out:
@@ -6193,8 +6174,7 @@ static void nfs4_lock_release(void *calldata)
 		struct rpc_task *task;
 		task = nfs4_do_unlck(&data->fl, data->ctx, data->lsp,
 				data->arg.lock_seqid);
-		if (!IS_ERR(task))
-			rpc_put_task_async(task);
+		rpc_put_task_async(task);
 		dprintk("%s: cancelling lock!\n", __func__);
 	} else
 		nfs_free_seqid(data->arg.lock_seqid);
@@ -6263,8 +6243,6 @@ static int _nfs4_do_setlk(struct nfs4_state *state, int cmd, struct file_lock *f
 	} else
 		data->arg.new_lock = 1;
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	ret = rpc_wait_for_completion_task(task);
 	if (ret == 0) {
 		ret = data->rpc_status;
@@ -7222,11 +7200,8 @@ int nfs4_proc_bind_one_conn_to_session(struct rpc_clnt *clnt,
 		args.dir = NFS4_CDFC4_FORE;
 
 	task = rpc_run_task(&task_setup_data);
-	if (!IS_ERR(task)) {
-		status = task->tk_status;
-		rpc_put_task(task);
-	} else
-		status = PTR_ERR(task);
+	status = task->tk_status;
+	rpc_put_task(task);
 	trace_nfs4_bind_conn_to_session(clp, status);
 	if (status == 0) {
 		if (memcmp(res.sessionid.data,
@@ -7573,8 +7548,6 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	task_setup_data.callback_data = calldata;
 
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 
 	status = calldata->rpc_status;
 
@@ -7799,9 +7772,6 @@ int nfs4_proc_get_lease_time(struct nfs_client *clp, struct nfs_fsinfo *fsinfo)
 	nfs4_set_sequence_privileged(&args.la_seq_args);
 	task = rpc_run_task(&task_setup);
 
-	if (IS_ERR(task))
-		return PTR_ERR(task);
-
 	status = task->tk_status;
 	rpc_put_task(task);
 	return status;
@@ -8149,10 +8119,7 @@ static int nfs41_proc_async_sequence(struct nfs_client *clp, struct rpc_cred *cr
 	if ((renew_flags & NFS4_RENEW_TIMEOUT) == 0)
 		return -EAGAIN;
 	task = _nfs41_proc_sequence(clp, cred, false);
-	if (IS_ERR(task))
-		ret = PTR_ERR(task);
-	else
-		rpc_put_task_async(task);
+	rpc_put_task_async(task);
 	dprintk("<-- %s status=%d\n", __func__, ret);
 	return ret;
 }
@@ -8163,15 +8130,10 @@ static int nfs4_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred)
 	int ret;
 
 	task = _nfs41_proc_sequence(clp, cred, true);
-	if (IS_ERR(task)) {
-		ret = PTR_ERR(task);
-		goto out;
-	}
 	ret = rpc_wait_for_completion_task(task);
 	if (!ret)
 		ret = task->tk_status;
 	rpc_put_task(task);
-out:
 	dprintk("<-- %s status=%d\n", __func__, ret);
 	return ret;
 }
@@ -8280,10 +8242,6 @@ static int nfs41_proc_reclaim_complete(struct nfs_client *clp,
 	msg.rpc_resp = &calldata->res;
 	task_setup_data.callback_data = calldata;
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task)) {
-		status = PTR_ERR(task);
-		goto out;
-	}
 	status = rpc_wait_for_completion_task(task);
 	if (status == 0)
 		status = task->tk_status;
@@ -8515,8 +8473,6 @@ nfs4_proc_layoutget(struct nfs4_layoutget *lgp, long *timeout, gfp_t gfp_flags)
 	nfs4_init_sequence(&lgp->args.seq_args, &lgp->res.seq_res, 0);
 
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return ERR_CAST(task);
 	status = rpc_wait_for_completion_task(task);
 	if (status == 0) {
 		status = nfs4_layoutget_handle_exception(task, lgp, &exception);
@@ -8632,8 +8588,6 @@ int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp, bool sync)
 	}
 	nfs4_init_sequence(&lrp->args.seq_args, &lrp->res.seq_res, 1);
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	if (sync)
 		status = task->tk_status;
 	trace_nfs4_layoutreturn(lrp->args.inode, &lrp->args.stateid, status);
@@ -8779,8 +8733,6 @@ nfs4_proc_layoutcommit(struct nfs4_layoutcommit_data *data, bool sync)
 	}
 	nfs4_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	if (sync)
 		status = task->tk_status;
 	trace_nfs4_layoutcommit(data->args.inode, &data->args.stateid, status);
@@ -9106,8 +9058,6 @@ static int nfs41_free_stateid(struct nfs_server *server,
 	struct rpc_task *task;
 
 	task = _nfs41_free_stateid(server, stateid, cred, is_recovery);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	rpc_put_task(task);
 	return 0;
 }
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index de9066a92c0d..12c1465c1472 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -646,17 +646,12 @@ int nfs_initiate_pgio(struct rpc_clnt *clnt, struct nfs_pgio_header *hdr,
 		(unsigned long long)hdr->args.offset);
 
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task)) {
-		ret = PTR_ERR(task);
-		goto out;
-	}
 	if (how & FLUSH_SYNC) {
 		ret = rpc_wait_for_completion_task(task);
 		if (ret == 0)
 			ret = task->tk_status;
 	}
 	rpc_put_task(task);
-out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(nfs_initiate_pgio);
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index e3949d93085c..3cb88c073fde 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -108,8 +108,7 @@ static void nfs_do_call_unlink(struct nfs_unlinkdata *data)
 
 	task_setup_data.rpc_client = NFS_CLIENT(dir);
 	task = rpc_run_task(&task_setup_data);
-	if (!IS_ERR(task))
-		rpc_put_task_async(task);
+	rpc_put_task_async(task);
 }
 
 static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata *data)
@@ -497,12 +496,6 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry)
 	/* run the rename task, undo unlink if it fails */
 	task = nfs_async_rename(dir, dir, dentry, sdentry,
 					nfs_complete_sillyrename);
-	if (IS_ERR(task)) {
-		error = -EBUSY;
-		nfs_cancel_async_unlink(dentry);
-		goto out_dput;
-	}
-
 	/* wait for the RPC task to complete, unless a SIGKILL intervenes */
 	error = rpc_wait_for_completion_task(task);
 	if (error == 0)
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index e833179d0d49..c2924d3f172a 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1691,8 +1691,6 @@ int nfs_initiate_commit(struct rpc_clnt *clnt, struct nfs_commit_data *data,
 		NFS_SP4_MACH_CRED_COMMIT, &task_setup_data.rpc_client, &msg);
 
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	if (how & FLUSH_SYNC)
 		rpc_wait_for_completion_task(task);
 	rpc_put_task(task);
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 9463af4b32e8..bfcf5e96483c 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -1232,8 +1232,7 @@ gss_destroying_context(struct rpc_cred *cred)
 	get_rpccred(cred);
 
 	task = rpc_call_null(gss_auth->client, cred, RPC_TASK_ASYNC|RPC_TASK_SOFT);
-	if (!IS_ERR(task))
-		rpc_put_task(task);
+	rpc_put_task(task);
 
 	put_rpccred(cred);
 	return 1;
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 2ad827db2704..e1d96f52e97f 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1080,8 +1080,6 @@ int rpc_call_sync(struct rpc_clnt *clnt, const struct rpc_message *msg, int flag
 	}
 
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	status = task->tk_status;
 	rpc_put_task(task);
 	return status;
@@ -1110,8 +1108,6 @@ rpc_call_async(struct rpc_clnt *clnt, const struct rpc_message *msg, int flags,
 	};
 
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	rpc_put_task(task);
 	return 0;
 }
@@ -2590,8 +2586,6 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
 			RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC,
 			&rpc_cb_add_xprt_call_ops, data);
 	put_rpccred(cred);
-	if (IS_ERR(task))
-		return PTR_ERR(task);
 	rpc_put_task(task);
 	return 1;
 }
@@ -2637,10 +2631,6 @@ int rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *clnt,
 				    RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
 				    NULL, NULL);
 	put_rpccred(cred);
-	if (IS_ERR(task)) {
-		status = PTR_ERR(task);
-		goto out_err;
-	}
 	status = task->tk_status;
 	rpc_put_task(task);
 
diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index ea0676f199c8..57a955583360 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -782,12 +782,6 @@ void rpcb_getport_async(struct rpc_task *task)
 
 	child = rpcb_call_async(rpcb_clnt, map, proc);
 	rpc_release_client(rpcb_clnt);
-	if (IS_ERR(child)) {
-		/* rpcb_map_release() has freed the arguments */
-		dprintk("RPC: %5u %s: rpc_run_task failed\n",
-			task->tk_pid, __func__);
-		return;
-	}
 
 	xprt->stat.bind_count++;
 	rpc_put_task(child);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 85ce0db5b0a6..10c2182f70aa 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1508,16 +1508,10 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 	/* Finally, send the reply synchronously */
 	memcpy(&req->rq_snd_buf, &rqstp->rq_res, sizeof(req->rq_snd_buf));
 	task = rpc_run_bc_task(req);
-	if (IS_ERR(task)) {
-		error = PTR_ERR(task);
-		goto out;
-	}
-
 	WARN_ON_ONCE(atomic_read(&task->tk_count) != 1);
 	error = task->tk_status;
 	rpc_put_task(task);
 
-out:
 	dprintk("svc: %s(), error=%d\n", __func__, error);
 	return error;
 }



  parent reply	other threads:[~2017-08-18  7:14 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-18  7:12 [PATCH 0/8] Assorted NFS patches NeilBrown
2017-08-18  7:12 ` [PATCH 3/8] NFS: don't expect errors from mempool_alloc() NeilBrown
2017-08-18  7:12 ` [PATCH 2/8] NFSv4: don't let hanging mounts block other mounts NeilBrown
2017-08-18 19:05   ` Trond Myklebust
2017-08-19  0:50     ` NeilBrown
2017-08-18  7:12 ` [PATCH 1/8] SUNRPC: ECONNREFUSED should cause a rebind NeilBrown
2017-08-18 14:40   ` Chuck Lever
2017-08-18  7:12 ` [PATCH 4/8] NFS: flush out dirty data on file fput() NeilBrown
2017-08-18 19:15   ` Trond Myklebust
2017-08-19  1:02     ` NeilBrown
2017-08-19  1:27       ` Trond Myklebust
2017-08-21  1:35         ` NeilBrown
2017-08-23 11:12           ` Jeff Layton
2017-08-18  7:12 ` NeilBrown [this message]
2017-08-18  7:12 ` [PATCH 5/8] SUNRPC: remove some dead code NeilBrown
2017-08-18  7:12 ` [PATCH 8/8] NFSv4.1: don't use machine credentials for CLOSE when using 'sec=sys' NeilBrown
2017-08-18  7:12 ` [PATCH 6/8] NFS: flush data when locking a file to ensure cache coherence for mmap NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=150304037206.30218.6010632417893362395.stgit@noble \
    --to=neilb@suse.com \
    --cc=anna.schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox