Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     Oleg Nesterov <oleg@redhat.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Wen Yang <wen.yang99@zte.com.cn>, majiang <ma.jiang@zte.com.cn>
References: <874lgo5xdg.fsf@xmission.com>
        <CA+55aFw_CTTRTZfUf1e9yg04X_djG3q7HrCG-XqnyQorpKjaTQ@mail.gmail.com>
        <87fu084cxj.fsf@xmission.com>
        <CA+55aFxUc3ttEAuRS7GKs9aTjTFw1poE80DASFSXudVQCuWpHg@mail.gmail.com>
        <87a7qg4bb3.fsf_-_@xmission.com>
        <CA+55aFxe+Ku78ZEvd_F2TD_3gTmLXw0kzo4HDE-ngkYfSHQfPw@mail.gmail.com>
        <87pnzc2upf.fsf@xmission.com> <87k1pk2cj9.fsf_-_@xmission.com>
        <20180726134143.GB32718@redhat.com> <87pnza6ou2.fsf@xmission.com>
        <20180726155546.GA467@redhat.com>
Date:   Thu, 09 Aug 2018 01:19:26 -0500
In-Reply-To: <20180726155546.GA467@redhat.com> (Oleg Nesterov's message of
        "Thu, 26 Jul 2018 17:55:47 +0200")
Message-ID: <87600k2hap.fsf@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH v3 20/20] signal: Don't restart fork when signals come in.
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Oleg Nesterov <oleg@redhat.com> writes:

> On 07/26, Eric W. Biederman wrote:
>>
>> Are the earlier patches looking ok to you?
>
> I obviously like 1-15.
>
> "[PATCH 16/20] fork: Move and describe why the code examines PIDNS_ADDING"
> is "interesting". I mean it is fine, but at the end of this series it doesn't
> matter what we check first, PIDNS_ADDING or fatal_signal_pending() - restart
> is not possible in both cases.
>
>
> As for 17-20... Yes I am biased. But I still think the simple approach I tried
> to propose from the very beginning is better. At least simpler, in that you do
> not need to worry about all these special cases/reasons for signal_pending().

I think worrying about them all now results in a future where we don't
have to worry about reasons why we can't let fork continue.  Giving a
better progress guarantee.  Which ultimately should be more maintainable
going forward.

> And you can not imagine how much I hate "[PATCH 19/20] fork: Have new threads
> join on-going signal group stops" ;) Because I spent HOURS looking at this trivial
> patch and I am still not sure...
>
> To clarify, the CLONE_THREAD with JOBCTL_STOP_PENDING case is simple, I am mostly
> worried about JOBCTL_TRAP_STOP/etc with or without CLONE_THREAD, this adds some
> subtle changes but unfortunately I failed to find something wrong so I
> can't argue

I can understand taking a hard look at JOBCTL_TRAP_STOP especially as it
gets mixed in with the multi-task (whole process) stop handling when at
least one of the tasks of a process are being ptraced.  To make certain
I understood your concern I took a second look at it myself.

The ptrace actions are defined to only affect a single task, and except
for multi-task stop handling all of the jobctl bits are used for ptrace
actions.  So I don't see how there is anything we could possibly miss
in the jobctl bits.

Eric