On Wed, 20 Nov 2024, Chuck Lever wrote:
> On Wed, Nov 20, 2024 at 09:24:52AM +1100, NeilBrown wrote:
> > On Wed, 20 Nov 2024, Chuck Lever wrote:
> > > On Tue, Nov 19, 2024 at 11:41:30AM +1100, NeilBrown wrote:
> > > > Each client now reports the number of slots allocated in each session.
> > > >
> > > > Signed-off-by: NeilBrown <neilb@xxxxxxx>
> > > > ---
> > > > fs/nfsd/nfs4state.c | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > > index 3889ba1c653f..31ff9f92a895 100644
> > > > --- a/fs/nfsd/nfs4state.c
> > > > +++ b/fs/nfsd/nfs4state.c
> > > > @@ -2642,6 +2642,7 @@ static const char *cb_state2str(int state)
> > > > static int client_info_show(struct seq_file *m, void *v)
> > > > {
> > > > struct inode *inode = file_inode(m->file);
> > > > + struct nfsd4_session *ses;
> > > > struct nfs4_client *clp;
> > > > u64 clid;
> > > >
> > > > @@ -2678,6 +2679,13 @@ static int client_info_show(struct seq_file *m, void *v)
> > > > seq_printf(m, "callback address: \"%pISpc\"\n", &clp->cl_cb_conn.cb_addr);
> > > > seq_printf(m, "admin-revoked states: %d\n",
> > > > atomic_read(&clp->cl_admin_revoked));
> > > > + seq_printf(m, "session slots:");
> > > > + spin_lock(&clp->cl_lock);
> > > > + list_for_each_entry(ses, &clp->cl_sessions, se_perclnt)
> > > > + seq_printf(m, " %u", ses->se_fchannel.maxreqs);
> > > > + spin_unlock(&clp->cl_lock);
> > > > + seq_puts(m, "\n");
> > > > +
> > >
> > > Also, I wonder if information about the backchannel session can be
> > > surfaced in this way?
> > >
> >
> > Probably make sense. Maybe we should invent a syntax for reporting
> > arbitrary info about each session.
> >
> > session %d slots: %d
> > session %d cb-slots: %d
> > ...
> >
> > ???
>
> If each client has a directory, then it should have a subdirectory
> called "sessions". Each subdirectory of "sessions" should be one
> session, named by its hex session ID (as it is presented by
> Wireshark). Each session directory could have a file for the forward
> channel, one for the backchannel, and maybe one for generic
> information like when the session was created and how many
> connections it has.
>
> We don't need all of that in this patch set, but whatever is
> introduced here should be extensible to allow us to add more over
> time.
I cannot say I'm excited about the proliferation of tiny files. Your
suggestion isn't quite as bad as sysfs which claims to want one file per
value, but I think the sysfs approach provided more pain than gain and
you seem to be heading that way. As evidence I present the rise of
netlink. Netlink's main advantage is that it allows you to access a
collection of data in a single syscall (or maybe pair of syscalls). If
we had a standard format for doing that with open/read/close, the
filesystem would be a much nicer interface. But the sysfs rules prevent
that, so people who care avoid it.
We don't need to impose those same rules on nfsd-fs.
Having separate dirs for the clients makes some sense as the clients are
quite independent. Sessions aren't - they are just part of the client.
The *only* way session information is different from other client
information is that there is more structure - an array of sessions each
with detail. I don't think that justifies a new directory. I does
justify a carefully designed (or chosen) format for representing
structured data.
Thanks,
NeilBrown
