On Sunday June 22, jlayton@redhat.com wrote:Let's do it? diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index 941041f..914e579 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -36,12 +36,7 @@ #define NFSDDBG_FACILITY NFSDDBG_SVC -/* these signals will be delivered to an nfsd thread - * when handling a request - */ -#define ALLOWED_SIGS (sigmask(SIGKILL)) -/* these signals will be delivered to an nfsd thread - * when not handling a request. i.e. when waiting +/* These signals can be used to shutdown an nfsd thread. */ #define SHUTDOWN_SIGS (sigmask(SIGKILL) | sigmask(SIGHUP) | sigmask(SIGINT) | sigmask(SIGQUIT)) /* if the last thread dies with SIGHUP, then the exports table is @@ -396,7 +391,7 @@ nfsd(struct svc_rqst *rqstp) { struct fs_struct *fsp; int err; - sigset_t shutdown_mask, allowed_mask; + sigset_t shutdown_mask; /* Lock module and set up kernel thread */ lock_kernel(); @@ -415,7 +410,8 @@ nfsd(struct svc_rqst *rqstp) current->fs->umask = 0; siginitsetinv(&shutdown_mask, SHUTDOWN_SIGS); - siginitsetinv(&allowed_mask, ALLOWED_SIGS); + /* Block all but the shutdown signals */ + sigprocmask(SIG_SETMASK, &shutdown_mask, NULL); nfsdstats.th_cnt++; @@ -435,8 +431,6 @@ nfsd(struct svc_rqst *rqstp) * The main request loop */ for (;;) { - /* Block all but the shutdown signals */ - sigprocmask(SIG_SETMASK, &shutdown_mask, NULL); /* * Find a socket with data available and call its @@ -452,9 +446,6 @@ nfsd(struct svc_rqst *rqstp) /* Lock the export hash tables for reading. */ exp_readlock(); - /* Process request with signals blocked. */ - sigprocmask(SIG_SETMASK, &allowed_mask, NULL); - svc_process(rqstp); /* Unlock export hash tables */ Oh, good. I must have been looking at an old patch. One the one hand, kthreads are expected to not block and to check for kthread_should_stop very often, so this wouldn't be a problem. On the other hand, expectations like this are quickly invalidated, and the code probably should be fixed. I suspect we could do without adding anything to task_struct. Instead of just one kthread_stop_info, have a linked list, one entry for each thread being stopped at the moment. kthread_stop adds to the list, sets PF_EXITING (I hope that is safe) and wakes the process. kthread_should_stop tests PF_EXITING. When kthread() finishes, it does a linear search (the list should be short) and calls complete and the relevant .done. Something like this? NeilBrown diff --git a/kernel/kthread.c b/kernel/kthread.c index bd1b9ea..107290a 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -39,12 +39,13 @@ struct kthread_stop_info struct task_struct *k; int err; struct completion done; + struct kthread_stp_info *next; }; /* Thread stopping is done by setthing this var: lock serializes * multiple kthread_stop calls. */ static DEFINE_MUTEX(kthread_stop_lock); -static struct kthread_stop_info kthread_stop_info; +static struct kthread_stop_info *kthread_stop_info; /** * kthread_should_stop - should this kthread return now? @@ -55,7 +56,7 @@ static struct kthread_stop_info kthread_stop_info; */ int kthread_should_stop(void) { - return (kthread_stop_info.k == current); + return test_bit(PF_EXITING, ¤t.flags); } EXPORT_SYMBOL(kthread_should_stop); @@ -80,8 +81,17 @@ static int kthread(void *_create) /* It might have exited on its own, w/o kthread_stop. Check. */ if (kthread_should_stop()) { - kthread_stop_info.err = ret; - complete(&kthread_stop_info.done); + struct kthread_stop_info **ksip; + mutex_lock(&kthread_stop_lock); + for (ksip = &kthread_stop_info ; *ksip ; ksip = &(*ksip)->next) + if ((*ksip)->k == current) + break; + *ksip = (*ksip)->next; + (*ksip)->next = NULL; + mutex_unlock(&kthread_stop_lock); + + ksi->err = ret; + complete(&ksi->done); } return 0; } @@ -199,26 +209,20 @@ EXPORT_SYMBOL(kthread_bind); int kthread_stop(struct task_struct *k) { int ret; + struct kthread_stop_info ksi; mutex_lock(&kthread_stop_lock); - /* It could exit after stop_info.k set, but before wake_up_process. */ - get_task_struct(k); + init_completion(&ksi->done); + ksi->k = k; - /* Must init completion *before* thread sees kthread_stop_info.k */ - init_completion(&kthread_stop_info.done); - smp_wmb(); - - /* Now set kthread_should_stop() to true, and wake it up. */ - kthread_stop_info.k = k; + set_bit(PF_EXITING, &k->flags); wake_up_process(k); - put_task_struct(k); + mutex_unlock(&kthread_stop_lock); - /* Once it dies, reset stop ptr, gather result and we're done. */ - wait_for_completion(&kthread_stop_info.done); - kthread_stop_info.k = NULL; + /* Once it dies gather result and we're done. */ + wait_for_completion(&ksi->done); ret = kthread_stop_info.err; - mutex_unlock(&kthread_stop_lock); return ret; } --
| Greg Kroah-Hartman | [PATCH 008/196] Chinese: add translation of volatile-considered-harmful.txt |
| Tarkan Erimer | Re: Dual-Licensing Linux Kernel with GPL V2 and GPL V3 |
| Greg KH | Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in |
| Andrew Morton | -mm merge plans for 2.6.23 |
git: | |
| Gerrit Renker | [PATCH 27/37] dccp: Integration of dynamic feature activation - part 2 (server side) |
| Herbert Xu | Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock(). |
| David Miller | [GIT]: Networking |
| Rémi Denis-Courmont | [PATCH 01/14] Phonet global definitions |
