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;
From:   "Benjamin Coddington" <bcodding@redhat.com>
To:     "Chuck Lever III" <chuck.lever@oracle.com>
Cc:     trondmy@kernel.org,
        "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v9 03/27] NFS: Trace lookup revalidation failure
Date:   Wed, 09 Mar 2022 16:35:53 -0500
Message-ID: <FBED54D6-7C0F-4AE1-961E-78D3DDE00F6F@redhat.com>
In-Reply-To: <52214581-7C9A-425A-9B8E-082B7ACE23C1@oracle.com>
References: <20220227231227.9038-1-trondmy@kernel.org>
 <20220227231227.9038-2-trondmy@kernel.org>
 <20220227231227.9038-3-trondmy@kernel.org>
 <20220227231227.9038-4-trondmy@kernel.org>
 <638A1DDB-5424-4FFA-A5A5-D212519D3A37@redhat.com>
 <52214581-7C9A-425A-9B8E-082B7ACE23C1@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed
Precedence: bulk

On 9 Mar 2022, at 10:28, Chuck Lever III wrote:

>> On Mar 9, 2022, at 8:42 AM, Benjamin Coddington <bcodding@redhat.com> 
>> wrote:
>>
>> On 24 Feb 2022, at 21:09, Trond Myklebust wrote:
>>> On Thu, 2022-02-24 at 09:14 -0500, Benjamin Coddington wrote:
>>>> There's a path through nfs4_lookup_revalidate that will now only 
>>>> produce
>>>> this exit tracepoint.  Does it need the _enter tracepoint added?
>>>
>>> You're thinking about the nfs_lookup_revalidate_delegated() path? 
>>> The
>>> _enter() tracepoint doesn't provide any useful information that 
>>> isn't
>>> already provided by the _exit(), AFAICS.
>>
>> No, the path through nfs4_do_lookup_revalidate(), reval_dentry: jump. 
>>  But I
>> agree there's not much value in the _enter() tracepoint.  Maybe we 
>> can
>> remove it, and _exit more like _done.
>>
>> I am thinking about hearing back from folks about mis-matched 
>> _enter() and
>> _exit() results, but also realize this is nit-picking.
>
> I think the _enter / _exit trace points simply replaced
> dprintk call sites which did much the same reporting.
> Maybe we should consider replacing some of these because
> we can rely on function call tracing instead.
>
> But generally we like to see trace points that report
> exceptional events rather than "I made it to this point".
> The latter category of trace points are interesting
> while code is under development but often loses its
> value once the code is in the field.

Instead of "hearing back from folks" I should have said "I worry our QE 
team
is going to discover and possibly report as a bug". :P  Thanks for 
filling
in Chuck!

Ben