[PATCH] use kthread infrastructure in md

[PATCH] use kthread infrastructure in md

[Christoph Hellwig]

Switch MD to use the kthread infrastructure, to simplify the code and
get rid of tasklist_lock abuse in md_unregister_thread.  Long-term I
wonder whether workqueues wouldn't be a better choice than the
MD-specific thread wrappers for the lowlevel drivers.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/drivers/md/md.c
===================================================================
--- linux-2.6.orig/drivers/md/md.c	2005-08-09 19:28:16.000000000 +0200
+++ linux-2.6/drivers/md/md.c	2005-08-09 20:17:21.000000000 +0200
@@ -34,6 +34,7 @@
 
 #include <linux/module.h>
 #include <linux/config.h>
+#include <linux/kthread.h>
 #include <linux/linkage.h>
 #include <linux/raid/md.h>
 #include <linux/raid/bitmap.h>
@@ -2947,18 +2948,6 @@
 {
 	mdk_thread_t *thread = arg;
 
-	lock_kernel();
-
-	/*
-	 * Detach thread
-	 */
-
-	daemonize(thread->name, mdname(thread->mddev));
-
-	current->exit_signal = SIGCHLD;
-	allow_signal(SIGKILL);
-	thread->tsk = current;
-
 	/*
 	 * md_thread is a 'system-thread', it's priority should be very
 	 * high. We avoid resource deadlocks individually in each
@@ -2970,10 +2959,9 @@
 	 * bdflush, otherwise bdflush will deadlock if there are too
 	 * many dirty RAID5 blocks.
 	 */
-	unlock_kernel();
 
 	complete(thread->event);
-	while (thread->run) {
+	while (!kthread_should_stop()) {
 		void (*run)(mddev_t *);
 
 		wait_event_interruptible_timeout(thread->wqueue,
@@ -2986,11 +2974,8 @@
 		run = thread->run;
 		if (run)
 			run(thread->mddev);
-
-		if (signal_pending(current))
-			flush_signals(current);
 	}
-	complete(thread->event);
+
 	return 0;
 }
 
@@ -3007,11 +2992,9 @@
 				 const char *name)
 {
 	mdk_thread_t *thread;
-	int ret;
 	struct completion event;
 
-	thread = (mdk_thread_t *) kmalloc
-				(sizeof(mdk_thread_t), GFP_KERNEL);
+	thread = kmalloc(sizeof(mdk_thread_t), GFP_KERNEL);
 	if (!thread)
 		return NULL;
 
@@ -3024,8 +3007,8 @@
 	thread->mddev = mddev;
 	thread->name = name;
 	thread->timeout = MAX_SCHEDULE_TIMEOUT;
-	ret = kernel_thread(md_thread, thread, 0);
-	if (ret < 0) {
+	thread->tsk = kthread_run(md_thread, thread, mdname(thread->mddev));
+	if (IS_ERR(thread->tsk)) {
 		kfree(thread);
 		return NULL;
 	}
@@ -3035,21 +3018,9 @@
 
 void md_unregister_thread(mdk_thread_t *thread)
 {
-	struct completion event;
-
-	init_completion(&event);
-
-	thread->event = &event;
-
-	/* As soon as ->run is set to NULL, the task could disappear,
-	 * so we need to hold tasklist_lock until we have sent the signal
-	 */
 	dprintk("interrupting MD-thread pid %d\n", thread->tsk->pid);
-	read_lock(&tasklist_lock);
-	thread->run = NULL;
-	send_sig(SIGKILL, thread->tsk, 1);
-	read_unlock(&tasklist_lock);
-	wait_for_completion(&event);
+
+	kthread_stop(thread->tsk);
 	kfree(thread);
 }
 

Re: [PATCH] use kthread infrastructure in md

[Neil Brown]

On Tuesday August 9, hch@lst.de wrote:
> Switch MD to use the kthread infrastructure, to simplify the code and
> get rid of tasklist_lock abuse in md_unregister_thread.  Long-term I
> wonder whether workqueues wouldn't be a better choice than the
> MD-specific thread wrappers for the lowlevel drivers.
> 

Thanks.  This is definitely a step in the right direction.   However
I think it still needs a bit of work.
The old md_unregister_thread sent a signal to the thread so that if it
was in 'wait_event_interruptible_timeout', that call would complete.
However I cannot see how the new md_unregister_thread will interrupt
the wait_event_interruptible_timeout.
I'll look into it..


Thanks,
NeilBrown