Re: [PATCH] Add module_kernel_thread for threads that live in modules.

[Martin Diehl]

On Mon, 16 Jun 2003, Rusty Russell wrote:

> > You mean your cleanup_thread would block for completion of the keventd 
> > stuff? Ok, this would work. But then, when calling cleanup_thread, f.e. we 
> > must not hold any semaphore which might be acquired by _any_ other work 
> > scheduled for keventd or we might end in deadlock (like the rtnl+hotplug 
> > issue we had seen recently).
> 
> I think we're talking across each other: take a look at the existing
> kernel/kmod.c __call_usermodehelper to see how we wait at the moment.

Maybe talking across each other... what I meant is some deadlock like 
this (IMHO possible both on UP and SMP):

rmmod (f.e.)			keventd			somewhere else

down(&some_sem)
cleanup_thread()
	.
	.						schedule_work(w1)
	.
	.		w1 (queued, or maybe running):
	.			down(&some_sem);
	.			...
	.			up(&some_sem);
	.
schedule_work(w2)
	.
	.		w2 (queued behind w1)
	.			should_die = 1
	.			sys_wait4()
	.			complete(thread_exit)
	.
/* some_sem still hold */
wait_for_completion(thread_exit)

Next time we schedule keventd w1 will be executed first which wants to 
acquire some_sem which is still hold by the rmmod-thread - which in turn 
blocks for completion of w2 which is queued behind w1 -> deadlock.

The point is the queueing in keventd combined with stuff waiting for 
keventd-completion could create some possibilities for lock order 
violation which are at least not very obvious.

IMHO cleanup_thread would be something one MUST NOT call with any lock 
hold, not even a semaphore if it might get acquired anywhere else in 
keventd-context.

Thanks.
Martin