Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210426171947.99233-1-olga.kornievskaia@gmail.com>
 <20210426171947.99233-10-olga.kornievskaia@gmail.com> <20210427044214.vlbmbfdh5dpq4vhl@gmail.com>
In-Reply-To: <20210427044214.vlbmbfdh5dpq4vhl@gmail.com>
From:   Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date:   Thu, 13 May 2021 17:18:09 -0400
Message-ID: <CAN-5tyGe32FEeK0+bnMU16zu5Y6RkVqEey4G3VocEtx8vSQwiw@mail.gmail.com>
Subject: Re: [PATCH v3 09/13] sunrpc: add a symlink from rpc-client directory
 to the xprt_switch
To:     Dan Aloni <dan@kernelim.com>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        linux-nfs <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Apr 27, 2021 at 12:42 AM Dan Aloni <dan@kernelim.com> wrote:
>
> On Mon, Apr 26, 2021 at 01:19:43PM -0400, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <kolga@netapp.com>
> >
> > An rpc client uses a transport switch and one ore more transports
> > associated with that switch. Since transports are shared among
> > rpc clients, create a symlink into the xprt_switch directory
> > instead of duplicating entries under each rpc client.
> >
> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> >
> >..
> > @@ -188,6 +204,11 @@ void rpc_sysfs_client_destroy(struct rpc_clnt *clnt)
> >       struct rpc_sysfs_client *rpc_client = clnt->cl_sysfs;
> >
> >       if (rpc_client) {
> > +             char name[23];
> > +
> > +             snprintf(name, sizeof(name), "switch-%d",
> > +                      rpc_client->xprt_switch->xps_id);
> > +             sysfs_remove_link(&rpc_client->kobject, name);
>
> Hi Olga,
>
> If a client can use a single switch, shouldn't the name of the symlink
> be just "switch"? This is to be consistent with other symlinks in
> `sysfs` such as the ones in block layer, for example in my
> `/sys/block/sda`:
>
>     bdi -> ../../../../../../../../../../../virtual/bdi/8:0
>     device -> ../../../5:0:0:0
>

Jumping back to this comment because now that I went back to try to
modify the code I'm having doubts.

We still need numbering of xprt switches because they are different
for different mounts. So xprt_switches directory would still have
switch-0 for say a mount to server A and then switch-0 for a mount to
server B.  While yes I see that for a given rpc client that's making a
link into a xprt_switches directory will only have 1 link. And "yes"
the name of the link could be "switch". But isn't it more informative
to keep this to be the same name as the name of the directory under
the xprt_switches?

>
> --
> Dan Aloni