kthread interface

[Attilio Rao]

2010/1/28 Giovanni Trematerra <giovanni.trematerra at gmail.com>:
> Hi all,
> There is a race in kthread_exit when all the threads of a kernel
> process exit at same time. I came up with a quick and dirty patch that
> resolve the issue at least in my test case.
>
> http://www.trematerra.net/patches/kthread_exit.diff
>
> Nonetheless I see space for improvement into kthread interface.
> At present, with kproc_kthread_add you could have a kernel process
> without a main thread and that seems to me only a way to logical
> grouping threads and pretty useless.
> I propose to remove kproc_kthread_add and don't let kthread_exit call
> kproc_exit on the last exiting thread but demand user to handle
> process termination.
> If you need kernel threads but no reason to have a kernel process with
> a main thread that acts as a coordinator you can attach them to proc0
> by kthread_add passing NULL for (struct proc *) argument.

The patch is right but for a different reason than what you expose.
What really happens is that kproc_kthread_add() has mostly an evil semantic.
In the common case you define a precise callback, but you are not
going to know if the callback you are offering will end up being the
first thread (thus 'reconduited' to kproc_create()) or just one of the
others (thus added via kthread_add()).
The problem is that in the former case the callback should close with
a kproc_exit() call, while on the latter an ideal thread_exit() should
happen.
As long as the callback can't know in which case it is, the
kthread_exit() (used in this context, but as I can see, not in all of
them) needs to take care of this problem. This is only possible,
however, if the callbacks also ensure (and so far I don't see any
similar safety belt somewhere) that the threads adding is correctly
drained when the threads are going to die. This is a race the
primitive itself can't handle (aka: the count of threads within the
kproc can't grow up when the first thread start dying) because it
needs of external help.

The normal proc/thread working model is quite different because the
process is created with an explicit interface (fork1()) and it is
closed by an explicit interface (eventually reused, but exit1() takes
explicitly place in the interested paths) and the threads are added
with a explicit logic (via thr interfaces). kproc_kthread_add() does
complicate all this simple logic, of course.

As a first band-aid your patch is ok, but what I really would like is
that the whole semantic gets sanitized and gets a better logic (in
other words: explicit ordering of the kproc/kthreads creation and
destruction).

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein