On Tue, 2021-02-02 at 16:49 -0500, Trond Myklebust wrote:
> On Tue, 2021-02-02 at 13:42 -0500, schumaker.anna@xxxxxxxxx wrote:
> > From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> >
> > We won't have a task structure when we go to change IP addresses,
> > so
> > check for one before calling the WARN_ON() to avoid crashing.
> >
> > Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> > ---
> > net/sunrpc/xprtsock.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > index c56a66cdf4ac..250abf1aa018 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -2311,7 +2311,8 @@ static void xs_connect(struct rpc_xprt *xprt,
> > struct rpc_task *task)
> > struct sock_xprt *transport = container_of(xprt, struct
> > sock_xprt, xprt);
> > unsigned long delay = 0;
> >
> > - WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
> > + if (task)
> > + WARN_ON_ONCE(!xprt_lock_connect(xprt, task,
> > transport));
>
> Nit: That's the same as
> WARN_ON_ONCE(task && !xprt_lock_connect(xprt, task, transport));
>
> >
> > if (transport->sock != NULL) {
> > dprintk("RPC: xs_connect delayed xprt %p for
> > %lu "
>
So, the problem with this patch is that you're deliberately
circumventing the process of locking the socket. That's not going to
work.
What you could do is follow the example set by xprt_destroy() and
xs_enable_swap(), to call wait_on_bit_lock() when there is no task.
That should work, but you'd better make sure that your process holds a
reference to the xprt->kref before doing that, or else you could easily
end up deadlocking with xprt_destroy().
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
