2001-03-22 11:51:12

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: kernel_thread vs. zombie

Hi !

I'm changing the ADB bus reset & probe code to be in a kernel threads
that is created when a bus reset is triggered and that dies of it's own
death.

Everything is fine when the bus reset is triggered during kernel init as
my thread is a child of init. However, when created as a result of an
ioctl, the thread becomes a zombie as it's a child of the process who
caused the ioctl (typically when entering sleep mode).

How do I force a kernel thread to always be a child of init and never
become a zombie ?

I do call daemonize at the beginning of the thread (as it won't do
anything with files, signals or whatever), but that doesn't seem to be enough.

Reagrds,
Ben.




2001-03-22 14:34:10

by Martin Frey

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

Hi,

>How do I force a kernel thread to always be a child of init and never
>become a zombie ?
>
>I do call daemonize at the beginning of the thread (as it won't do
>anything with files, signals or whatever), but that doesn't
>seem to be enough.
>
Have a look at:
http://www.scs.ch/~frey/linux/kernelthreads.html
I have an example there that starts and stops kernel threads
from init_module and never produced a zombie.
I use the same code also to start threads from ioctl and it
works for me. I tested it on UP and SMP, Intel and Alpha,
2.2.18 and 2.4.2.

Regards,

Martin

--
Supercomputing Systems AG email: [email protected]
Martin Frey web: http://www.scs.ch/~frey/
at Compaq Computer Corporation phone: +1 603 884 4266
ZKO2-3P09, 110 Spit Brook Road, Nashua, NH 03062

2001-03-22 15:09:30

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

>Have a look at:
>http://www.scs.ch/~frey/linux/kernelthreads.html
>I have an example there that starts and stops kernel threads
>from init_module and never produced a zombie.
>I use the same code also to start threads from ioctl and it
>works for me. I tested it on UP and SMP, Intel and Alpha,
>2.2.18 and 2.4.2.

Thanks !

Could you explain me a bit why you need the lock_kernel ? My probe
thread is already protected by some atomic ops, but I'm considering
changing them to semaphores. Is there any need for the bkl to be taken
when calling daemonize or is this just for your own syncronisation needs ?

I don't think you do more than what I currently do to prevent the
zombie (except for the daemonize call, I don't see you changing anything
about the parent thread or whatever).

At first I though daemonize() would do the trick, but I still see
zombies on my tests. I'm running UP now so I don't since my lack
of lock_kernel() could explain it.

Ben.



2001-03-22 17:23:37

by Martin Frey

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

Hi,
>> http://www.scs.ch/~frey/linux/kernelthreads.html

>Could you explain me a bit why you need the lock_kernel ? My probe
>thread is already protected by some atomic ops, but I'm considering
>changing them to semaphores. Is there any need for the bkl to be taken
>when calling daemonize or is this just for your own
>syncronisation needs ?
>
The stuff done in daemonize() and the exit_files could need
the kernel lock. At least on some 2.2.x version it does,
I did not check whether it is still needed on 2.4.

On stop of the thread I need the big kernel lock to make
sure the kernel thread exited (everything really done
from my up() till the thread is in zombie state) before
I unload the module. The comment in the code should explain
in.

Note that the threads itself do not run with the kernel lock
held. After setting everything up the make an unlock.

>I don't think you do more than what I currently do to prevent the
>zombie (except for the daemonize call, I don't see you
>changing anything
>about the parent thread or whatever).
>
No, changing the parent thread is done by daemonize().

>At first I though daemonize() would do the trick, but I still see
>zombies on my tests. I'm running UP now so I don't since my lack
>of lock_kernel() could explain it.
>
I needed the exit_files() on 2.2.x to prevent the zombies. On 2.4
daemonize() does that as well.

In the kill_thread() call I actually wait till the thread has
terminated. This is assured with the semaphore and the big kernel
lock. I did have problems when I just sent the termination signal
and unloaded the module, but since I really block there I did not
see zombies. I stress-tested the module for several days in a loop
- no zombies at all.

Regards,

Martin

2001-03-22 18:49:07

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

>The stuff done in daemonize() and the exit_files could need
>the kernel lock. At least on some 2.2.x version it does,
>I did not check whether it is still needed on 2.4.

Well, I don't really plan to backport this to 2.2.x. I'll
try to see if my problem is related to the lack of kernel
lock, or maybe I have just something else wrong.

>On stop of the thread I need the big kernel lock to make
>sure the kernel thread exited (everything really done
>from my up() till the thread is in zombie state) before
>I unload the module. The comment in the code should explain
>in.

Ok. I don't need that as I'm not in a module, no chances I ever
get unloaded. At least not in 2.4. Making ADB and all the controllers
and device drivers in modules would be an interesting exercise with
module dependencies ;)

>Note that the threads itself do not run with the kernel lock
>held. After setting everything up the make an unlock.

Ok. Well, I just have an atomic flag test&set'ed before starting the
bus reset, and released at the end of the thread. No need to make sure
the previous one is really dead before starting a new one. I could
benefit from semaphores when starting it since if it's already running,
I just loop scheduling waiting for the lock bit to be available. But
that case will almost never happen in real life. ADB probes are quite
rare.

Many thanks for your help,

I'll see what's wrong in my code ;)

ben.



2001-03-22 19:51:47

by Martin Frey

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

>>The stuff done in daemonize() and the exit_files could need
>>the kernel lock. At least on some 2.2.x version it does,
>>I did not check whether it is still needed on 2.4.
>
>Well, I don't really plan to backport this to 2.2.x. I'll
>try to see if my problem is related to the lack of kernel
>lock, or maybe I have just something else wrong.
>
daemonize() makes calls that are all protected with the
big kernel lock in do_exit(). All usages of daemonize have
the big kernel lock held. So I guess it just needs it.

