Re: please revert kthread from loop.c

[Andrew Morton <akpm@osdl.org>]

On Tue, 11 Jul 2006 22:26:47 -0500
"Serge E. Hallyn" <serue@us.ibm.com> wrote:

> > If so, this should plug it.  The same race is not possible against the
> > loop_set_fd() wakeup because the thread isn't running at that stage, yes?
> 
> Right, it's not yet running at loop_set_fd().  However what about
> kthread_stop() called from loop_clr_fd()?  Unfortunately fixing
> that seems hairy.  Need to think about it...

Yes, there does seem to be a little race there.

I think it would be sufficient to do


diff -puN drivers/block/loop.c~a drivers/block/loop.c
--- a/drivers/block/loop.c~a
+++ a/drivers/block/loop.c
@@ -602,7 +602,8 @@ static int loop_thread(void *data)
 		}
 		__set_current_state(TASK_INTERRUPTIBLE);
 		spin_unlock_irq(&lo->lo_lock);
-		schedule();
+		if (lo->state != Lo_rundown)
+			schedule();
 	}
 
 	return 0;
@@ -888,12 +889,11 @@ static int loop_clr_fd(struct loop_devic
 	if (filp == NULL)
 		return -EINVAL;
 
+	kthread_stop(lo->lo_thread);
 	spin_lock_irq(&lo->lo_lock);
 	lo->lo_state = Lo_rundown;
 	spin_unlock_irq(&lo->lo_lock);
 
-	kthread_stop(lo->lo_thread);
-
 	lo->lo_backing_file = NULL;
 
 	loop_release_xfer(lo);
_

where the tweak to loop_clr_fd() is just there to prevent loop_thread()
from going into a very brief busyloop.

I'm not sure why it's all so tricky in there, really.  Loop is doing a
pretty conventional stop, wakeup, stick-things-on-lists operation and we do
that all over the kernel using pretty well-understood idioms.  But for some
reason, loop is all difficult about it.  I wonder why.  hm.