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: <3FF89A33-6C7C-4BC2-9973-A20E46A8A339@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>,
 <168972716973.11078.4474704978173049217@noble.neil.brown.name>,
 <3FF89A33-6C7C-4BC2-9973-A20E46A8A339@oracle.com>
Date:   Thu, 20 Jul 2023 09:27:23 +1000
Message-id: <168980924309.11078.8450297121939615798@noble.neil.brown.name>
Precedence: bulk

On Wed, 19 Jul 2023, Chuck Lever III wrote:
> 
> > On Jul 18, 2023, at 8:39 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > 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.
> > 
> > I'm now wondering if that is a reliable statistic.
> > 
> > This counter as implemented doesn't count "pool threads that were awoken
> > but found no work to do".  Rather, it counts "pool threads that found no
> > work to do, either after having been awoken, or having just completed
> > some other request".
> 
> In the current code, the only way to get to "return -EAGAIN;" is if the
> thread calls schedule_timeout() (ie, it actually sleeps), then the
> svc_rqst was specifically selected and awoken, and the schedule_timeout()
> did not time out.
> 
> I don't see a problem.
> 

Yeah - I don't either any more.  Sorry for the noise.


> 
> > And it doesn't even really count that is it doesn't notice that lockd
> > "retry blocked" work (or the NFSv4.1 callback work, but we don't count
> > that at all I think).
> > 
> > Maybe we should only update the count if we had actually been woken up
> > recently.
> 
> So this one can be dropped for now since it's currently of value only
> for working on the scheduler changes. If the thread scheduler were to
> change such that a work item was actually assigned to a thread before
> it is awoken (a la, a work queue model) then this counter would be
> truly meaningless. I think we can wait for a bit.
> 

We used to assign a workitem to a thread before it was woken.  I find
that model to be aesthetically pleasing.
Trond changed that in
  Commit: 22700f3c6df5 ("SUNRPC: Improve ordering of transport processing")

claiming that the wake-up time for a sleeping thread could result in
poorer throughput.  No data given but I find the reasoning quite
credible.

Thanks,
NeilBrown

> 
> --
> Chuck Lever
> 
> 
>