Re: [PATCH v2 1/3] SUNRPC query xprt switch for number of active transports

[Olga Kornievskaia <olga.kornievskaia@gmail.com>]

On Thu, Jun 10, 2021 at 9:34 AM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
>
>
> > On Jun 9, 2021, at 5:53 PM, Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote:
> >
> > From: Olga Kornievskaia <kolga@netapp.com>
> >
> > To keep track of how many transports have already been added, add
> > ability to query the number.
>
> Just a random thought: Would it make more sense to plug the
> maximum allowed transports value into the struct rpc_clnt,
> and then rpc_clnt_test_and_add_xprt() could prevent the
> addition of the new xprt if the maximum would be exceeded?

Sure that could be done. Then the value of maximum allowed transports
should be defined at the RPC layer and not NFS? I currently check for
upper bounds during the parsing of the mount option, would I be not
doing that or exposing the RPC value to the NFS layer?

Actually I think it might be nice to add some kind of warning in the
log that a trunking transport wasn't created because the limit was
reached but if we move things to the RPC, we can't distinguish between
nconnect and trunking transports.

> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > ---
> > include/linux/sunrpc/clnt.h |  2 ++
> > net/sunrpc/clnt.c           | 13 +++++++++++++
> > 2 files changed, 15 insertions(+)
> >
> > diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> > index 02e7a5863d28..27042f1e581f 100644
> > --- a/include/linux/sunrpc/clnt.h
> > +++ b/include/linux/sunrpc/clnt.h
> > @@ -234,6 +234,8 @@ void rpc_clnt_xprt_switch_put(struct rpc_clnt *);
> > void rpc_clnt_xprt_switch_add_xprt(struct rpc_clnt *, struct rpc_xprt *);
> > bool rpc_clnt_xprt_switch_has_addr(struct rpc_clnt *clnt,
> >                       const struct sockaddr *sap);
> > +size_t rpc_clnt_xprt_switch_nactive(struct rpc_clnt *);
> > +
> > void rpc_cleanup_clids(void);
> >
> > static inline int rpc_reply_expected(struct rpc_task *task)
> > diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> > index 42623d6b8f0e..b46262ffcf72 100644
> > --- a/net/sunrpc/clnt.c
> > +++ b/net/sunrpc/clnt.c
> > @@ -2959,6 +2959,19 @@ bool rpc_clnt_xprt_switch_has_addr(struct rpc_clnt *clnt,
> > }
> > EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_has_addr);
> >
> > +size_t rpc_clnt_xprt_switch_nactive(struct rpc_clnt *clnt)
> > +{
> > +     struct rpc_xprt_switch *xps;
> > +     size_t num;
> > +
> > +     rcu_read_lock();
> > +     xps = rcu_dereference(clnt->cl_xpi.xpi_xpswitch);
> > +     num = xps->xps_nactive;
> > +     rcu_read_unlock();
> > +     return num;
> > +}
> > +EXPORT_SYMBOL_GPL(rpc_clnt_xprt_switch_nactive);
> > +
> > #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> > static void rpc_show_header(void)
> > {
> > --
> > 2.27.0
> >
>
> --
> Chuck Lever
>
>
>