Fix the problem reported by Kinglong, whereby the callback channel was
failing to shut down properly on umount.
Trond Myklebust (2):
SUNRPC: Refactor svc_set_num_threads()
NFSv4: Fix callback server shutdown
fs/nfs/callback.c | 24 +++++---
include/linux/sunrpc/svc.h | 1 +
net/sunrpc/svc.c | 134 ++++++++++++++++++++++++++++++++-------------
3 files changed, 113 insertions(+), 46 deletions(-)
--
2.9.3
Refactor to separate out the functions of starting and stopping threads
so that they can be used in other helpers.
Signed-off-by: Trond Myklebust <[email protected]>
Cc: J. Bruce Fields <[email protected]>
---
net/sunrpc/svc.c | 96 ++++++++++++++++++++++++++++++++++----------------------
1 file changed, 58 insertions(+), 38 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index a08aeb56b8e4..98dc33ae738b 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -702,59 +702,32 @@ choose_victim(struct svc_serv *serv, struct svc_pool *pool, unsigned int *state)
return task;
}
-/*
- * Create or destroy enough new threads to make the number
- * of threads the given number. If `pool' is non-NULL, applies
- * only to threads in that pool, otherwise round-robins between
- * all pools. Caller must ensure that mutual exclusion between this and
- * server startup or shutdown.
- *
- * Destroying threads relies on the service threads filling in
- * rqstp->rq_task, which only the nfs ones do. Assumes the serv
- * has been created using svc_create_pooled().
- *
- * Based on code that used to be in nfsd_svc() but tweaked
- * to be pool-aware.
- */
-int
-svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
+/* create new threads */
+static int
+svc_start_kthreads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
{
struct svc_rqst *rqstp;
struct task_struct *task;
struct svc_pool *chosen_pool;
- int error = 0;
unsigned int state = serv->sv_nrthreads-1;
int node;
- 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);
- }
-
- /* create new threads */
- while (nrservs > 0) {
+ do {
nrservs--;
chosen_pool = choose_pool(serv, pool, &state);
node = svc_pool_map_get_node(chosen_pool->sp_id);
rqstp = svc_prepare_thread(serv, chosen_pool, node);
- if (IS_ERR(rqstp)) {
- error = PTR_ERR(rqstp);
- break;
- }
+ if (IS_ERR(rqstp))
+ return PTR_ERR(rqstp);
__module_get(serv->sv_ops->svo_module);
task = kthread_create_on_node(serv->sv_ops->svo_function, rqstp,
node, "%s", serv->sv_name);
if (IS_ERR(task)) {
- error = PTR_ERR(task);
module_put(serv->sv_ops->svo_module);
svc_exit_thread(rqstp);
- break;
+ return PTR_ERR(task);
}
rqstp->rq_task = task;
@@ -763,15 +736,62 @@ svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
svc_sock_update_bufs(serv);
wake_up_process(task);
- }
+ } while (nrservs > 0);
+
+ return 0;
+}
+
+
+/* destroy old threads */
+static int
+svc_signal_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 */
- while (nrservs < 0 &&
- (task = choose_victim(serv, pool, &state)) != NULL) {
+ do {
+ task = choose_victim(serv, pool, &state);
+ if (task == NULL)
+ break;
send_sig(SIGINT, task, 1);
nrservs++;
+ } while (nrservs < 0);
+
+ return 0;
+}
+
+/*
+ * Create or destroy enough new threads to make the number
+ * of threads the given number. If `pool' is non-NULL, applies
+ * only to threads in that pool, otherwise round-robins between
+ * all pools. Caller must ensure that mutual exclusion between this and
+ * server startup or shutdown.
+ *
+ * Destroying threads relies on the service threads filling in
+ * rqstp->rq_task, which only the nfs ones do. Assumes the serv
+ * has been created using svc_create_pooled().
+ *
+ * Based on code that used to be in nfsd_svc() but tweaked
+ * to be pool-aware.
+ */
+int
+svc_set_num_threads(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);
}
- return error;
+ if (nrservs > 0)
+ return svc_start_kthreads(serv, pool, nrservs);
+ if (nrservs < 0)
+ return svc_signal_kthreads(serv, pool, nrservs);
+ return 0;
}
EXPORT_SYMBOL_GPL(svc_set_num_threads);
--
2.9.3
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 <[email protected]>
Fixes: bb6aeba736ba9 ("NFSv4.x: Switch to using svc_set_num_threads()...")
Signed-off-by: Trond Myklebust <[email protected]>
Cc: J. Bruce Fields <[email protected]>
---
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);
+
/*
* Called from a server thread as it's exiting. Caller must hold the "service
* mutex" for the service.
--
2.9.3
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 <[email protected]>
> Fixes: bb6aeba736ba9 ("NFSv4.x: Switch to using svc_set_num_threads()...")
> Signed-off-by: Trond Myklebust <[email protected]>
> Cc: J. Bruce Fields <[email protected]>
> ---
> 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
>
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 <[email protected]>
> Fixes: bb6aeba736ba9 ("NFSv4.x: Switch to using svc_set_num_threads()...")
> Signed-off-by: Trond Myklebust <[email protected]>
> Cc: J. Bruce Fields <[email protected]>
> ---
> 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
As the comments for svc_set_num_threads() said,
" Destroying threads relies on the service threads filling in
rqstp->rq_task, which only the nfs ones do. Assumes the serv
has been created using svc_create_pooled()."
If creating service through svc_create(), the svc_pool_map_put()
will be called in svc_destroy(), but the pool map isn't used.
So that, the reference of pool map will be drop, the next using
of pool map will get a zero npools.
[ 137.992130] divide error: 0000 [#1] SMP
[ 137.992148] Modules linked in: nfsd(E) nfsv4 nfs fscache fuse tun bridge stp llc ip_set nfnetlink vmw_vsock_vmci_transport vsock snd_seq_midi snd_seq_midi_event vmw_balloon coretemp crct10dif_pclmul crc32_pclmul ppdev ghash_clmulni_intel intel_rapl_perf joydev snd_ens1371 gameport snd_ac97_codec ac97_bus snd_seq snd_pcm snd_rawmidi snd_timer snd_seq_device snd soundcore parport_pc parport nfit acpi_cpufreq tpm_tis tpm_tis_core tpm vmw_vmci i2c_piix4 shpchp auth_rpcgss nfs_acl lockd(E) grace sunrpc(E) xfs libcrc32c vmwgfx drm_kms_helper ttm crc32c_intel drm e1000 mptspi scsi_transport_spi serio_raw mptscsih mptbase ata_generic pata_acpi [last unloaded: nfsd]
[ 137.992336] CPU: 0 PID: 4514 Comm: rpc.nfsd Tainted: G E 4.11.0-rc8+ #536
[ 137.992777] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[ 137.993757] task: ffff955984101d00 task.stack: ffff9873c2604000
[ 137.994231] RIP: 0010:svc_pool_for_cpu+0x2b/0x80 [sunrpc]
[ 137.994768] RSP: 0018:ffff9873c2607c18 EFLAGS: 00010246
[ 137.995227] RAX: 0000000000000000 RBX: ffff95598376f000 RCX: 0000000000000002
[ 137.995673] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9559944aec00
[ 137.996156] RBP: ffff9873c2607c18 R08: ffff9559944aec28 R09: 0000000000000000
[ 137.996609] R10: 0000000001080002 R11: 0000000000000000 R12: ffff95598376f010
[ 137.997063] R13: ffff95598376f018 R14: ffff9559944aec28 R15: ffff9559944aec00
[ 137.997584] FS: 00007f755529eb40(0000) GS:ffff9559bb600000(0000) knlGS:0000000000000000
[ 137.998048] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 137.998548] CR2: 000055f3aecd9660 CR3: 0000000084290000 CR4: 00000000001406f0
[ 137.999052] Call Trace:
[ 137.999517] svc_xprt_do_enqueue+0xef/0x260 [sunrpc]
[ 138.000028] svc_xprt_received+0x47/0x90 [sunrpc]
[ 138.000487] svc_add_new_perm_xprt+0x76/0x90 [sunrpc]
[ 138.000981] svc_addsock+0x14b/0x200 [sunrpc]
[ 138.001424] ? recalc_sigpending+0x1b/0x50
[ 138.001860] ? __getnstimeofday64+0x41/0xd0
[ 138.002346] ? do_gettimeofday+0x29/0x90
[ 138.002779] write_ports+0x255/0x2c0 [nfsd]
[ 138.003202] ? _copy_from_user+0x4e/0x80
[ 138.003676] ? write_recoverydir+0x100/0x100 [nfsd]
[ 138.004098] nfsctl_transaction_write+0x48/0x80 [nfsd]
[ 138.004544] __vfs_write+0x37/0x160
[ 138.004982] ? selinux_file_permission+0xd7/0x110
[ 138.005401] ? security_file_permission+0x3b/0xc0
[ 138.005865] vfs_write+0xb5/0x1a0
[ 138.006267] SyS_write+0x55/0xc0
[ 138.006654] entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 138.007071] RIP: 0033:0x7f7554b9dc30
[ 138.007437] RSP: 002b:00007ffc9f92c788 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 138.007807] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f7554b9dc30
[ 138.008168] RDX: 0000000000000002 RSI: 00005640cd536640 RDI: 0000000000000003
[ 138.008573] RBP: 00007ffc9f92c780 R08: 0000000000000001 R09: 0000000000000002
[ 138.008918] R10: 0000000000000064 R11: 0000000000000246 R12: 0000000000000004
[ 138.009254] R13: 00005640cdbf77a0 R14: 00005640cdbf7720 R15: 00007ffc9f92c238
[ 138.009610] Code: 0f 1f 44 00 00 48 8b 87 98 00 00 00 55 48 89 e5 48 83 78 08 00 74 10 8b 05 07 42 02 00 83 f8 01 74 40 83 f8 02 74 19 31 c0 31 d2 <f7> b7 88 00 00 00 5d 89 d0 48 c1 e0 07 48 03 87 90 00 00 00 c3
[ 138.010664] RIP: svc_pool_for_cpu+0x2b/0x80 [sunrpc] RSP: ffff9873c2607c18
[ 138.011061] ---[ end trace b3468224cafa7d11 ]---
Signed-off-by: Kinglong Mee <[email protected]>
---
fs/nfs/callback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 484bebc..0a21150 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -279,7 +279,7 @@ static struct svc_serv *nfs_callback_create_svc(int minorversion)
printk(KERN_WARNING "nfs_callback_create_svc: no kthread, %d users??\n",
cb_info->users);
- serv = svc_create(&nfs4_callback_program, NFS4_CALLBACK_BUFSIZE, sv_ops);
+ serv = svc_create_pooled(&nfs4_callback_program, NFS4_CALLBACK_BUFSIZE, sv_ops);
if (!serv) {
printk(KERN_ERR "nfs_callback_create_svc: create service failed\n");
return ERR_PTR(-ENOMEM);
--
2.9.3
T24gVGh1LCAyMDE3LTA0LTI3IGF0IDExOjA2ICswODAwLCBLaW5nbG9uZyBNZWUgd3JvdGU6DQo+
IA0KPiBUaGVyZSBpcyBvbmx5IG9uZSB1c2Ugb2Ygc3ZjX3NldF9udW1fdGhyZWFkcyB0aGF0IGJ5
IG5mc2QsDQo+IEknZCBsaWtlIHRvIGNoYW5nZSBzdmNfc2V0X251bV90aHJlYWRzIGFuZCB1cGRh
dGUgbmZzZCB0aGFuIGFkZCBhIG5ldw0KPiBmdW5jdGlvbi4NCg0KWW91IGNhbid0IHJlYWxseSBj
b21iaW5lIHRoZSB0d28gbWV0aG9kcy4gRWl0aGVyIHlvdSBjaG9vc2Ugc2lnbmFscyBvcg0KeW91
IGNob29zZSBrdGhyZWFkX3N0b3AoKS4gVGhlIHByb2JsZW0gaXMgdGhhdCBzaWduYWxzIHJlcXVp
cmUgdGhlDQp0aHJlYWQgdG8gYmUgYWJsZSB0byBwZXJzaXN0IHBhc3QgdGhlIG5mc2RfZGVzdHJv
eSgpICh3aGljaCBhZ2Fpbg0KZm9yY2VzIHRoaW5ncyBsaWtlIG5mc2QoKSBoYXZpbmcgdG8gdGFr
ZSBuZnNkX211dGV4KSwgb3IgeW91IGhhdmUgdG8NCm1ha2UgdGhpbmdzIHN5bmNocm9ub3VzLCBp
biB3aGljaCBjYXNlIGhhdmluZyBuZnNkKCkgdHJ5IHRvIHRha2UNCm5mc2RfbXV0ZXggY2F1c2Vz
IGRlYWRsb2Nrcy4NCg0KSU9XOiBpZiB0aGVyZSBpcyBsZWdhY3kgYmVoYXZpb3VyIGhlcmUgdGhh
dCByZXF1aXJlcyB0aGUgc2lnbmFsIG1ldGhvZCwNCnRoZW4ga25mc2QgY2Fubm90IGJlIGNvbnZl
cnRlZC4NCg0KPiBQcywNCj4gIk5GU3Y0LngvY2FsbGJhY2s6IENyZWF0ZSB0aGUgY2FsbGJhY2sg
c2VydmljZSB0aHJvdWdoDQo+IHN2Y19jcmVhdGVfcG9vbGVkIg0KPiBtdXN0IGJlIGluY2x1ZGUg
YmVmb3JlIHRoZSBmaXguIEkgd2lsbCByZXNlbmQgaXQgbGF0ZXIuDQo+IA0KT0suIFRoYW5rcyEN
Cg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgUHJp
bWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg==
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 <[email protected]>
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!
>
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 <[email protected]>
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.
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
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 <[email protected]>
> >
> > 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.