Received-SPF: pass (google.com: domain of linux-nfs+bounces-1714-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Benjamin Coddington <bcodding@redhat.com>
To: Olga Kornievskaia <olga.kornievskaia@gmail.com>
Cc: trond.myklebust@hammerspace.com, anna.schumaker@netapp.com,
 linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/1] NFSv4.1: add tracepoint to trunked nfs4_exchange_id
 calls
Date: Fri, 02 Feb 2024 10:36:11 -0500
Message-ID: <D23CBB93-3883-4E41-A1E5-848BCB2D477E@redhat.com>
In-Reply-To: <CAN-5tyGDQaaud7emd-Sgx_0E31bLx0k6EgrONSp0SbfoMTwY9A@mail.gmail.com>
References: <20240126190333.13528-1-olga.kornievskaia@gmail.com>
 <1DBC4AE1-E253-454F-9E7E-12DFBA14EBA6@redhat.com>
 <CAN-5tyGDQaaud7emd-Sgx_0E31bLx0k6EgrONSp0SbfoMTwY9A@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On 2 Feb 2024, at 9:42, Olga Kornievskaia wrote:

> On Fri, Feb 2, 2024 at 8:01=E2=80=AFAM Benjamin Coddington <bcodding@re=
dhat.com> wrote:
>>
>> On 26 Jan 2024, at 14:03, Olga Kornievskaia wrote:
>>
>>> From: Olga Kornievskaia <kolga@netapp.com>
>>>
>>> Add a tracepoint to track when the client sends EXCHANGE_ID to test
>>> a new transport for session trunking.
>>>
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>>  fs/nfs/nfs4proc.c  |  3 +++
>>>  fs/nfs/nfs4trace.h | 30 ++++++++++++++++++++++++++++++
>>>  2 files changed, 33 insertions(+)
>>>
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index 23819a756508..cdda7971c945 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -8974,6 +8974,9 @@ void nfs4_test_session_trunk(struct rpc_clnt *c=
lnt, struct rpc_xprt *xprt,
>>>               status =3D nfs4_detect_session_trunking(adata->clp,
>>>                               task->tk_msg.rpc_resp, xprt);
>>>
>>> +     trace_nfs4_trunked_exchange_id(adata->clp,
>>> +                     xprt->address_strings[RPC_DISPLAY_ADDR], status=
);
>>> +
>>
>> Any worry about the ambiguity of whether "status" comes from tk_status=
 or
>> from nfs4_detect_session_trunking() here?  The latter can return -EINV=
AL
>> which isn't in show_nfs4_status().
>
> Good catch, I didn't realize there wasn't an EINVAL mapping. I was
> focusing on capturing the fact that exchangeid was happening and ip
> info of the trunking connection that I didn't pay attention to the
> status. I'll send a v2 with EINVAL added to show_nfs4_status.

If you're only interested in tk_status, you could just move the tracepoin=
t.
That would resolve the conditional branch that changes the source of
"status".

Ben