Re: [PATCH] kthread conversion: convert ieee1394 from kernel_thread

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Christoph Hellwig wrote on 2006-06-17:
Below is a draft patch to convert it to the kthread API and replace the
reset_sem with a simple wake_up_process scheme.  I removed the down_trylock
loop there which I think should be fine because we get the wakeup again
ASAP, but please double-check.
[...]
[in nodemgr_host_thread(), top of event loop]
@@ -1579,15 +1573,14 @@
 		unsigned int generation = 0;
 		int i;
- if (down_interruptible(&hi->reset_sem) ||
-		    down_interruptible(&nodemgr_serialize)) {
+		if (down_interruptible(&nodemgr_serialize)) {

This won't work. "down_interruptible(&hi->reset_sem)" was there to put the thread to sleep after it did its work, until the next bus reset. Now the event loop would be entered continuously without an actual bus reset event.

(nodemgr_serialize is just a mutex disguised as a semaphore which prevents multiple nodemgr host threads to enter their event loop concurrently.)

 			if (try_to_freeze())
 				continue;
 			printk("NodeMgr: received unexpected signal?!\n" );
 			break;
 		}
- if (hi->kill_me) {
+		if (kthread_should_stop()) {
 			up(&nodemgr_serialize);
 			break;
 		}
@@ -1608,13 +1601,8 @@
 			 * returning bogus data. */
 			generation = get_hpsb_generation(host);
- /* If we get a reset before we are done waiting, then
-			 * start the the waiting over again */
-			while (!down_trylock(&hi->reset_sem))
-				i = 0;
[...]

Another minor issue: This check cannot be removed without replacement. However we could implement this check easily without a counting semaphore. (We could check the bus generation before and after the sleep.)

I will try to rework this patch and split it into more patches: One which converts nodemgr to the kthread API but keeps the reset_sem, one patch which gets rid of the counting semaphore reset_sem, one patch which converts nodemgr_serialize to a mutex.

BTW, it may be possible to remove nodemgr_serialize too. There are other exclusion mechanisms in nodemgr.c which should already prevent most if not all undesirable concurrency, notably the semaphores dev->bus->subsys.rwsem and class->subsys.rwsem.
--
Stefan Richter
-=====-=-==- -==- =---=
http://arcgraph.de/sr/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux