Thread (21 messages) 21 messages, 5 authors, 2021-02-14

Re: [PATCH v2 4/5] sunrpc: Prepare xs_connect() for taking NULL tasks

From: Trond Myklebust <hidden>
Date: 2021-02-02 22:00:34

On Tue, 2021-02-02 at 16:49 -0500, Trond Myklebust wrote:
On Tue, 2021-02-02 at 13:42 -0500, schumaker.anna@gmail.com wrote:
quoted
From: Anna Schumaker <redacted>

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 <redacted>
---
 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));
quoted
 
        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@hammerspace.com

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help