Re: [PATCH 1/2] fix EH thread teardown

[Rolf Eike Beer <eike-kernel@xxxxxxxxx>]

Christoph Hellwig wrote:
>As Rolf Eike Beer noted we might not actually get to the
>kthread_should_stop() because we are waiting in the semaphore forever.
>I didn't get a rmmod hang because of this, but my instrumentation showed
>the thread defitiyly didn't exit.
>
>So make sure to wake the EH thread before the kthread_stop, and to plug
>the reaming race check kthead_should_stop() a second time just before
>calling down_interruptible().

Ok, we looked a bit closer on all this. http://lwn.net/Articles/65178/ tells 
that ktread_stop() will not send a signal, so the down_interruptible() will 
never return on kthread_stop(). But calling up() directly before 
kthread_stop() adds a race condition: if reschedule happens right after the 
up() we've won nothing and it will hang again.

Using this kind of semaphore for thread wakeup looks more dangerous everytime 
I think on it. down_interruptible() may be interrupted by someone with a 
signal and return without the semaphore locked. Better would be something 
like

i = down_interruptible();

if (kthread_should_stop())
	break;

if (i)
	continue;

Or we have to block all signals, then we can just ignore the _interruptible at 
all. What about something like this instead?

while(!kthread_should_stop()) {
	if (!down_trylock(&sem)) {
		if (kthread_should_stop())
			break;
		yield();
		continue;
	}

	/* something */
}

Eike

Attachment: pgpMCpR4KqFv5.pgp
Description: PGP signature