Please let me know whether you have success if it makes
a difference with having it held.

Martin

2001-03-22 23:35:39

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

>daemonize() makes calls that are all protected with the
>big kernel lock in do_exit(). All usages of daemonize have
>the big kernel lock held. So I guess it just needs it.
>
>Please let me know whether you have success if it makes
>a difference with having it held.

With a bit more experiments, I have this behaviour:

(I hold the kerne lock, daemonize(), and release the kernel lock, then do
my probe thing which takes a few seconds, and let the thread die by itself)

- When started during boot (low PID (9)) It becomes a zombie
- When started from a process that quits after sending the ioctl,
it is correctly "garbage collected".
- When started from a process that stays around, it becomes a zombie too

So something is not working, or I'm missing something obvious, or whatever...

Any clue ?

Ben.




2001-03-23 00:07:40

by Andrew Morton

[permalink] [raw]
Subject: Re: kernel_thread vs. zombie

Benjamin Herrenschmidt wrote:
>
> >daemonize() makes calls that are all protected with the
> >big kernel lock in do_exit(). All usages of daemonize have
> >the big kernel lock held. So I guess it just needs it.
> >
> >Please let me know whether you have success if it makes
> >a difference with having it held.
>
> With a bit more experiments, I have this behaviour:
>
> (I hold the kerne lock, daemonize(), and release the kernel lock, then do
> my probe thing which takes a few seconds, and let the thread die by itself)
>
> - When started during boot (low PID (9)) It becomes a zombie
> - When started from a process that quits after sending the ioctl,
> it is correctly "garbage collected".
> - When started from a process that stays around, it becomes a zombie too
>
> So something is not working, or I'm missing something obvious, or whatever...
>
> Any clue ?

Take a look at kernel/kmod.c:call_usermodehelper(). Copy it.

This will make your thread a child of keventd. This takes
care of things like chrootedness, uids, cwds, signal masks,
reaping children, open files, and all the other crud which
you can accidentally inherit from your caller.

something like:

struct my_struct
{
struct tq_struct tq;
void (*function)(void *);
struct semaphore sem;
<other stuff>
};

/* keventd runs this */
void helper(void *data)
{
struct my_struct *my_ptr = data;

kernel_thread(my_ptr->function, my_ptr, CLONE_FLAGS|SIGCHLD);
}

start_thread(struct my_struct *my_ptr)
{
my_ptr->tq.sync = 0;
INIT_LIST_HEAD(&my_ptr->tq.list);
my_ptr->routine = helper;
my_ptr->data = my_ptr;
schedule_task(&my_ptr->tq);
}

2001-03-23 01:59:04

by Martin Frey

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

>> - When started during boot (low PID (9)) It becomes a zombie
>> - When started from a process that quits after sending the ioctl,
>> it is correctly "garbage collected".
>> - When started from a process that stays around, it becomes
>> a zombie too

>Take a look at kernel/kmod.c:call_usermodehelper(). Copy it.
>
>This will make your thread a child of keventd. This takes
>care of things like chrootedness, uids, cwds, signal masks,
>reaping children, open files, and all the other crud which
>you can accidentally inherit from your caller.
>
So depending on the state of the caller daemonize() will not really
put us into the background as we want. With being created from
keventd we inherit a state as we'd like to have in a kernel thread.
Did I get it right?
I will change my example and test that.

Thanks,

Martin

2001-03-23 02:14:14

by Andrew Morton

[permalink] [raw]
Subject: Re: kernel_thread vs. zombie

Martin Frey wrote:
>
> >> - When started during boot (low PID (9)) It becomes a zombie
> >> - When started from a process that quits after sending the ioctl,
> >> it is correctly "garbage collected".
> >> - When started from a process that stays around, it becomes
> >> a zombie too
>
> >Take a look at kernel/kmod.c:call_usermodehelper(). Copy it.
> >
> >This will make your thread a child of keventd. This takes
> >care of things like chrootedness, uids, cwds, signal masks,
> >reaping children, open files, and all the other crud which
> >you can accidentally inherit from your caller.
> >
> So depending on the state of the caller daemonize() will not really
> put us into the background as we want.

Well, kernel_thread() will put you in the background, in the
sense that it creates an async thread. But you inherit
heaps of stuff from the parent. daemonize() cleans up
some of those things, but it can't clean up everything.

Kernel threads *need* to run in a well-understood and
sensible environment. We went through a lot of fun late
last year when there was a sudden proliferation of kernel
threads and quite a few things were subtly broken.

Things like kernel threads blocking signals because that's
what their user-space parent happened to do. Things like
user-space applications receiving a surprise SIGCHLD from
the kernel as a consequence of some system call which they
happened to have executed some while beforehand.

One approach would be to tromp through your task state setting
everything back where you want it. That's quite complex. Plus
there's the issue of who reaps the thread when it exits.

So I think it's reasonable to use keventd as `kinit', if you like.
Something which knows how to launch and reap kernel daemons, and
which provides a known environment to them.

A kernel API function (`kernel_daemon'?) which does all this
boilerplate is needed, I think.

-

2001-03-23 14:57:09

by Martin Frey

[permalink] [raw]
Subject: RE: kernel_thread vs. zombie

>So I think it's reasonable to use keventd as `kinit', if you like.
>Something which knows how to launch and reap kernel daemons, and
>which provides a known environment to them.
>
>A kernel API function (`kernel_daemon'?) which does all this
>boilerplate is needed, I think.
>
I completely agree. I'll be on a trip for the next two weeks, but
after that I can write a small example how it would look. I'll mail
again when it is ready.

Thanks, Martin