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: <b6dd961f8e4bb43a4a14876ace227d1e3f924463.camel@redhat.com>
Subject: Re: [PATCH v3 5/9] SUNRPC: Count pool threads that were awoken but
 found no work to do
From:   Jeff Layton <jlayton@redhat.com>
To:     NeilBrown <neilb@suse.de>
Cc:     Chuck Lever III <chuck.lever@oracle.com>,
        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 10:07:50 -0400
In-Reply-To: <168907783478.8939.2524484078705916909@noble.neil.brown.name>
References: <168900729243.7514.15141312295052254929.stgit@manet.1015granger.net>
        , <168900734678.7514.887270657845753276.stgit@manet.1015granger.net>
        , <168902815363.8939.4838920400288577480@noble.neil.brown.name>
        , <D20E4286-30D2-461D-B856-41D3C53C756C@oracle.com>
        , <168906929382.8939.6551236368797730920@noble.neil.brown.name>
        , <18061cab9338b08da691e8651e75f8fe8e88b830.camel@redhat.com>
         <168907783478.8939.2524484078705916909@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 22:17 +1000, NeilBrown wrote:
> On Tue, 11 Jul 2023, Jeff Layton wrote:
> > On Tue, 2023-07-11 at 19:54 +1000, NeilBrown wrote:
> > > On Tue, 11 Jul 2023, Chuck Lever III wrote:
> > > >=20
> > > > > On Jul 10, 2023, at 6:29 PM, NeilBrown <neilb@suse.de> wrote:
> > > > >=20
> > > > > On Tue, 11 Jul 2023, Chuck Lever wrote:
> > > > > > From: Chuck Lever <chuck.lever@oracle.com>
> > > > > >=20
> > > > > > Measure a source of thread scheduling inefficiency -- count thr=
eads
> > > > > > that were awoken but found that the transport queue had already=
 been
> > > > > > emptied.
> > > > > >=20
> > > > > > An empty transport queue is possible when threads that run betw=
een
> > > > > > the wake_up_process() call and the woken thread returning from =
the
> > > > > > scheduler have pulled all remaining work off the transport queu=
e
> > > > > > using the first svc_xprt_dequeue() in svc_get_next_xprt().
> > > > >=20
> > > > > I'm in two minds about this.  The data being gathered here is
> > > > > potentially useful
> > > >=20
> > > > It's actually pretty shocking: I've measured more than
> > > > 15% of thread wake-ups find no work to do.
> > >=20
> > > That is a bigger number than I would have guessed!
> > >=20
> >=20
> > I'm guessing the idea is that the receiver is waking a thread to do the
> > work, and that races with one that's already running? I'm sure there ar=
e
> > ways we can fix that, but it really does seem like we're trying to
> > reinvent workqueues here.
>=20
> True.  But then workqueues seem to reinvent themselves every so often
> too.  Once gets the impression they are trying to meet an enormous
> variety of needs.
> I'm not against trying to see if nfsd could work well in a workqueue
> environment, but I'm not certain it is a good idea.  Maintaining control
> of our own thread pools might be safer.
>=20
> I have a vague memory of looking into this in more detail once and
> deciding that it wasn't a good fit, but I cannot recall or easily deduce
> the reason.  Obviously we would have to give up SIGKILL, but we want to
> do that anyway.
>=20
> Would we want unbound work queues - so they can be scheduled across
> different CPUs?  Are NFSD threads cpu-intensive or not?  I'm not sure.
>=20
> I would be happy to explore a credible attempt at a conversion.
>=20

I had some patches several years ago that did a conversion from nfsd
threads to workqueues.=A0

It worked reasonably well, but under heavy loads it didn't perform as
well as having a dedicated threadpool...which is not too surprising,
really. nfsd has been tuned for performance over years and it's fairly
"greedy" about squatting on system resources even when it's idle.

If we wanted to look again at doing this with workqueues, we'd need the
workqueue infrastructure to allow for long-running jobs that may block
for a long time. That means it might need to be more cavalier about
spinning up new workqueue threads and keeping them running when there
are a lot of concurrent, but sleeping workqueue jobs.

We probably would want unbound workqueues so we can have more jobs in
flight at any given time than cpus, given that a lot of them will end up
being blocked in some way or another.

CPU utilization is a good question. Mostly we just call down into the
filesystem to do things, and the encoding and decoding is not _that_ cpu
intensive. For most storage stacks, I suspect we don't use a lot of CPU
aside from normal copying of data.  There might be some exceptions
though, like when the underlying storage is using encryption, etc.

The main upside to switching to workqueues is that it would allow us to
get rid of a lot of fiddly, hand-tuned thread handling code. It might
also make it simpler to convert to a more asynchronous model.

For instance, it would be nice to be able to not have to put a thread to
sleep while waiting on writeback for a COMMIT. I think that'd be easier
to handle with a workqueue-like model.
--=20
Jeff Layton <jlayton@redhat.com>