> On Nov 21, 2024, at 4:03 PM, NeilBrown <neilb@xxxxxxx> wrote:
>
> 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.
I don't see this set of information as being in a
performance path. Needing multiple open/read/close
iterations doesn't seem like an impediment to me.
The only possible issue is that user space might
want a snapshot of certain related values, and
having to get the values from multiple files means
there's no guarantee that the values are consistent
with each other.
> 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.
Hrm. IMHO a directory is exactly suited to this kind of
information hierarchy. I can't say that I understand your
view; perhaps you feel this way because the client
implementations we are familiar with use only a single
session. For that, of course, a directory is overkill.
> It does
> justify a carefully designed (or chosen) format for representing
> structured data.
That usually means JSON or XML, which also have their haters.
However, I don't feel strongly about this. You asked me
for some thoughts, and here they are, at random.
My bottom line is reasonable extensibility -- the ability to
provide more information in these files in the future without
perturbing current consumers. IME that's been nearly impossible
with designs that have one file full of fields that need to be
parsed.
Should we expose session information via the new NFSD netlink
protocol instead? Or a sessions/ directory with one formatted
file per session? I'm open to discussion.
--
Chuck Lever
