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

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 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 *cln=
t, 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 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.

> Otherwise, looks good to me.
>
> Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
>
> Ben
>