[RFC,v4,1/9] SUNRPC: Add flag to kill new tasks
diff mbox

Message ID 20171117174552.18722-2-JPEWhacker@gmail.com
State New
Headers show

Commit Message

Joshua Watt Nov. 17, 2017, 5:45 p.m. UTC
The flag causes any new tasks that are queued to exit immediately with
-EIO instead of executing. This will allow clients (particularly NFS) to
prevents these task from delaying shutdown of the RPC session longer
than necessary.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 include/linux/sunrpc/clnt.h | 1 +
 net/sunrpc/clnt.c           | 5 ++---
 net/sunrpc/sched.c          | 3 +++
 3 files changed, 6 insertions(+), 3 deletions(-)

Comments

NeilBrown Dec. 5, 2017, 10:59 p.m. UTC | #1
On Fri, Nov 17 2017, Joshua Watt wrote:

> The flag causes any new tasks that are queued to exit immediately with
> -EIO instead of executing. This will allow clients (particularly NFS) to
> prevents these task from delaying shutdown of the RPC session longer
> than necessary.
>
> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> ---
>  include/linux/sunrpc/clnt.h | 1 +
>  net/sunrpc/clnt.c           | 5 ++---
>  net/sunrpc/sched.c          | 3 +++
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index 71c237e8240e..d36dc529fdf0 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -54,6 +54,7 @@ struct rpc_clnt {
>  				cl_noretranstimeo: 1,/* No retransmit timeouts */
>  				cl_autobind : 1,/* use getport() */
>  				cl_chatty   : 1;/* be verbose */
> +	atomic_t		cl_kill_new_tasks;	/* Kill all new tasks */

It looks a little weird that this is an atomic_t.
Looking further ahead, it seems that it is a counter of threads that
have request new tasks be killed.
It might help to make that clear, at least in a comment.

>  
>  	struct rpc_rtt *	cl_rtt;		/* RTO estimator data */
>  	const struct rpc_timeout *cl_timeout;	/* Timeout strategy */
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index df4ecb042ebe..d5eedabf0b4d 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -626,6 +626,8 @@ static struct rpc_clnt *__rpc_clone_client(struct rpc_create_args *args,
>  	new->cl_noretranstimeo = clnt->cl_noretranstimeo;
>  	new->cl_discrtry = clnt->cl_discrtry;
>  	new->cl_chatty = clnt->cl_chatty;
> +	atomic_set(&new->cl_kill_new_tasks,
> +		   atomic_read(&clnt->cl_kill_new_tasks));

However ... as it is a counter, it isn't clear that copying the counter
makes sense, and the new counter will never get decremented.
Maybe __rpc_clone_client() should fail if ->cl_kill_new_tasks > 0 ??

Thanks,
NeilBrown


>  	return new;
>  
>  out_err:
> @@ -818,9 +820,6 @@ void rpc_killall_tasks(struct rpc_clnt *clnt)
>  {
>  	struct rpc_task	*rovr;
>  
> -
> -	if (list_empty(&clnt->cl_tasks))
> -		return;
>  	dprintk("RPC:       killing all tasks for client %p\n", clnt);
>  	/*
>  	 * Spin lock all_tasks to prevent changes...
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index 0cc83839c13c..e70034e19ccd 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -748,6 +748,9 @@ static void __rpc_execute(struct rpc_task *task)
>  	dprintk("RPC: %5u __rpc_execute flags=0x%x\n",
>  			task->tk_pid, task->tk_flags);
>  
> +	if (atomic_read(&task->tk_client->cl_kill_new_tasks))
> +		rpc_exit(task, -EIO);
> +
>  	WARN_ON_ONCE(RPC_IS_QUEUED(task));
>  	if (RPC_IS_QUEUED(task))
>  		return;
> -- 
> 2.13.6

Patch
diff mbox

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 71c237e8240e..d36dc529fdf0 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -54,6 +54,7 @@  struct rpc_clnt {
 				cl_noretranstimeo: 1,/* No retransmit timeouts */
 				cl_autobind : 1,/* use getport() */
 				cl_chatty   : 1;/* be verbose */
+	atomic_t		cl_kill_new_tasks;	/* Kill all new tasks */
 
 	struct rpc_rtt *	cl_rtt;		/* RTO estimator data */
 	const struct rpc_timeout *cl_timeout;	/* Timeout strategy */
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index df4ecb042ebe..d5eedabf0b4d 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -626,6 +626,8 @@  static struct rpc_clnt *__rpc_clone_client(struct rpc_create_args *args,
 	new->cl_noretranstimeo = clnt->cl_noretranstimeo;
 	new->cl_discrtry = clnt->cl_discrtry;
 	new->cl_chatty = clnt->cl_chatty;
+	atomic_set(&new->cl_kill_new_tasks,
+		   atomic_read(&clnt->cl_kill_new_tasks));
 	return new;
 
 out_err:
@@ -818,9 +820,6 @@  void rpc_killall_tasks(struct rpc_clnt *clnt)
 {
 	struct rpc_task	*rovr;
 
-
-	if (list_empty(&clnt->cl_tasks))
-		return;
 	dprintk("RPC:       killing all tasks for client %p\n", clnt);
 	/*
 	 * Spin lock all_tasks to prevent changes...
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 0cc83839c13c..e70034e19ccd 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -748,6 +748,9 @@  static void __rpc_execute(struct rpc_task *task)
 	dprintk("RPC: %5u __rpc_execute flags=0x%x\n",
 			task->tk_pid, task->tk_flags);
 
+	if (atomic_read(&task->tk_client->cl_kill_new_tasks))
+		rpc_exit(task, -EIO);
+
 	WARN_ON_ONCE(RPC_IS_QUEUED(task));
 	if (RPC_IS_QUEUED(task))
 		return;