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;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "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>,
        "Jeff Layton" <jlayton@redhat.com>,
        "david@fromorbit.com" <david@fromorbit.com>
Subject: Re: [PATCH v3 5/9] SUNRPC: Count pool threads that were awoken but
 found no work to do
In-reply-to: <D20E4286-30D2-461D-B856-41D3C53C756C@oracle.com>
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>
Date:   Tue, 11 Jul 2023 19:54:53 +1000
Message-id: <168906929382.8939.6551236368797730920@noble.neil.brown.name>
Precedence: bulk

On Tue, 11 Jul 2023, Chuck Lever III wrote:
> 
> > On Jul 10, 2023, at 6:29 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > On Tue, 11 Jul 2023, Chuck Lever wrote:
> >> From: Chuck Lever <chuck.lever@oracle.com>
> >> 
> >> Measure a source of thread scheduling inefficiency -- count threads
> >> that were awoken but found that the transport queue had already been
> >> emptied.
> >> 
> >> 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().
> > 
> > I'm in two minds about this.  The data being gathered here is
> > potentially useful
> 
> It's actually pretty shocking: I've measured more than
> 15% of thread wake-ups find no work to do.

That is a bigger number than I would have guessed!

> 
> 
> > - but who it is useful to?
> > I think it is primarily useful for us - to understand the behaviour of
> > 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.
> > 
> > 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 good
> > patch, and maybe we'll revert the functionality one day if it turns out
> > that we can change the implementation so that a thread is never woken
> > when there is no work to do ....
> 
> 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.
> 
> How are the pool_stats used by administrators?

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.

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.

> 
> (And, why are they in /proc/fs/nfsd/ rather than under
> something RPC-related?)

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.

Thanks,
NeilBrown