Re: [PATCH v2 2/3] NFSv4 introduce max_connect mount options

[Trond Myklebust <trondmy@hammerspace.com>]

On Thu, 2021-06-10 at 13:30 +0000, Chuck Lever III wrote:
> 
> 
> > On Jun 9, 2021, at 5:53 PM, Olga Kornievskaia < 
> > olga.kornievskaia@gmail.com> wrote:
> > 
> > From: Olga Kornievskaia <kolga@netapp.com>
> > 
> > This option will control up to how many xprts can the client
> > establish to the server. This patch parses the value and sets
> > up structures that keep track of max_connect.
> > 
> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > ---
> > fs/nfs/client.c           |  1 +
> > fs/nfs/fs_context.c       |  8 ++++++++
> > fs/nfs/internal.h         |  2 ++
> > fs/nfs/nfs4client.c       | 12 ++++++++++--
> > fs/nfs/super.c            |  2 ++
> > include/linux/nfs_fs_sb.h |  1 +
> > 6 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index 330f65727c45..486dec59972b 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -179,6 +179,7 @@ struct nfs_client *nfs_alloc_client(const
> > struct nfs_client_initdata *cl_init)
> > 
> >         clp->cl_proto = cl_init->proto;
> >         clp->cl_nconnect = cl_init->nconnect;
> > +       clp->cl_max_connect = cl_init->max_connect ? cl_init-
> > >max_connect : 1;
> 
> So, 1 is the default setting, meaning the "add another transport"
> facility is disabled by default. Would it be less surprising for
> an admin to allow some extra connections by default?
> 
> 
> >         clp->cl_net = get_net(cl_init->net);
> > 
> >         clp->cl_principal = "*";
> > diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> > index d95c9a39bc70..cfbff7098f8e 100644
> > --- a/fs/nfs/fs_context.c
> > +++ b/fs/nfs/fs_context.c
> > @@ -29,6 +29,7 @@
> > #endif
> > 
> > #define NFS_MAX_CONNECTIONS 16
> > +#define NFS_MAX_TRANSPORTS 128
> 
> This maximum seems excessive... again, there are diminishing
> returns to adding more connections to the same server. what's
> wrong with re-using NFS_MAX_CONNECTIONS for the maximum?
> 
> As always, I'm a little queasy about adding yet another mount
> option. Are there real use cases where a whole-client setting
> (like a sysfs attribute) would be inadequate? Is there a way
> the client could figure out a reasonable maximum without a
> human intervention, say, by counting the number of NICs on
> the system?

Oh, hell no! We're not tying anything to the number of NICs...


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com