From 3903902401451b1cd9d797a8c79769eb26ac7fe5 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 18 Jul 2023 16:38:08 +1000 Subject: nfsd: don't allow nfsd threads to be signalled. The original implementation of nfsd used signals to stop threads during shutdown. In Linux 2.3.46pre5 nfsd gained the ability to shutdown threads internally it if was asked to run "0" threads. After this user-space transitioned to using "rpc.nfsd 0" to stop nfsd and sending signals to threads was no longer an important part of the API. In commit 3ebdbe5203a8 ("SUNRPC: discard svo_setup and rename svc_set_num_threads_sync()") (v5.17-rc1~75^2~41) we finally removed the use of signals for stopping threads, using kthread_stop() instead. This patch makes the "obvious" next step and removes the ability to signal nfsd threads - or any svc threads. nfsd stops allowing signals and we don't check for their delivery any more. This will allow for some simplification in later patches. A change worth noting is in nfsd4_ssc_setup_dul(). There was previously a signal_pending() check which would only succeed when the thread was being shut down. It should really have tested kthread_should_stop() as well. Now it just does the latter, not the former. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfs/callback.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'fs/nfs') diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index 456af7d230cf..46a0a2d6962e 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c @@ -80,9 +80,6 @@ nfs4_callback_svc(void *vrqstp) set_freezable(); while (!kthread_freezable_should_stop(NULL)) { - - if (signal_pending(current)) - flush_signals(current); /* * Listen for a request on the socket */ @@ -112,11 +109,7 @@ nfs41_callback_svc(void *vrqstp) set_freezable(); while (!kthread_freezable_should_stop(NULL)) { - - if (signal_pending(current)) - flush_signals(current); - - prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE); + prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_IDLE); spin_lock_bh(&serv->sv_cb_lock); if (!list_empty(&serv->sv_cb_list)) { req = list_first_entry(&serv->sv_cb_list, -- cgit v1.2.3 From f78116d3bf4fd7a84451e1a2adc35df7a63fbbf4 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 18 Jul 2023 16:38:08 +1000 Subject: SUNRPC: call svc_process() from svc_recv(). All callers of svc_recv() go on to call svc_process() on success. Simplify callers by having svc_recv() do that for them. This loses one call to validate_process_creds() in nfsd. That was debugging code added 14 years ago. I don't think we need to keep it. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfs/callback.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs/nfs') diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index 46a0a2d6962e..2d94384bd6a9 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c @@ -86,7 +86,6 @@ nfs4_callback_svc(void *vrqstp) err = svc_recv(rqstp, MAX_SCHEDULE_TIMEOUT); if (err == -EAGAIN || err == -EINTR) continue; - svc_process(rqstp); } svc_exit_thread(rqstp); -- cgit v1.2.3 From 7b719e2bf342a59e88b2b6215b98ca4cf824bc58 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 18 Jul 2023 16:38:08 +1000 Subject: SUNRPC: change svc_recv() to return void. svc_recv() currently returns a 0 on success or one of two errors: - -EAGAIN means no message was successfully received - -EINTR means the thread has been told to stop Previously nfsd would stop as the result of a signal as well as following kthread_stop(). In that case the difference was useful: EINTR means stop unconditionally. EAGAIN means stop if kthread_should_stop(), continue otherwise. Now threads only exit when kthread_should_stop() so we don't need the distinction. Signed-off-by: NeilBrown Signed-off-by: Chuck Lever --- fs/nfs/callback.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'fs/nfs') diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index 2d94384bd6a9..54155b484f7b 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c @@ -74,19 +74,12 @@ out_err: static int nfs4_callback_svc(void *vrqstp) { - int err; struct svc_rqst *rqstp = vrqstp; set_freezable(); - while (!kthread_freezable_should_stop(NULL)) { - /* - * Listen for a request on the socket - */ - err = svc_recv(rqstp, MAX_SCHEDULE_TIMEOUT); - if (err == -EAGAIN || err == -EINTR) - continue; - } + while (!kthread_freezable_should_stop(NULL)) + svc_recv(rqstp, MAX_SCHEDULE_TIMEOUT); svc_exit_thread(rqstp); return 0; -- cgit v1.2.3 From c743b4259c3af2c0637c307f08a062d25fa3c99f Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 18 Jul 2023 16:38:08 +1000 Subject: SUNRPC: remove timeout arg from svc_recv() Most svc threads have no interest in a timeout. nfsd sets it to 1 hour, but this is a wart of no significance. lockd uses the timeout so that it can call nlmsvc_retry_blocked(). It also sometimes calls svc_wake_up() to ensure this is called. So change lockd to be consistent and always use svc_wake_up() to trigger nlmsvc_retry_blocked() - using a timer instead of a timeout to svc_recv(). And change svc_recv() to not take a timeout arg. This makes the sp_threads_timedout counter always zero. Signed-off-by: NeilBrown Signed-off-by: Chuck Lever --- fs/nfs/callback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/nfs') diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index 54155b484f7b..39a0ba746267 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c @@ -79,7 +79,7 @@ nfs4_callback_svc(void *vrqstp) set_freezable(); while (!kthread_freezable_should_stop(NULL)) - svc_recv(rqstp, MAX_SCHEDULE_TIMEOUT); + svc_recv(rqstp); svc_exit_thread(rqstp); return 0; -- cgit v1.2.3 From 78c542f916bccafffef4f3bec9bc60d7cda548f5 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 29 Jul 2023 20:58:54 -0400 Subject: SUNRPC: Add enum svc_auth_status In addition to the benefits of using an enum rather than a set of macros, we now have a named type that can improve static type checking of function return values. As part of this change, I removed a stale comment from svcauth.h; the return values from current implementations of the auth_ops::release method are all zero/negative errno, not the SVC_OK enum values as the old comment suggested. Suggested-by: NeilBrown Signed-off-by: Chuck Lever --- fs/nfs/callback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/nfs') diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index 39a0ba746267..466ebf1d41b2 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c @@ -372,7 +372,7 @@ check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp) * All other checking done after NFS decoding where the nfs_client can be * found in nfs4_callback_compound */ -static int nfs_callback_authenticate(struct svc_rqst *rqstp) +static enum svc_auth_status nfs_callback_authenticate(struct svc_rqst *rqstp) { rqstp->rq_auth_stat = rpc_autherr_badcred; -- cgit v1.2.3