| Message ID | 20201002225750.16452-1-calum.mackay@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lockd: don't use timed rebind with TCP | expand |
Please hold off for now on this one; I think I need to adjust the reclaimer a little. thanks, calum. On 02/10/2020 11:57 pm, Calum Mackay wrote: > It is possible for nlm_bind_host() to clear XPRT_BOUND whilst a connection > worker is in the middle of trying to reconnect. When the latter notices > that XPRT_BOUND been cleared under it, in xs_tcp_finish_connecting(), > that results in: > > xs_tcp_setup_socket: connect returned unhandled error -107 > > Worse, it's possible that the two can get into lockstep, resulting in > the same behaviour repeated indefinitely, with the above error every > 300 seconds, without ever recovering, and the connection never being > established. This is most likely to occur when there's a large number > of NLM client tasks following a server reboot. > > Since the timed rebind would seem not to be needed for TCP in any case, > whilst the existing connection remains, restrict the timed rebinding to > UDP only. > > For TCP, we will still rebind when needed, e.g. on timeout, connection > error (including closure), and in the reclaimer. > > Whilst there, refactor some duplicate code. > > Signed-off-by: Calum Mackay <calum.mackay@oracle.com> > --- > fs/lockd/host.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/fs/lockd/host.c b/fs/lockd/host.c > index 0afb6d59bad0..6e98c2ed6ffc 100644 > --- a/fs/lockd/host.c > +++ b/fs/lockd/host.c > @@ -439,12 +439,7 @@ nlm_bind_host(struct nlm_host *host) > * RPC rebind is required > */ > if ((clnt = host->h_rpcclnt) != NULL) { > - if (time_after_eq(jiffies, host->h_nextrebind)) { > - rpc_force_rebind(clnt); > - host->h_nextrebind = jiffies + NLM_HOST_REBIND; > - dprintk("lockd: next rebind in %lu jiffies\n", > - host->h_nextrebind - jiffies); > - } > + nlm_rebind_host(host); > } else { > unsigned long increment = nlmsvc_timeout; > struct rpc_timeout timeparms = { > @@ -495,15 +490,18 @@ nlm_bind_host(struct nlm_host *host) > } > > /* > - * Force a portmap lookup of the remote lockd port > + * Force a portmap lookup of the remote lockd port, unless we're using a > + * TCP connection. > */ > void > nlm_rebind_host(struct nlm_host *host) > { > - dprintk("lockd: rebind host %s\n", host->h_name); > - if (host->h_rpcclnt && time_after_eq(jiffies, host->h_nextrebind)) { > + if (unlikely(host->h_proto == IPPROTO_UDP) && host->h_rpcclnt && > + time_after_eq(jiffies, host->h_nextrebind)) { > rpc_force_rebind(host->h_rpcclnt); > host->h_nextrebind = jiffies + NLM_HOST_REBIND; > + dprintk("lockd: rebind host %s; next rebind in %lu jiffies\n", > + host->h_name, host->h_nextrebind - jiffies); > } > } > >
diff --git a/fs/lockd/host.c b/fs/lockd/host.c index 0afb6d59bad0..6e98c2ed6ffc 100644 --- a/fs/lockd/host.c +++ b/fs/lockd/host.c @@ -439,12 +439,7 @@ nlm_bind_host(struct nlm_host *host) * RPC rebind is required */ if ((clnt = host->h_rpcclnt) != NULL) { - if (time_after_eq(jiffies, host->h_nextrebind)) { - rpc_force_rebind(clnt); - host->h_nextrebind = jiffies + NLM_HOST_REBIND; - dprintk("lockd: next rebind in %lu jiffies\n", - host->h_nextrebind - jiffies); - } + nlm_rebind_host(host); } else { unsigned long increment = nlmsvc_timeout; struct rpc_timeout timeparms = { @@ -495,15 +490,18 @@ nlm_bind_host(struct nlm_host *host) } /* - * Force a portmap lookup of the remote lockd port + * Force a portmap lookup of the remote lockd port, unless we're using a + * TCP connection. */ void nlm_rebind_host(struct nlm_host *host) { - dprintk("lockd: rebind host %s\n", host->h_name); - if (host->h_rpcclnt && time_after_eq(jiffies, host->h_nextrebind)) { + if (unlikely(host->h_proto == IPPROTO_UDP) && host->h_rpcclnt && + time_after_eq(jiffies, host->h_nextrebind)) { rpc_force_rebind(host->h_rpcclnt); host->h_nextrebind = jiffies + NLM_HOST_REBIND; + dprintk("lockd: rebind host %s; next rebind in %lu jiffies\n", + host->h_name, host->h_nextrebind - jiffies); } }
It is possible for nlm_bind_host() to clear XPRT_BOUND whilst a connection worker is in the middle of trying to reconnect. When the latter notices that XPRT_BOUND been cleared under it, in xs_tcp_finish_connecting(), that results in: xs_tcp_setup_socket: connect returned unhandled error -107 Worse, it's possible that the two can get into lockstep, resulting in the same behaviour repeated indefinitely, with the above error every 300 seconds, without ever recovering, and the connection never being established. This is most likely to occur when there's a large number of NLM client tasks following a server reboot. Since the timed rebind would seem not to be needed for TCP in any case, whilst the existing connection remains, restrict the timed rebinding to UDP only. For TCP, we will still rebind when needed, e.g. on timeout, connection error (including closure), and in the reclaimer. Whilst there, refactor some duplicate code. Signed-off-by: Calum Mackay <calum.mackay@oracle.com> --- fs/lockd/host.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)