Re: [PATCH 5.15] nfsd: don't allow nfsd threads to be signalled.

Chuck Lever III
2024-05-18T14:50:41Z

> On May 18, 2024, at 12:19 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>
> On Fri, May 17, 2024 at 01:59:30PM -0400, cel@kernel.org wrote:
>> From: NeilBrown <neilb@suse.de>
>>
>> [ Upstream commit 3903902401451b1cd9d797a8c79769eb26ac7fe5 ]
>>
>> 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 <neilb@suse.de>
>> Reviewed-by: Jeff Layton <jlayton@kernel.org>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> fs/nfs/callback.c | 9 +--------
>> fs/nfsd/nfs4proc.c | 5 ++---
>> fs/nfsd/nfssvc.c | 12 ------------
>> net/sunrpc/svc_xprt.c | 16 ++++++----------
>> 4 files changed, 9 insertions(+), 33 deletions(-)
>>
>> Greg, Sasha - This is the third resend for this fix. Why isn't it
>> applied to origin/linux-5.15.y yet?
>
> I only see one previous send, where is the second?
>
> Anyway, we are working to catch up, there's been a few hundred other
> commits that were also needed :)

That's what I thought at first, but it's been a while! I will leave
you to it.

--
Chuck Lever