Messages in this thread | | | Subject | Re: [PATCH] exit: Clarify choice of new parent in forget_original_parent() | From | Kirill Tkhai <> | Date | Wed, 17 Jun 2015 20:24:24 +0300 |
| |
В Вт, 16/06/2015 в 22:03 +0200, Oleg Nesterov пишет: > 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.
Sure, it's more logical. Thanks, Oleg.
| |