Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   "Eric W. Biederman" <ebiederm@xmission.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     rjw@rjwysocki.net, oleg@redhat.com, mingo@kernel.org,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, mgorman@suse.de, bigeasy@linutronix.de,
        Will Deacon <will@kernel.org>, linux-kernel@vger.kernel.org,
        tj@kernel.org, linux-pm@vger.kernel.org
In-Reply-To: <20220421200721.GF2762@worktop.programming.kicks-ass.net> (Peter
        Zijlstra's message of "Thu, 21 Apr 2022 22:07:21 +0200")
References: <20220421150248.667412396@infradead.org>
        <20220421150655.001952823@infradead.org>
        <878rrys5yj.fsf@email.froward.int.ebiederm.org>
        <20220421195551.GO2731@worktop.programming.kicks-ass.net>
        <20220421200721.GF2762@worktop.programming.kicks-ass.net>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
Date:   Fri, 22 Apr 2022 10:52:01 -0500
Message-ID: <87levxnmji.fsf@email.froward.int.ebiederm.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH v2 5/5] freezer,sched: Rewrite core freezer logic
Precedence: bulk

Peter Zijlstra <peterz@infradead.org> writes:

> On Thu, Apr 21, 2022 at 09:55:51PM +0200, Peter Zijlstra wrote:
>> On Thu, Apr 21, 2022 at 12:26:44PM -0500, Eric W. Biederman wrote:
>> > Peter Zijlstra <peterz@infradead.org> writes:
>> > 
>> > > --- a/kernel/ptrace.c
>> > > +++ b/kernel/ptrace.c
>> > > @@ -288,7 +288,7 @@ static int ptrace_check_attach(struct ta
>> > >  	}
>> > >  	__set_current_state(TASK_RUNNING);
>> > >  
>> > > -	if (!wait_task_inactive(child, TASK_TRACED) ||
>> > > +	if (!wait_task_inactive(child, TASK_TRACED|TASK_FREEZABLE) ||
>> > >  	    !ptrace_freeze_traced(child))
>> > >  		return -ESRCH;
>> > 
>> > Do we mind that this is going to fail if the child is frozen
>> > during ptrace_check_attach?
>> 
>> Why should this fail? wait_task_inactive() will in fact succeed if it is
>> frozen due to the added TASK_FREEZABLE and some wait_task_inactive()
>> changes elsewhere in this patch.
>
> These:

I had missed that change to wait_task_inactive.

Still that change to wait_task_inactive fundamentally depends upon the
fact that we don't care about the state we are passing into
wait_task_inactive.  So I think it would be better to simply have a
precursor patch that changes wait_task_inactive(child, TASK_TRACED) to
wait_task_inactive(child, 0) and say so explicitly.

Eric