| Message ID | 20170426155527.92213-3-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 26, 2017 at 11:55:27AM -0400, Trond Myklebust wrote: > We want to use kthread_stop() in order to ensure the threads are > shut down before we tear down the nfs_callback_info in nfs_callback_down. > > Reported-by: Kinglong Mee <kinglongmee@gmail.com> > Fixes: bb6aeba736ba9 ("NFSv4.x: Switch to using svc_set_num_threads()...") > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > Cc: J. Bruce Fields <bfields@redhat.com> > --- > fs/nfs/callback.c | 24 ++++++++++++++++-------- > include/linux/sunrpc/svc.h | 1 + > net/sunrpc/svc.c | 38 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 55 insertions(+), 8 deletions(-) > > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c > index 773774531aff..8cabae9b140e 100644 > --- a/fs/nfs/callback.c > +++ b/fs/nfs/callback.c > @@ -76,7 +76,10 @@ nfs4_callback_svc(void *vrqstp) > > set_freezable(); > > - while (!kthread_should_stop()) { > + while (!kthread_freezable_should_stop(NULL)) { OK, I looked quickly at the comments for those two calls (kthread_should_stop and kthread_freezable_should_stop) and don't completely understand the difference. In any case, what does that change have to do with this patch? Patches make sense to me otherwise, shall I take them? --b. > + > + if (signal_pending(current)) > + flush_signals(current); > /* > * Listen for a request on the socket > */ > @@ -85,6 +88,8 @@ nfs4_callback_svc(void *vrqstp) > continue; > svc_process(rqstp); > } > + svc_exit_thread(rqstp); > + module_put_and_exit(0); > return 0; > } > > @@ -103,9 +108,10 @@ nfs41_callback_svc(void *vrqstp) > > set_freezable(); > > - while (!kthread_should_stop()) { > - if (try_to_freeze()) > - continue; > + while (!kthread_freezable_should_stop(NULL)) { > + > + if (signal_pending(current)) > + flush_signals(current); > > prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE); > spin_lock_bh(&serv->sv_cb_lock); > @@ -121,11 +127,13 @@ nfs41_callback_svc(void *vrqstp) > error); > } else { > spin_unlock_bh(&serv->sv_cb_lock); > - schedule(); > + if (!kthread_should_stop()) > + schedule(); > finish_wait(&serv->sv_cb_waitq, &wq); > } > - flush_signals(current); > } > + svc_exit_thread(rqstp); > + module_put_and_exit(0); > return 0; > } > > @@ -221,14 +229,14 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, > static struct svc_serv_ops nfs40_cb_sv_ops = { > .svo_function = nfs4_callback_svc, > .svo_enqueue_xprt = svc_xprt_do_enqueue, > - .svo_setup = svc_set_num_threads, > + .svo_setup = svc_set_num_threads_sync, > .svo_module = THIS_MODULE, > }; > #if defined(CONFIG_NFS_V4_1) > static struct svc_serv_ops nfs41_cb_sv_ops = { > .svo_function = nfs41_callback_svc, > .svo_enqueue_xprt = svc_xprt_do_enqueue, > - .svo_setup = svc_set_num_threads, > + .svo_setup = svc_set_num_threads_sync, > .svo_module = THIS_MODULE, > }; > > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h > index e770abeed32d..11cef5a7bc87 100644 > --- a/include/linux/sunrpc/svc.h > +++ b/include/linux/sunrpc/svc.h > @@ -474,6 +474,7 @@ void svc_pool_map_put(void); > struct svc_serv * svc_create_pooled(struct svc_program *, unsigned int, > struct svc_serv_ops *); > int svc_set_num_threads(struct svc_serv *, struct svc_pool *, int); > +int svc_set_num_threads_sync(struct svc_serv *, struct svc_pool *, int); > int svc_pool_stats_open(struct svc_serv *serv, struct file *file); > void svc_destroy(struct svc_serv *); > void svc_shutdown_net(struct svc_serv *, struct net *); > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c > index 98dc33ae738b..bc0f5a0ecbdc 100644 > --- a/net/sunrpc/svc.c > +++ b/net/sunrpc/svc.c > @@ -795,6 +795,44 @@ svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > } > EXPORT_SYMBOL_GPL(svc_set_num_threads); > > +/* destroy old threads */ > +static int > +svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > +{ > + struct task_struct *task; > + unsigned int state = serv->sv_nrthreads-1; > + > + /* destroy old threads */ > + do { > + task = choose_victim(serv, pool, &state); > + if (task == NULL) > + break; > + kthread_stop(task); > + nrservs++; > + } while (nrservs < 0); > + return 0; > +} > + > +int > +svc_set_num_threads_sync(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > +{ > + if (pool == NULL) { > + /* The -1 assumes caller has done a svc_get() */ > + nrservs -= (serv->sv_nrthreads-1); > + } else { > + spin_lock_bh(&pool->sp_lock); > + nrservs -= pool->sp_nrthreads; > + spin_unlock_bh(&pool->sp_lock); > + } > + > + if (nrservs > 0) > + return svc_start_kthreads(serv, pool, nrservs); > + if (nrservs < 0) > + return svc_stop_kthreads(serv, pool, nrservs); > + return 0; > +} > +EXPORT_SYMBOL_GPL(svc_set_num_threads_sync); > + > /* > * Called from a server thread as it's exiting. Caller must hold the "service > * mutex" for the service. > -- > 2.9.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/26/2017 23:55, Trond Myklebust wrote: > We want to use kthread_stop() in order to ensure the threads are > shut down before we tear down the nfs_callback_info in nfs_callback_down. > > Reported-by: Kinglong Mee <kinglongmee@gmail.com> > Fixes: bb6aeba736ba9 ("NFSv4.x: Switch to using svc_set_num_threads()...") > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > Cc: J. Bruce Fields <bfields@redhat.com> > --- > fs/nfs/callback.c | 24 ++++++++++++++++-------- > include/linux/sunrpc/svc.h | 1 + > net/sunrpc/svc.c | 38 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 55 insertions(+), 8 deletions(-) > > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c > index 773774531aff..8cabae9b140e 100644 > --- a/fs/nfs/callback.c > +++ b/fs/nfs/callback.c > @@ -76,7 +76,10 @@ nfs4_callback_svc(void *vrqstp) > > set_freezable(); > > - while (!kthread_should_stop()) { > + while (!kthread_freezable_should_stop(NULL)) { > + > + if (signal_pending(current)) > + flush_signals(current); > /* > * Listen for a request on the socket > */ > @@ -85,6 +88,8 @@ nfs4_callback_svc(void *vrqstp) > continue; > svc_process(rqstp); > } > + svc_exit_thread(rqstp); > + module_put_and_exit(0); > return 0; > } > > @@ -103,9 +108,10 @@ nfs41_callback_svc(void *vrqstp) > > set_freezable(); > > - while (!kthread_should_stop()) { > - if (try_to_freeze()) > - continue; > + while (!kthread_freezable_should_stop(NULL)) { > + > + if (signal_pending(current)) > + flush_signals(current); > > prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE); > spin_lock_bh(&serv->sv_cb_lock); > @@ -121,11 +127,13 @@ nfs41_callback_svc(void *vrqstp) > error); > } else { > spin_unlock_bh(&serv->sv_cb_lock); > - schedule(); > + if (!kthread_should_stop()) > + schedule(); > finish_wait(&serv->sv_cb_waitq, &wq); > } > - flush_signals(current); > } > + svc_exit_thread(rqstp); > + module_put_and_exit(0); > return 0; > } > > @@ -221,14 +229,14 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, > static struct svc_serv_ops nfs40_cb_sv_ops = { > .svo_function = nfs4_callback_svc, > .svo_enqueue_xprt = svc_xprt_do_enqueue, > - .svo_setup = svc_set_num_threads, > + .svo_setup = svc_set_num_threads_sync, > .svo_module = THIS_MODULE, > }; > #if defined(CONFIG_NFS_V4_1) > static struct svc_serv_ops nfs41_cb_sv_ops = { > .svo_function = nfs41_callback_svc, > .svo_enqueue_xprt = svc_xprt_do_enqueue, > - .svo_setup = svc_set_num_threads, > + .svo_setup = svc_set_num_threads_sync, > .svo_module = THIS_MODULE, > }; > > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h > index e770abeed32d..11cef5a7bc87 100644 > --- a/include/linux/sunrpc/svc.h > +++ b/include/linux/sunrpc/svc.h > @@ -474,6 +474,7 @@ void svc_pool_map_put(void); > struct svc_serv * svc_create_pooled(struct svc_program *, unsigned int, > struct svc_serv_ops *); > int svc_set_num_threads(struct svc_serv *, struct svc_pool *, int); > +int svc_set_num_threads_sync(struct svc_serv *, struct svc_pool *, int); > int svc_pool_stats_open(struct svc_serv *serv, struct file *file); > void svc_destroy(struct svc_serv *); > void svc_shutdown_net(struct svc_serv *, struct net *); > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c > index 98dc33ae738b..bc0f5a0ecbdc 100644 > --- a/net/sunrpc/svc.c > +++ b/net/sunrpc/svc.c > @@ -795,6 +795,44 @@ svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > } > EXPORT_SYMBOL_GPL(svc_set_num_threads); > > +/* destroy old threads */ > +static int > +svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > +{ > + struct task_struct *task; > + unsigned int state = serv->sv_nrthreads-1; > + > + /* destroy old threads */ > + do { > + task = choose_victim(serv, pool, &state); > + if (task == NULL) > + break; > + kthread_stop(task); > + nrservs++; > + } while (nrservs < 0); > + return 0; > +} > + > +int > +svc_set_num_threads_sync(struct svc_serv *serv, struct svc_pool *pool, int nrservs) > +{ > + if (pool == NULL) { > + /* The -1 assumes caller has done a svc_get() */ > + nrservs -= (serv->sv_nrthreads-1); > + } else { > + spin_lock_bh(&pool->sp_lock); > + nrservs -= pool->sp_nrthreads; > + spin_unlock_bh(&pool->sp_lock); > + } > + > + if (nrservs > 0) > + return svc_start_kthreads(serv, pool, nrservs); > + if (nrservs < 0) > + return svc_stop_kthreads(serv, pool, nrservs); > + return 0; > +} > +EXPORT_SYMBOL_GPL(svc_set_num_threads_sync); There is only one use of svc_set_num_threads that by nfsd, I'd like to change svc_set_num_threads and update nfsd than add a new function. Ps, "NFSv4.x/callback: Create the callback service through svc_create_pooled" must be include before the fix. I will resend it later. thanks, Kinglong Mee -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2017-04-27 at 11:06 +0800, Kinglong Mee wrote: > > There is only one use of svc_set_num_threads that by nfsd, > I'd like to change svc_set_num_threads and update nfsd than add a new > function. You can't really combine the two methods. Either you choose signals or you choose kthread_stop(). The problem is that signals require the thread to be able to persist past the nfsd_destroy() (which again forces things like nfsd() having to take nfsd_mutex), or you have to make things synchronous, in which case having nfsd() try to take nfsd_mutex causes deadlocks. IOW: if there is legacy behaviour here that requires the signal method, then knfsd cannot be converted. > Ps, > "NFSv4.x/callback: Create the callback service through > svc_create_pooled" > must be include before the fix. I will resend it later. > OK. Thanks! -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com
On 4/27/2017 11:24, Trond Myklebust wrote: > On Thu, 2017-04-27 at 11:06 +0800, Kinglong Mee wrote: >> >> There is only one use of svc_set_num_threads that by nfsd, >> I'd like to change svc_set_num_threads and update nfsd than add a new >> function. > > You can't really combine the two methods. Either you choose signals or > you choose kthread_stop(). The problem is that signals require the > thread to be able to persist past the nfsd_destroy() (which again > forces things like nfsd() having to take nfsd_mutex), or you have to > make things synchronous, in which case having nfsd() try to take > nfsd_mutex causes deadlocks. > > IOW: if there is legacy behaviour here that requires the signal method, > then knfsd cannot be converted. Got it. Tested-and-reviewed-by: Kinglong Mee <kinglongmee@gmail.com> thanks, Kinglong Mee > >> Ps, >> "NFSv4.x/callback: Create the callback service through >> svc_create_pooled" >> must be include before the fix. I will resend it later. >> > OK. Thanks! > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 27, 2017 at 11:54:48AM +0800, Kinglong Mee wrote: > > > On 4/27/2017 11:24, Trond Myklebust wrote: > > On Thu, 2017-04-27 at 11:06 +0800, Kinglong Mee wrote: > >> > >> There is only one use of svc_set_num_threads that by nfsd, > >> I'd like to change svc_set_num_threads and update nfsd than add a new > >> function. > > > > You can't really combine the two methods. Either you choose signals or > > you choose kthread_stop(). The problem is that signals require the > > thread to be able to persist past the nfsd_destroy() (which again > > forces things like nfsd() having to take nfsd_mutex), or you have to > > make things synchronous, in which case having nfsd() try to take > > nfsd_mutex causes deadlocks. > > > > IOW: if there is legacy behaviour here that requires the signal method, > > then knfsd cannot be converted. > > Got it. > > Tested-and-reviewed-by: Kinglong Mee <kinglongmee@gmail.com> So does what I have in git://linux-nfs.org/~bfields/linux.git nfsd-next look correct? (Alternatively, if Trond's taking this through his tree, that's fine too, feel free to add my ACK.) --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gVGh1LCAyMDE3LTA0LTI3IGF0IDE4OjAzIC0wNDAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6 DQo+IE9uIFRodSwgQXByIDI3LCAyMDE3IGF0IDExOjU0OjQ4QU0gKzA4MDAsIEtpbmdsb25nIE1l ZSB3cm90ZToNCj4gPiANCj4gPiANCj4gPiBPbiA0LzI3LzIwMTcgMTE6MjQsIFRyb25kIE15a2xl YnVzdCB3cm90ZToNCj4gPiA+IE9uIFRodSwgMjAxNy0wNC0yNyBhdCAxMTowNiArMDgwMCwgS2lu Z2xvbmcgTWVlIHdyb3RlOg0KPiA+ID4gPiANCj4gPiA+ID4gVGhlcmUgaXMgb25seSBvbmUgdXNl IG9mIHN2Y19zZXRfbnVtX3RocmVhZHMgdGhhdCBieSBuZnNkLA0KPiA+ID4gPiBJJ2QgbGlrZSB0 byBjaGFuZ2Ugc3ZjX3NldF9udW1fdGhyZWFkcyBhbmQgdXBkYXRlIG5mc2QgdGhhbiBhZGQNCj4g PiA+ID4gYSBuZXcNCj4gPiA+ID4gZnVuY3Rpb24uDQo+ID4gPiANCj4gPiA+IFlvdSBjYW4ndCBy ZWFsbHkgY29tYmluZSB0aGUgdHdvIG1ldGhvZHMuIEVpdGhlciB5b3UgY2hvb3NlDQo+ID4gPiBz aWduYWxzIG9yDQo+ID4gPiB5b3UgY2hvb3NlIGt0aHJlYWRfc3RvcCgpLiBUaGUgcHJvYmxlbSBp cyB0aGF0IHNpZ25hbHMgcmVxdWlyZQ0KPiA+ID4gdGhlDQo+ID4gPiB0aHJlYWQgdG8gYmUgYWJs ZSB0byBwZXJzaXN0IHBhc3QgdGhlIG5mc2RfZGVzdHJveSgpICh3aGljaCBhZ2Fpbg0KPiA+ID4g Zm9yY2VzIHRoaW5ncyBsaWtlIG5mc2QoKSBoYXZpbmcgdG8gdGFrZSBuZnNkX211dGV4KSwgb3Ig eW91IGhhdmUNCj4gPiA+IHRvDQo+ID4gPiBtYWtlIHRoaW5ncyBzeW5jaHJvbm91cywgaW4gd2hp Y2ggY2FzZSBoYXZpbmcgbmZzZCgpIHRyeSB0byB0YWtlDQo+ID4gPiBuZnNkX211dGV4IGNhdXNl cyBkZWFkbG9ja3MuDQo+ID4gPiANCj4gPiA+IElPVzogaWYgdGhlcmUgaXMgbGVnYWN5IGJlaGF2 aW91ciBoZXJlIHRoYXQgcmVxdWlyZXMgdGhlIHNpZ25hbA0KPiA+ID4gbWV0aG9kLA0KPiA+ID4g dGhlbiBrbmZzZCBjYW5ub3QgYmUgY29udmVydGVkLg0KPiA+IA0KPiA+IEdvdCBpdC4NCj4gPiAN Cj4gPiBUZXN0ZWQtYW5kLXJldmlld2VkLWJ5OiBLaW5nbG9uZyBNZWUgPGtpbmdsb25nbWVlQGdt YWlsLmNvbT4NCj4gDQo+IFNvIGRvZXMgd2hhdCBJIGhhdmUgaW4gZ2l0Oi8vbGludXgtbmZzLm9y Zy9+YmZpZWxkcy9saW51eC5naXQgbmZzZC0NCj4gbmV4dA0KPiBsb29rIGNvcnJlY3Q/DQo+IA0K PiAoQWx0ZXJuYXRpdmVseSwgaWYgVHJvbmQncyB0YWtpbmcgdGhpcyB0aHJvdWdoIGhpcyB0cmVl LCB0aGF0J3MgZmluZQ0KPiB0b28sIGZlZWwgZnJlZSB0byBhZGQgbXkgQUNLLikNCj4gDQpJZiB5 b3UndmUgYWxyZWFkeSBnb3QgaXQgcXVldWVkIHVwIHRoZW4gSSdtIGZpbmUgd2l0aCB0aGF0LiBJ IGhhdmUgaXQNCmluIG15ICJ0ZXN0aW5nIiBicmFuY2ggKHdoaWNoIHBhc3NlcyDimLopIGJ1dCBo YXZlbid0IHlldCBtb3ZlZCBpdCBpbnRvDQpsaW51eC1uZXh0Lg0KDQotLSANClRyb25kIE15a2xl YnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyLCBQcmltYXJ5RGF0YQ0KdHJvbmQubXlr bGVidXN0QHByaW1hcnlkYXRhLmNvbQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 27, 2017 at 10:07:07PM +0000, Trond Myklebust wrote: > On Thu, 2017-04-27 at 18:03 -0400, J. Bruce Fields wrote: > > On Thu, Apr 27, 2017 at 11:54:48AM +0800, Kinglong Mee wrote: > > > > > > > > > On 4/27/2017 11:24, Trond Myklebust wrote: > > > > On Thu, 2017-04-27 at 11:06 +0800, Kinglong Mee wrote: > > > > > > > > > > There is only one use of svc_set_num_threads that by nfsd, > > > > > I'd like to change svc_set_num_threads and update nfsd than add > > > > > a new > > > > > function. > > > > > > > > You can't really combine the two methods. Either you choose > > > > signals or > > > > you choose kthread_stop(). The problem is that signals require > > > > the > > > > thread to be able to persist past the nfsd_destroy() (which again > > > > forces things like nfsd() having to take nfsd_mutex), or you have > > > > to > > > > make things synchronous, in which case having nfsd() try to take > > > > nfsd_mutex causes deadlocks. > > > > > > > > IOW: if there is legacy behaviour here that requires the signal > > > > method, > > > > then knfsd cannot be converted. > > > > > > Got it. > > > > > > Tested-and-reviewed-by: Kinglong Mee <kinglongmee@gmail.com> > > > > So does what I have in git://linux-nfs.org/~bfields/linux.git nfsd- > > next > > look correct? > > > > (Alternatively, if Trond's taking this through his tree, that's fine > > too, feel free to add my ACK.) > > > If you've already got it queued up then I'm fine with that. I have it > in my "testing" branch (which passes ☺) but haven't yet moved it into > linux-next. OK, I'll take it. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c index 773774531aff..8cabae9b140e 100644 --- a/fs/nfs/callback.c +++ b/fs/nfs/callback.c @@ -76,7 +76,10 @@ nfs4_callback_svc(void *vrqstp) set_freezable(); - while (!kthread_should_stop()) { + while (!kthread_freezable_should_stop(NULL)) { + + if (signal_pending(current)) + flush_signals(current); /* * Listen for a request on the socket */ @@ -85,6 +88,8 @@ nfs4_callback_svc(void *vrqstp) continue; svc_process(rqstp); } + svc_exit_thread(rqstp); + module_put_and_exit(0); return 0; } @@ -103,9 +108,10 @@ nfs41_callback_svc(void *vrqstp) set_freezable(); - while (!kthread_should_stop()) { - if (try_to_freeze()) - continue; + while (!kthread_freezable_should_stop(NULL)) { + + if (signal_pending(current)) + flush_signals(current); prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE); spin_lock_bh(&serv->sv_cb_lock); @@ -121,11 +127,13 @@ nfs41_callback_svc(void *vrqstp) error); } else { spin_unlock_bh(&serv->sv_cb_lock); - schedule(); + if (!kthread_should_stop()) + schedule(); finish_wait(&serv->sv_cb_waitq, &wq); } - flush_signals(current); } + svc_exit_thread(rqstp); + module_put_and_exit(0); return 0; } @@ -221,14 +229,14 @@ static int nfs_callback_up_net(int minorversion, struct svc_serv *serv, static struct svc_serv_ops nfs40_cb_sv_ops = { .svo_function = nfs4_callback_svc, .svo_enqueue_xprt = svc_xprt_do_enqueue, - .svo_setup = svc_set_num_threads, + .svo_setup = svc_set_num_threads_sync, .svo_module = THIS_MODULE, }; #if defined(CONFIG_NFS_V4_1) static struct svc_serv_ops nfs41_cb_sv_ops = { .svo_function = nfs41_callback_svc, .svo_enqueue_xprt = svc_xprt_do_enqueue, - .svo_setup = svc_set_num_threads, + .svo_setup = svc_set_num_threads_sync, .svo_module = THIS_MODULE, }; diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h index e770abeed32d..11cef5a7bc87 100644 --- a/include/linux/sunrpc/svc.h +++ b/include/linux/sunrpc/svc.h @@ -474,6 +474,7 @@ void svc_pool_map_put(void); struct svc_serv * svc_create_pooled(struct svc_program *, unsigned int, struct svc_serv_ops *); int svc_set_num_threads(struct svc_serv *, struct svc_pool *, int); +int svc_set_num_threads_sync(struct svc_serv *, struct svc_pool *, int); int svc_pool_stats_open(struct svc_serv *serv, struct file *file); void svc_destroy(struct svc_serv *); void svc_shutdown_net(struct svc_serv *, struct net *); diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 98dc33ae738b..bc0f5a0ecbdc 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -795,6 +795,44 @@ svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) } EXPORT_SYMBOL_GPL(svc_set_num_threads); +/* destroy old threads */ +static int +svc_stop_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs) +{ + struct task_struct *task; + unsigned int state = serv->sv_nrthreads-1; + + /* destroy old threads */ + do { + task = choose_victim(serv, pool, &state); + if (task == NULL) + break; + kthread_stop(task); + nrservs++; + } while (nrservs < 0); + return 0; +} + +int +svc_set_num_threads_sync(struct svc_serv *serv, struct svc_pool *pool, int nrservs) +{ + if (pool == NULL) { + /* The -1 assumes caller has done a svc_get() */ + nrservs -= (serv->sv_nrthreads-1); + } else { + spin_lock_bh(&pool->sp_lock); + nrservs -= pool->sp_nrthreads; + spin_unlock_bh(&pool->sp_lock); + } + + if (nrservs > 0) + return svc_start_kthreads(serv, pool, nrservs); + if (nrservs < 0) + return svc_stop_kthreads(serv, pool, nrservs); + return 0; +} +EXPORT_SYMBOL_GPL(svc_set_num_threads_sync); + /* * Called from a server thread as it's exiting. Caller must hold the "service * mutex" for the service.
We want to use kthread_stop() in order to ensure the threads are shut down before we tear down the nfs_callback_info in nfs_callback_down. Reported-by: Kinglong Mee <kinglongmee@gmail.com> Fixes: bb6aeba736ba9 ("NFSv4.x: Switch to using svc_set_num_threads()...") Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> Cc: J. Bruce Fields <bfields@redhat.com> --- fs/nfs/callback.c | 24 ++++++++++++++++-------- include/linux/sunrpc/svc.h | 1 + net/sunrpc/svc.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-)