lkml.org 
[lkml]   [2006]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] kthread conversion: convert ieee1394 from kernel_thread
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-06-17 20:51    [W:0.091 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site