| Message ID | 20241119004928.3245873-4-neilb@suse.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | nfsd: allocate/free session-based DRC slots on demand | expand |
On Tue, Nov 19, 2024 at 11:41:30AM +1100, NeilBrown wrote: > Each client now reports the number of slots allocated in each session. Can this file also report the target slot count? Ie, is the server matching the client's requested slot count, or is it over or under by some number? Would it be useful for a server tester or administrator to poke a target slot count value into this file and watch the machinery adjust? > Signed-off-by: NeilBrown <neilb@suse.de> > --- > 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"); > + > drop_client(clp); > > return 0; > -- > 2.47.0 >
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@suse.de> > --- > 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? > drop_client(clp); > > return 0; > -- > 2.47.0 >
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. > > Can this file also report the target slot count? Ie, is the server > matching the client's requested slot count, or is it over or under > by some number? I could. Would you like to suggest a syntax? Usually the numbers would be the same except for short transition periods, so I'm not convinced of the value. Currently if the target is reduced while the client is idle there can be a longer delay before the slots are actually freed, but I think 2 lease-renewal SEQUENCE ops would do it. If/when we add use of the CB_RECALL_SLOT callback the delay should disappear. > > Would it be useful for a server tester or administrator to poke a > target slot count value into this file and watch the machinery > adjust? Maybe. By echo 3 > drop_caches does a pretty good job. I don't see that we need more. Thanks, NeilBrown
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@suse.de> > > --- > > 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 ... ??? NeilBrown
On Wed, Nov 20, 2024 at 09:22:59AM +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. > > > > Can this file also report the target slot count? Ie, is the server > > matching the client's requested slot count, or is it over or under > > by some number? > > I could. Would you like to suggest a syntax? > Usually the numbers would be the same except for short transition > periods, so I'm not convinced of the value. That's precisely the kind of situation I would like to be able catch -- the two are unequal longer than expected. > Currently if the target is reduced while the client is idle there can be > a longer delay before the slots are actually freed, but I think 2 > lease-renewal SEQUENCE ops would do it. If/when we add use of the > CB_RECALL_SLOT callback the delay should disappear. > > > Would it be useful for a server tester or administrator to poke a > > target slot count value into this file and watch the machinery > > adjust? > > Maybe. By echo 3 > drop_caches does a pretty good job. I don't see > that we need more. Fair enough.
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@suse.de> > > > --- > > > 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.
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"); + drop_client(clp); return 0;
Each client now reports the number of slots allocated in each session. Signed-off-by: NeilBrown <neilb@suse.de> --- fs/nfsd/nfs4state.c | 8 ++++++++ 1 file changed, 8 insertions(+)