Re: [PATCH v4 2/2] NFSD: add rpc_status entry in nfsd debug filesystem

Lorenzo Bianconi
2023-08-02T09:00:04Z

[-- Attachment #1: Type: text/plain, Size: 2895 bytes --]

> On Sat, 29 Jul 2023, Lorenzo Bianconi wrote:
> > Introduce rpc_status entry in nfsd debug filesystem in order to dump
> > pending RPC requests debugging information.
> >

Hi Neil,

thx for the review.

> > Link: https://bugzilla.linux-nfs.org/show_bug.cgi?id=366
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > fs/nfsd/nfs4proc.c | 4 +-
> > fs/nfsd/nfsctl.c | 10 +++
> > fs/nfsd/nfsd.h | 2 +
> > fs/nfsd/nfssvc.c | 122 +++++++++++++++++++++++++++++++++++++
> > include/linux/sunrpc/svc.h | 1 +
> > net/sunrpc/svc.c | 2 +-
> > 6 files changed, 137 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index f0f318e78630..b7ad3081bc36 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -2497,8 +2497,6 @@ static inline void nfsd4_increment_op_stats(u32 opnum)
> >
> > static const struct nfsd4_operation nfsd4_ops[];
> >
> > -static const char *nfsd4_op_name(unsigned opnum);
> > -
> > /*
> > * Enforce NFSv4.1 COMPOUND ordering rules:
> > *
> > @@ -3628,7 +3626,7 @@ void warn_on_nonidempotent_op(struct nfsd4_op *op)
> > }
> > }
> >
> > -static const char *nfsd4_op_name(unsigned opnum)
> > +const char *nfsd4_op_name(unsigned opnum)
> > {
> > if (opnum < ARRAY_SIZE(nfsd4_ops))
> > return nfsd4_ops[opnum].op_name;
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index 35d2e2cde1eb..f2e4f4b1e4d1 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -57,6 +57,8 @@ enum {
> > NFSD_RecoveryDir,
> > NFSD_V4EndGrace,
> > #endif
> > + NFSD_Rpc_Status,
>
> I think NFSD_Rpc_Status needs to come before the CONFIG_NFSD_V4 block.
> Otherwise the comment above (which I apparently approved) makes even
> less sense than it does now.
> (Maybe just remove the comment??)

ack, right. I will fix it.

>
> > +
> > NFSD_MaxReserved
> > };
> >
> > @@ -195,6 +197,13 @@ static inline struct net *netns(struct file *file)
> > return file_inode(file)->i_sb->s_fs_info;
> > }
> >

[...]

> > +#endif /* CONFIG_NFSD_V4 */
> > +
> > + /* In order to detect if the RPC request is pending and
> > + * RPC info are stable we check if rq_status_counter
> > + * has been incremented during the handler processing.
> > + */
> > + if (status_counter != atomic_read(&rqstp->rq_status_counter))
> > + continue;
> > +
> > + seq_printf(m,
> > + "0x%08x, 0x%08lx, 0x%08x, NFSv%d, %s, %016lld,",
>
> Please drop the commas.
> It might be defensible to have commas and no spaces by comparing with
> /proc/fs/nfsd/supported_krb5_enctypes, but the dominant pattern is to
> use only spaces to separate fields on /proc files.

ack, I will fix it.

Regards,
Lorenzo

>
> Thanks,
> NeilBrown
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]