On 11/29, Jürg Billeter wrote: > > On Thu, 2018-11-29 at 13:34 +0100, Oleg Nesterov wrote: > > To me it would be more clean to call walk_process_tree(kill_descendant_visitor) > > unconditionally in find_new_reaper() right before "if (has_child_subreaper)", but > > then we will need to shift read_lock(tasklist) from walk_process_tree(). > > Yes, that's the reason why I added the call before the tasklist lock. > Let me know if you want me to move the read lock from > walk_process_tree() to PR_SET_CHILD_SUBREAPER (the only caller) > instead. I am fine either way. We can do this later, lets keep your patch simple. > > So I think the patch is mostly fine, the only problem I can see is that > > PR_SET_KILL_DESCENDANTS_ON_EXIT can race with PR_SET_CHILD_SUBREAPER, they both > > need to update the bits in the same word. > > Good point. I'll make it a regular bool instead of a bitfield for v2, Agreed, > unless you have another approach in mind to fix this. Well, I think that is_child_subreaper/has_child_subreaper and the new kill_descendants_on_exit should live in signal->flags, but we need some cleanups to make this possible, so I agree with the boolean. Oleg.
