Received-SPF: pass (google.com: domain of linux-nfs+bounces-1720-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
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 13:24:15 -0500
Message-ID: <A124DBD9-38ED-4A4D-A2F8-C3296CA1E641@redhat.com>
In-Reply-To: <CAN-5tyEgFnr=NK=mqikC27ixyyORUp5igwtVKDUHs8G2=7bbmA@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>
 <D23CBB93-3883-4E41-A1E5-848BCB2D477E@redhat.com>
 <CAN-5tyHjkWsJPteVvLh83X3YTsvvS8vDfHBny-LMaAVbx0ww1g@mail.gmail.com>
 <A60265EE-8564-4772-A035-916F88C82107@redhat.com>
 <CAN-5tyGj_UcFLNkmqqc2bZD218r=Rvxj83_c69T7p9WNi_943Q@mail.gmail.com>
 <CAN-5tyEgFnr=NK=mqikC27ixyyORUp5igwtVKDUHs8G2=7bbmA@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 12:45, Olga Kornievskaia wrote:

> On Fri, Feb 2, 2024 at 12:05=E2=80=AFPM Olga Kornievskaia <
> olga.kornievskaia@gmail.com> wrote:
>
>>
>>
>> On Fri, Feb 2, 2024 at 11:56=E2=80=AFAM Benjamin Coddington <bcodding@=
redhat.com>
>> wrote:
>>
>>> On 2 Feb 2024, at 11:47, Olga Kornievskaia wrote:
>>>
>>>> On Fri, Feb 2, 2024 at 10:36=E2=80=AFAM Benjamin Coddington <
>>> bcodding@redhat.com>
>>>> wrote:
>>>>
>>>>> On 2 Feb 2024, at 9:42, Olga Kornievskaia wrote:
>>>>>
>>>>>> On Fri, Feb 2, 2024 at 8:01=E2=80=AFAM Benjamin Coddington <
>>> bcodding@redhat.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 t=
est
>>>>>>>> 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_cl=
nt
>>>>> *clnt, 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
>>> -EINVAL
>>>>>>> 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 i=
p
>>>>>> 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
>>> tracepoint.
>>>>> That would resolve the conditional branch that changes the source o=
f
>>>>> "status".
>>>>>
>>>>
>>>> We are not interested in tk_status that can be gotten from the
>>>> nfs4_xdr_status tracepoint. We are interested in the results of the
>>>> trunking decision.
>>>
>>> Gotcha, ok, I understand now.  Tucking into the conditional or moving=
 it
>>> into
>>> nfs4_detect_session_trunking() would make that clearer, but no big
>>> objection
>>> from me.
>>>
>>> If the task returns an error, this tracepoint will still be called wi=
th
>>> tk_status instead of any result from the trunking decision.
>>>
>>
>> :) I'll send a v3. though I don't see my v2 posted but i did send it
>> already.
>>
>
> Well looks like I'm not longer able to submit patches because of someth=
ing
> new fancy policies. I have used the same gitemail from mac as v1 and no=
w my
> emails are rejected. I use Mac's git sendmail command.
>
> Ben, how do you submit to your patches, do you use Mac git's or submit
> through something else? Thank you for the help.

I use git send-email from linux and I relay through redhat.com's smtp
servers, which are generally well-trusted.

Your v2 made it to me and the archives:
https://lore.kernel.org/linux-nfs/20240202145432.95487-1-olga.kornievskai=
a@gmail.com/T/#u


Ben