Skip to contentSkip to conversation

sunrpc: don't check for failure from mempool_alloc()

PATCH New NeilBrown 1 participant 2 messages Apr 10, 2017 – Apr 24, 2017

Subsystems (3)
Networking NFS client NFS server
lore
Share & tools
lore

NeilBrown

author
#0 lore raw

PATCH 1/1 sunrpc: don't check for failure from mempool_alloc()

When mempool_alloc() is allowed to sleep (GFP_NOIO allows sleeping) it cannot fail.
So rpc_alloc_task() cannot fail, so rpc_new_task doesn't need to test for failure.
Consequently rpc_new_task() cannot fail, so the callers don't need to test.

Signed-off-by NeilBrown <[email protected]>
---
 net/sunrpc/clnt.c  | 8 --------
 net/sunrpc/sched.c | 5 -----
 2 files changed, 13 deletions(-)
net/sunrpc/clnt.c +0 -8
index 52da3ce54bb5..b5cb921775a0 100644
@@ -1042,8 +1042,6 @@ struct rpc_task *rpc_run_task(const struct rpc_task_setup *task_setup_data)
 	struct rpc_task *task;
 
 	task = rpc_new_task(task_setup_data);
-	if (IS_ERR(task))
-		goto out;
 
 	rpc_task_set_client(task, task_setup_data->rpc_client);
 	rpc_task_set_rpc_message(task, task_setup_data->rpc_message);

@@ -1053,7 +1051,6 @@ struct rpc_task *rpc_run_task(const struct rpc_task_setup *task_setup_data)
 
 	atomic_inc(&task->tk_count);
 	rpc_execute(task);
-out:
 	return task;
 }
 EXPORT_SYMBOL_GPL(rpc_run_task);

@@ -1140,10 +1137,6 @@ struct rpc_task *rpc_run_bc_task(struct rpc_rqst *req)
 	 * Create an rpc_task to send the data
 	 */
 	task = rpc_new_task(&task_setup_data);
-	if (IS_ERR(task)) {
-		xprt_free_bc_request(req);
-		goto out;
-	}
 	task->tk_rqstp = req;
 
 	/*

@@ -1158,7 +1151,6 @@ struct rpc_task *rpc_run_bc_task(struct rpc_rqst *req)
 	WARN_ON_ONCE(atomic_read(&task->tk_count) != 2);
 	rpc_execute(task);
 
-out:
 	dprintk("RPC: rpc_run_bc_task: task= %p\n", task);
 	return task;
 }
net/sunrpc/sched.c +0 -5
index 5db68b371db2..0cc83839c13c 100644
@@ -965,11 +965,6 @@ struct rpc_task *rpc_new_task(const struct rpc_task_setup *setup_data)
 
 	if (task == NULL) {
 		task = rpc_alloc_task();
-		if (task == NULL) {
-			rpc_release_calldata(setup_data->callback_ops,
-					setup_data->callback_data);
-			return ERR_PTR(-ENOMEM);
-		}
 		flags = RPC_TASK_DYNAMIC;
 	}
Signature · 1 line
2.12.2
author
#1 lore raw

PATCH 1/1 NFS: remove error handling for callers of rpc_new_task()

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 <[email protected]>
Signed-off-by NeilBrown <[email protected]>

---

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.

Thanks,
NeilBrown

 fs/lockd/clntproc.c            |  4 ---
 fs/nfs/nfs42proc.c             |  2 --
 fs/nfs/nfs4proc.c              | 66 ++++++------------------------------------
 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, 11 insertions(+), 102 deletions(-)
fs/lockd/clntproc.c +0 -4
index 112952037933..6f472f4161b8 100644
@@ -357,8 +357,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;
 }

@@ -402,8 +400,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;
fs/nfs/nfs42proc.c +0 -2
index c81c61971625..7f08c972a138 100644
@@ -453,8 +453,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;
 }
fs/nfs/nfs4proc.c +9 -57

186 lines not on this page · raw message

fs/nfs/pagelist.c +0 -5
index f53610672f03..ec94f36d650b 100644
@@ -599,17 +599,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);
fs/nfs/unlink.c +1 -8
index 191aa577dd1f..fcdc4b289962 100644
@@ -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)

@@ -484,12 +483,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)
fs/nfs/write.c +0 -2
index bbb289591e02..1572e9b94173 100644
@@ -1622,8 +1622,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);
net/sunrpc/auth_gss/auth_gss.c +1 -2
index 4f16953e4954..8b0718b39930 100644
@@ -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;
net/sunrpc/clnt.c +0 -10
index b5cb921775a0..66fbf1d0c7fa 100644
@@ -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;
 }

@@ -2582,8 +2578,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;
 }

@@ -2629,10 +2623,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);
net/sunrpc/rpcb_clnt.c +0 -6
index 5b30603596d0..5e30f1b8e4d9 100644
@@ -781,12 +781,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);
net/sunrpc/svc.c +0 -6
index a08aeb56b8e4..58b823d7b68c 100644
@@ -1449,16 +1449,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;
 }
Signature · 1 line
2.12.2