Messages in this thread | | | Date | Tue, 16 Jun 2015 22:03:52 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] exit: Clarify choice of new parent in forget_original_parent() |
| |
forgot to mention,
On 06/16, Oleg Nesterov wrote: > > On 06/16, Kirill Tkhai wrote: > > > > + * child_reaper doesn't have children after zap_pid_ns_processes(), > > + * therefore it can't enter this function. > > + */ > > + BUG_ON(child_reaper == father); > > Yes, we can add this BUG_ON(). But please see the comments in > zap_pid_ns_processes(). We can change zap_pid_ns_processes() so that > it returns with non-empty ->children list due to EXIT_DEAD children. > > Unlikely we will actually do this, at least soon, so I won't argue > with this BUG_ON(). > > But. In this case it would be better to add it into forget_original_parent(), > > reaper = find_new_reaper(...); > BUG_ON(reaper == father);
because this way:
1. This BUG_ON() will still be valid even if we actually change zap_pid_ns_processes() to return with EXIT_DEAD children
2. If we really want this sanity check, we should not tie it to ->child_reaper case.
OTOH. If for some reason you want to check ->child_reaper only, then you should probably do this right after list_empty(&father->children) check, or at least before find_alive_thread(). Because otherwise it looks confusing, it looks as if "child_reaper == father" is only wrong if find_alive_thread(father) fails.
Oleg.
| |