cvs commit: src/sys/compat/ndis subr_ntoskrnl.c

[John Baldwin]

On Sunday 22 July 2007 04:53:28 pm Andrew Thompson wrote:
> thompsa     2007-07-22 20:53:28 UTC
> 
>   FreeBSD src repository
> 
>   Modified files:
>     sys/compat/ndis      subr_ntoskrnl.c 
>   Log:
>   ndis will signal the kthread to exit and then sleep on the proc pointer to
>   be woken up by kthread_exit. This is racey and in some cases the kthread 
will
>   exit before ndis gets around to sleep so it will be stuck indefinitely. 
This
>   change reuses the kq_exit variable to indicate that the thread has gone 
and
>   will loop on tsleep with a timeout waiting for it. If the kthread has 
already
>   exited then it will not sleep at all.

As long as you use a lock you are ok.  That is:

foo_detach()
{

	mtx_lock(&lock);
	please_die = 1;
	msleep(&proc, &lock, ..., 0);
	mtx_unlock(&lock);
}

foo_main()
{

	mtx_lock(&lock);
	while (!please_die) {
		do_stuff();
	}
	mtx_unlock(&lock);
	kthread_exit(0);
}

works fine.  If you try to do this:

foo_detach()
{

	mtx_lock(&lock);
	please_die = 1;
	while (!dead_yet)
		msleep(&proc, &lock, ... , hz/10);
	mtx_unlock(&lock);
}

foo_main()
{

	mtx_lock(&lock);
	while (!please_die) {
		do_stuff();
	}
	dead_yet = 1;
	mtx_unlock(&lock);
	kthread_exit(0);
}

and foo_main() can be unloaded (it's part of a module) then you are still 
racey and can panic on kldunload if you foo_main() is preempted after the 
mtx_unlock() but before the kthread_exit() and foo_detach() completes and 
returns to kldunload() which unmaps the module.  I think you didn't make the 
race worse though, as the old code was missing the lock and only used 
tsleep() before.

-- 
John Baldwin