[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-api
Subject:    Re: [PATCH] prctl: add PR_{GET,SET}_KILL_DESCENDANTS_ON_EXIT
From:       Oleg Nesterov <oleg () redhat ! com>
Date:       2018-11-30 10:33:30
Message-ID: 20181130103329.GB23670 () redhat ! com
[Download RAW message or body]

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.

[prev in list] [next in list] [prev in thread] [next in thread]