[...]
> > +#ifdef CONFIG_NFSD_V4
> > + if (rqstp->rq_vers == NFS4_VERSION &&
> > + rqstp->rq_proc == NFSPROC4_COMPOUND) {
> > + /* NFSv4 compund */
> > + struct nfsd4_compoundargs *args = rqstp->rq_argp;
> > + int j;
> > +
> > + opcnt = args->opcnt;
> > + for (j = 0; j < opcnt; j++) {
> > + struct nfsd4_op *op = &args->ops[j];
> > +
> > + rqstp_info.opnum[j] = op->opnum;
> > + }
> > + }
> > +#endif /* CONFIG_NFSD_V4 */
> > +
> > + /*
> > + * Acquire rq_status_counter before reporting the rqst
> > + * fields to the user.
> > + */
> > + if (smp_load_acquire(&rqstp->rq_status_counter) != status_counter)
> > + continue;
> > +
> > + seq_printf(m,
> > + "%04u %04ld NFSv%d %s %016lld",
> > + be32_to_cpu(rqstp_info.rq_xid),
>
> It's proper to display XIDs as 8-hexit hexadecimal values, as you
> did before. "0x%08x" is the correct format, as that matches the
> XID display format used by Wireshark and our tracepoints.
ops, I misunderstood your previous comments. I will address them in v7 if there
are no other comments.
Regards,
Lorenzo
>
>
> > + rqstp_info.rq_flags,
>
> I didn't mean for you to change the flags format to decimal. I was
> trying to point out that the content of this field will need to be
> displayed symbolically if we care about an easy user experience.
>
> Let's stick with hex here. A clever user can read the bits directly
> from that. All others should have a tool that parses this field and
> prints the list of bits in it symbolically.
>
>
> > + rqstp_info.rq_vers,
> > + rqstp_info.pc_name,
> > + ktime_to_us(rqstp_info.rq_stime));
> > + seq_printf(m, " %s",
> > + __svc_print_addr(&rqstp_info.saddr, buf,
> > + sizeof(buf), false));
> > + seq_printf(m, " %s",
> > + __svc_print_addr(&rqstp_info.daddr, buf,
> > + sizeof(buf), false));
> > + if (opcnt) {
> > + int j;
> > +
> > + seq_puts(m, " ");
> > + for (j = 0; j < opcnt; j++)
> > + seq_printf(m, "%s%s",
> > + nfsd4_op_name(rqstp_info.opnum[j]),
> > + j == opcnt - 1 ? "" : ":");
> > + } else {
> > + seq_puts(m, " -");
> > + }
>
> This looks correct to me.
>
> I'm leaning towards moving this to a netlink API that can be
> extended over time to handle other stats and also act as an NFSD
> control plane, similar to other network subsystems.
>
> Any comments, complaints or rotten fruit from anyone?
>
>
> > + seq_puts(m, "\n");
> > + }
> > + }
> > +
> > + rcu_read_unlock();
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * nfsd_rpc_status_open - open routine for nfsd_rpc_status handler
> > + * @inode: entry inode pointer.
> > + * @file: entry file pointer.
> > + *
> > + * nfsd_rpc_status_open is the open routine for nfsd_rpc_status procfs handler.
> > + * nfsd_rpc_status dumps pending RPC requests info queued into nfs server.
> > + */
> > +int nfsd_rpc_status_open(struct inode *inode, struct file *file)
> > +{
> > + int ret = nfsd_stats_open(inode);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + return single_open(file, nfsd_rpc_status_show, inode->i_private);
> > +}
> > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> > index 7838b37bcfa8..b49c0470b4fe 100644
> > --- a/include/linux/sunrpc/svc.h
> > +++ b/include/linux/sunrpc/svc.h
> > @@ -251,6 +251,7 @@ struct svc_rqst {
> > * net namespace
> > */
> > void ** rq_lease_breaker; /* The v4 client breaking a lease */
> > + unsigned int rq_status_counter; /* RPC processing counter */
> > };
> >
> > /* bits for rq_flags */
> > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> > index af692bff44ab..83bee19df104 100644
> > --- a/net/sunrpc/svc.c
> > +++ b/net/sunrpc/svc.c
> > @@ -1656,7 +1656,7 @@ const char *svc_proc_name(const struct svc_rqst *rqstp)
> > return rqstp->rq_procinfo->pc_name;
> > return "unknown";
> > }
> > -
> > +EXPORT_SYMBOL_GPL(svc_proc_name);
> >
> > /**
> > * svc_encode_result_payload - mark a range of bytes as a result payload
> > --
> > 2.41.0
> >
>
> --
> Chuck Lever
Attachment:
signature.asc
Description: PGP signature
