Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <162083366966.3108.12581818416105328952.stgit@klimt.1015granger.net>
 <162083375498.3108.14242612463783055564.stgit@klimt.1015granger.net>
In-Reply-To: <162083375498.3108.14242612463783055564.stgit@klimt.1015granger.net>
From:   David Wysochanski <dwysocha@redhat.com>
Date:   Thu, 13 May 2021 12:42:37 -0400
Message-ID: <CALF+zOkZYg7RzayueKGwFaE-8sHKTB6g4q_Ej-+u=MkH35Dnqw@mail.gmail.com>
Subject: Re: [PATCH v2 09/25] NFSD: Add a couple more nfsd_clid_expired call sites
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     linux-nfs <linux-nfs@vger.kernel.org>,
        "J. Bruce Fields" <bfields@fieldses.org>, rostedt@goodmis.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, May 12, 2021 at 11:36 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> Improve observation of NFSv4 lease expiry.
>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4state.c |    3 +++
>  1 file changed, 3 insertions(+)
>

How about adding a parameter to explain the location of the expiry to
make it more slightly more readable?
Below is an attempt at the two added here, I think there's one more
not shown though in nfs4_laundromat, which you could just use
"laundromat".

> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 08ff643e96fb..7fa90a3177fa 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2665,6 +2665,8 @@ static void force_expire_client(struct nfs4_client *clp)
>         struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
>         bool already_expired;
>
> +       trace_nfsd_clid_expired(&clp->cl_clientid);
> +

trace_nfsd_clid_expired(..., "admin forced");

>         spin_lock(&clp->cl_lock);
>         clp->cl_time = 0;
>         spin_unlock(&clp->cl_lock);
> @@ -4075,6 +4077,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
>                                 goto out;
>                         status = mark_client_expired_locked(old);
>                         if (status) {
> +                               trace_nfsd_clid_expired(&old->cl_clientid);

trace_nfsd_clid_expired(..., "setclientid_confirm existing"); /* found
an existing confirmed clientid by name */

>                                 old = NULL;
>                                 goto out;
>                         }
>
>