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: <18061cab9338b08da691e8651e75f8fe8e88b830.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>, 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 07:42:49 -0400
In-Reply-To: <168906929382.8939.6551236368797730920@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>
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 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 threads
> > > > that were awoken but found that the transport queue had already bee=
n
> > > > emptied.
> > > >=20
> > > > An empty transport queue is possible when threads that run between
> > > > the wake_up_process() call and the woken thread returning from the
> > > > scheduler have pulled all remaining work off the transport queue
> > > > 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

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 are
ways we can fix that, but it really does seem like we're trying to
reinvent workqueues here.

> >=20
> >=20
> > > - but who it is useful to?
> > > I think it is primarily useful for us - to understand the behaviour o=
f
> > > the implementation so we can know what needs to be optimised.
> > > It isn't really of any use to a sysadmin who wants to understand how
> > > their system is performing.
> > >=20
> > > But then .. who are tracepoints for?  Developers or admins?
> > > I guess that fact that we feel free to modify them whenever we need
> > > means they are primarily for developers?  In which case this is a goo=
d
> > > patch, and maybe we'll revert the functionality one day if it turns o=
ut
> > > that we can change the implementation so that a thread is never woken
> > > when there is no work to do ....
> >=20
> > A reasonable question to ask. The new "starved" metric
> > is similar: possibly useful while we are developing the
> > code, but not particularly valuable for system
> > administrators.
> >=20
> > How are the pool_stats used by administrators?
>=20
> That is a fair question.  Probably not much.
> Once upon a time we had stats which could show a histogram how thread
> usage.  I used that to decided if the load justified more threads.
> But we removed it because it was somewhat expensive and it was argued it
> may not be all that useful...
> I haven't really looked at any other stats in my work.  Almost always it
> is a packet capture that helps me see what is happening when I have an
> issue to address.
>=20
> Maybe I should just accept that stats are primarily for developers and
> they can be incredible useful for that purpose, and not worry if admins
> might ever need them.
>=20
> >=20
> > (And, why are they in /proc/fs/nfsd/ rather than under
> > something RPC-related?)
>=20
> Maybe because we "owned" /proc/fs/nfsd/, but the RPC-related stuff is
> under "net" and we didn't feel so comfortable sticking new stuff there.
> Or maybe not.
>=20

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