Re: [PATCH 5/7] NFSv4.2 add tracepoint to CB_OFFLOAD

[Chuck Lever III <chuck.lever@oracle.com>]

> On Oct 19, 2021, at 12:01 PM, Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote:
> 
> On Tue, Oct 19, 2021 at 11:17 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>> 
>> 
>> 
>>> On Oct 18, 2021, at 6:03 PM, Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote:
>>> 
>>> From: Olga Kornievskaia <kolga@netapp.com>
>>> 
>>> Add a tracepoint to the CB_OFFLOAD operation.
>>> 
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>> fs/nfs/callback_proc.c |  3 +++
>>> fs/nfs/nfs4trace.h     | 50 ++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 53 insertions(+)
>>> 
>>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>>> index ed9d580826f5..09c5b1cb3e07 100644
>>> --- a/fs/nfs/callback_proc.c
>>> +++ b/fs/nfs/callback_proc.c
>>> @@ -739,6 +739,9 @@ __be32 nfs4_callback_offload(void *data, void *dummy,
>>>              kfree(copy);
>>>      spin_unlock(&cps->clp->cl_lock);
>>> 
>>> +     trace_nfs4_cb_offload(&args->coa_fh, &args->coa_stateid,
>>> +                     args->wr_count, args->error,
>>> +                     args->wr_writeverf.committed);
>>>      return 0;
>>> }
>>> #endif /* CONFIG_NFS_V4_2 */
>>> diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
>>> index cc6537a20ebe..33f52d486528 100644
>>> --- a/fs/nfs/nfs4trace.h
>>> +++ b/fs/nfs/nfs4trace.h
>>> @@ -2714,6 +2714,56 @@ TRACE_EVENT(nfs4_clone,
>>>              )
>>> );
>>> 
>>> +#define show_write_mode(how)                 \
>>> +        __print_symbolic(how,                        \
>>> +                { NFS_UNSTABLE, "UNSTABLE" },        \
>>> +                { NFS_DATA_SYNC, "DATA_SYNC" },      \
>>> +             { NFS_FILE_SYNC, "FILE_SYNC"})
>> 
>> Is there no way to reuse fs/nfs/nfstrace.h::nfs_show_stable() ?
>> 
>> Btw, I have patches that move some NFS trace infrastructure
>> into include/trace/events so that it can be shared between the
>> NFS client and server trace subsystems. They might be useful
>> here too.
>> 
>> The generic FS macros are moved in this commit:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=nfsd-more-tracepoints&id=495731e1332c7e26af1e04a88eb65e3c08dfbf53
>> 
>> Some NFS macros are moved in this commit:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=nfsd-more-tracepoints&id=24763f8889e0a18a8d06ddcd05bac06a7d043515
>> 
>> Additional macros are introduced later in that same branch.
>> 
>> I don't have any opinion about whether these should be
>> applied before your patches or after them.
> 
> It sounds like, if there was already a show_nfs_stable_how() that I
> could call then I don't need to define what I'm defining? So if I
> apply your patches and and then my patches on top of that, that seems
> like the way to go? That depends on if your patch(es) are ready to be
> submitted or not? Alternatively, your patch(es) can fix my code. I
> don't have a preference either way.

I can post those two now and the list can decide if they are ready.


>>> +
>>> +TRACE_EVENT(nfs4_cb_offload,
>>> +             TP_PROTO(
>>> +                     const struct nfs_fh *cb_fh,
>>> +                     const nfs4_stateid *cb_stateid,
>>> +                     uint64_t cb_count,
>>> +                     int cb_error,
>>> +                     int cb_how_stable
>>> +             ),
>>> +
>>> +             TP_ARGS(cb_fh, cb_stateid, cb_count, cb_error,
>>> +                     cb_how_stable),
>>> +
>>> +             TP_STRUCT__entry(
>>> +                     __field(unsigned long, error)
>>> +                     __field(u32, fhandle)
>>> +                     __field(loff_t, cb_count)
>>> +                     __field(int, cb_how)
>>> +                     __field(int, cb_stateid_seq)
>>> +                     __field(u32, cb_stateid_hash)
>>> +             ),
>>> +
>>> +             TP_fast_assign(
>>> +                     __entry->error = cb_error < 0 ? -cb_error : 0;
>>> +                     __entry->fhandle = nfs_fhandle_hash(cb_fh);
>>> +                     __entry->cb_stateid_seq =
>>> +                             be32_to_cpu(cb_stateid->seqid);
>>> +                     __entry->cb_stateid_hash =
>>> +                             nfs_stateid_hash(cb_stateid);
>>> +                     __entry->cb_count = cb_count;
>>> +                     __entry->cb_how = cb_how_stable;
>>> +             ),
>>> +
>>> +             TP_printk(
>>> +                     "error=%ld (%s) fhandle=0x%08x cb_stateid=%d:0x%08x "
>>> +                     "cb_count=%llu cb_how=%s",
>>> +                     -__entry->error,
>>> +                     show_nfsv4_errors(__entry->error),
>>> +                     __entry->fhandle,
>>> +                     __entry->cb_stateid_seq, __entry->cb_stateid_hash,
>>> +                     __entry->cb_count,
>>> +                     show_write_mode(__entry->cb_how)
>>> +             )
>>> +);
>>> +
>>> #endif /* CONFIG_NFS_V4_1 */
>>> 
>>> #endif /* _TRACE_NFS4_H */
>>> --
>>> 2.27.0
>>> 
>> 
>> --
>> Chuck Lever

--
Chuck Lever