* [PATCH 0/1] NFSv4 callback service ate my homework @ 2023-03-03 21:08 Benjamin Coddington 2023-03-03 21:08 ` [PATCH 1/1] SUNRPC: Fix a server shutdown leak Benjamin Coddington 0 siblings, 1 reply; 5+ messages in thread From: Benjamin Coddington @ 2023-03-03 21:08 UTC (permalink / raw) To: chuck.lever, jlayton, trond.myklebust, anna, davem, edumazet, kuba, pabeni, bfields Cc: linux-nfs A system with very low autofs timeouts for many mounts of v4.0 exports, while attempting to mount with the non-specific vers=4 option would run out of memory every few days. Each mount was doing the v4.2, v4.1, v4.0 negotiation and constantly setting up and tearing down the backchannel service for positive minorversions. We found a probable cause: wake_up_process() isn't guarunteed to execute the threadfn; kthread_stop() can race in and prevent it. Benjamin Coddington (1): SUNRPC: Fix a server shutdown leak net/sunrpc/svc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] SUNRPC: Fix a server shutdown leak 2023-03-03 21:08 [PATCH 0/1] NFSv4 callback service ate my homework Benjamin Coddington @ 2023-03-03 21:08 ` Benjamin Coddington 2023-03-08 3:14 ` Chuck Lever III 2023-03-08 10:48 ` Jeff Layton 0 siblings, 2 replies; 5+ messages in thread From: Benjamin Coddington @ 2023-03-03 21:08 UTC (permalink / raw) To: chuck.lever, jlayton, trond.myklebust, anna, davem, edumazet, kuba, pabeni, bfields Cc: linux-nfs Fix a race where kthread_stop() may prevent the threadfn from ever getting called. If that happens the svc_rqst will not be cleaned up. Fixes: ed6473ddc704 ("NFSv4: Fix callback server shutdown") Signed-off-by: Benjamin Coddington <bcodding@redhat.com> --- net/sunrpc/svc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 1fd3f5e57285..fea7ce8fba14 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -798,6 +798,7 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) static int svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) { + struct svc_rqst *rqstp; struct task_struct *task; unsigned int state = serv->sv_nrthreads-1; @@ -806,7 +807,10 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) task = choose_victim(serv, pool, &state); if (task == NULL) break; - kthread_stop(task); + rqstp = kthread_data(task); + /* Did we lose a race to svo_function threadfn? */ + if (kthread_stop(task) == -EINTR) + svc_exit_thread(rqstp); nrservs++; } while (nrservs < 0); return 0; -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] SUNRPC: Fix a server shutdown leak 2023-03-03 21:08 ` [PATCH 1/1] SUNRPC: Fix a server shutdown leak Benjamin Coddington @ 2023-03-08 3:14 ` Chuck Lever III 2023-03-08 10:37 ` Benjamin Coddington 2023-03-08 10:48 ` Jeff Layton 1 sibling, 1 reply; 5+ messages in thread From: Chuck Lever III @ 2023-03-08 3:14 UTC (permalink / raw) To: Benjamin Coddington Cc: Jeff Layton, Trond Myklebust, Anna Schumaker, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bruce Fields, Linux NFS Mailing List > On Mar 3, 2023, at 4:08 PM, Benjamin Coddington <bcodding@redhat.com> wrote: > > Fix a race where kthread_stop() may prevent the threadfn from ever getting > called. If that happens the svc_rqst will not be cleaned up. > > Fixes: ed6473ddc704 ("NFSv4: Fix callback server shutdown") > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> > --- > net/sunrpc/svc.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c > index 1fd3f5e57285..fea7ce8fba14 100644 > --- a/net/sunrpc/svc.c > +++ b/net/sunrpc/svc.c > @@ -798,6 +798,7 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > static int > svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > { > + struct svc_rqst *rqstp; > struct task_struct *task; > unsigned int state = serv->sv_nrthreads-1; > > @@ -806,7 +807,10 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > task = choose_victim(serv, pool, &state); > if (task == NULL) > break; > - kthread_stop(task); > + rqstp = kthread_data(task); > + /* Did we lose a race to svo_function threadfn? */ > + if (kthread_stop(task) == -EINTR) > + svc_exit_thread(rqstp); > nrservs++; > } while (nrservs < 0); > return 0; > -- > 2.31.1 > Seems sensible, applied. Is there a bugzilla link that should be included? -- Chuck Lever ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] SUNRPC: Fix a server shutdown leak 2023-03-08 3:14 ` Chuck Lever III @ 2023-03-08 10:37 ` Benjamin Coddington 0 siblings, 0 replies; 5+ messages in thread From: Benjamin Coddington @ 2023-03-08 10:37 UTC (permalink / raw) To: Chuck Lever III Cc: Jeff Layton, Trond Myklebust, Anna Schumaker, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Bruce Fields, Linux NFS Mailing List On 7 Mar 2023, at 22:14, Chuck Lever III wrote: >> On Mar 3, 2023, at 4:08 PM, Benjamin Coddington <bcodding@redhat.com> wrote: >> >> Fix a race where kthread_stop() may prevent the threadfn from ever getting >> called. If that happens the svc_rqst will not be cleaned up. >> >> Fixes: ed6473ddc704 ("NFSv4: Fix callback server shutdown") >> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >> --- >> net/sunrpc/svc.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c >> index 1fd3f5e57285..fea7ce8fba14 100644 >> --- a/net/sunrpc/svc.c >> +++ b/net/sunrpc/svc.c >> @@ -798,6 +798,7 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) >> static int >> svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) >> { >> + struct svc_rqst *rqstp; >> struct task_struct *task; >> unsigned int state = serv->sv_nrthreads-1; >> >> @@ -806,7 +807,10 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) >> task = choose_victim(serv, pool, &state); >> if (task == NULL) >> break; >> - kthread_stop(task); >> + rqstp = kthread_data(task); >> + /* Did we lose a race to svo_function threadfn? */ >> + if (kthread_stop(task) == -EINTR) >> + svc_exit_thread(rqstp); >> nrservs++; >> } while (nrservs < 0); >> return 0; >> -- >> 2.31.1 >> > > Seems sensible, applied. Is there a bugzilla link that should be included? No, the issue was found in a private environment. Ben ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] SUNRPC: Fix a server shutdown leak 2023-03-03 21:08 ` [PATCH 1/1] SUNRPC: Fix a server shutdown leak Benjamin Coddington 2023-03-08 3:14 ` Chuck Lever III @ 2023-03-08 10:48 ` Jeff Layton 1 sibling, 0 replies; 5+ messages in thread From: Jeff Layton @ 2023-03-08 10:48 UTC (permalink / raw) To: Benjamin Coddington, chuck.lever, trond.myklebust, anna, davem, edumazet, kuba, pabeni, bfields Cc: linux-nfs On Fri, 2023-03-03 at 16:08 -0500, Benjamin Coddington wrote: > Fix a race where kthread_stop() may prevent the threadfn from ever getting > called. If that happens the svc_rqst will not be cleaned up. > > Fixes: ed6473ddc704 ("NFSv4: Fix callback server shutdown") > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> > --- > net/sunrpc/svc.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c > index 1fd3f5e57285..fea7ce8fba14 100644 > --- a/net/sunrpc/svc.c > +++ b/net/sunrpc/svc.c > @@ -798,6 +798,7 @@ svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > static int > svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > { > + struct svc_rqst *rqstp; > struct task_struct *task; > unsigned int state = serv->sv_nrthreads-1; > > @@ -806,7 +807,10 @@ svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > task = choose_victim(serv, pool, &state); > if (task == NULL) > break; > - kthread_stop(task); > + rqstp = kthread_data(task); > + /* Did we lose a race to svo_function threadfn? */ > + if (kthread_stop(task) == -EINTR) > + svc_exit_thread(rqstp); > nrservs++; > } while (nrservs < 0); > return 0; Reviewed-by: Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-08 10:48 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-03 21:08 [PATCH 0/1] NFSv4 callback service ate my homework Benjamin Coddington 2023-03-03 21:08 ` [PATCH 1/1] SUNRPC: Fix a server shutdown leak Benjamin Coddington 2023-03-08 3:14 ` Chuck Lever III 2023-03-08 10:37 ` Benjamin Coddington 2023-03-08 10:48 ` Jeff Layton
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.