Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <79125af67939f5ef8c03c313a346711b768717d9.camel@redhat.com>
Subject: Re: [PATCH/RFC] sunrpc: constant-time code to wake idle thread
From:   Jeff Layton <jlayton@redhat.com>
To:     NeilBrown <neilb@suse.de>, Chuck Lever III <chuck.lever@oracle.com>
Cc:     Chuck Lever <cel@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "lorenzo@kernel.org" <lorenzo@kernel.org>,
        "david@fromorbit.com" <david@fromorbit.com>
Date:   Tue, 11 Jul 2023 06:43:40 -0400
In-Reply-To: <168906970156.8939.2877716074642019289@noble.neil.brown.name>
References: <168842897573.139194.15893960758088950748.stgit@manet.1015granger.net>
        , <168842927591.139194.16920372497489479670.stgit@manet.1015granger.net>
        , <168843152047.8939.5788535164524204707@noble.neil.brown.name>
        , <ZKN6GZ8q9NVLywOJ@manet.1015granger.net>
        , <168894969894.8939.6993305724636717703@noble.neil.brown.name>
        , <ZKwYhbo76v8ElI1b@manet.1015granger.net>
        , <168902749573.8939.3668770103738816387@noble.neil.brown.name>
        , <2211CC3B-806F-461D-A5AA-E95E35E1E408@oracle.com>
         <168906970156.8939.2877716074642019289@noble.neil.brown.name>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2023-07-11 at 20:01 +1000, NeilBrown wrote:
> On Tue, 11 Jul 2023, Chuck Lever III wrote:
> > > On Jul 10, 2023, at 6:18 PM, NeilBrown <neilb@suse.de> wrote:
> > >=20
> > > What do you think of removing the ability to stop an nfsd thread by
> > > sending it a signal.  Note that this doesn't apply to lockd or to nfs=
v4
> > > callback threads.  And nfs-utils never relies on this.
> > > I'm keen.  It would make this patch a lot simpler.
> >=20
> > I agree the code base would be cleaner for it.
> >=20
> > But I'm the new kid. I'm not really sure if this is
> > part of a kernel - user space API that we mustn't
> > alter, or whether it's something that was added but
> > never used, or ....
> >=20
> > I can sniff around to get a better understanding.
>=20
> Once upon a time it was the only way to kill the threads.
> There was a syscall which let you start some threads.  You couldn't
> change the number of threads, but you could kill them.
> And shutdown always kills processes, so letting nfsd threads be killed
> meant that would be removed at system shutdown.
>=20
> When I added the ability to dynamically change the number of threads it
> made sense that we could set the number to zero, and then to use that
> functionality to shut down the nfs server.  So the /etc/init.d/nfsd
> script changed from "killall -9 nfsd" or whatever it was to=20
> "rpc.nfsd 0".
>=20
> But it didn't seem sensible to remove the "kill" functionality until
> after a transition process, and I never thought the schedule a formal
> deprecation.  So it just stayed...
>=20
> The more I think about it, the more I am in favour of removing it.  I
> don't think any other kernel threads can be killed.  nfsd doesn't need
> to be special.
>=20
> Maybe I'll post a patch which just does that.
>=20

I'd be in favor of removing signal handling from the nfsd threads. It's
always been a bit hacky, particularly since we moved everything to the
kthread API around a decade ago.

--=20
Jeff Layton <jlayton@redhat.com>