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;
Subject: Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for
 signaled process.
To:     Oleg Nesterov <oleg@redhat.com>
Cc:     christian.koenig@amd.com,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        David.Panariti@amd.com, amd-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, Alexander.Deucher@amd.com,
        akpm@linux-foundation.org
References: <87muxsbmkp.fsf@xmission.com>
 <8840ac96-50c4-f94d-eb7c-f007940163f3@amd.com> <877eowa5qh.fsf@xmission.com>
 <20180425135552.GD7592@redhat.com>
 <f45c7017-c75a-7f0c-b6d5-4d79600cdd33@amd.com>
 <20180425171757.GA10441@redhat.com>
 <c39cb438-1e8a-4f2c-50f6-c6e2a144487b@amd.com> <874ljyu98e.fsf@xmission.com>
 <c3c9787d-b279-8169-43d1-74eeb666ffbd@gmail.com>
 <bceb1a1b-c453-782d-5a7d-40fa2f22c813@amd.com>
 <20180430160006.GB10583@redhat.com>
From:   Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
Message-ID: <e5b0221d-84ba-10ff-4a58-4fa27c99650f@amd.com>
Date:   Mon, 30 Apr 2018 12:10:36 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.4.0
MIME-Version: 1.0
In-Reply-To: <20180430160006.GB10583@redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Received-SPF: None (protection.outlook.com: amd.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Apr 2018 16:10:43.0245 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: a28e10b5-e2c3-47ba-6be7-08d5aeb4edb0
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB0319
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 04/30/2018 12:00 PM, Oleg Nesterov wrote:
> On 04/30, Andrey Grodzovsky wrote:
>> What about changing PF_SIGNALED to  PF_EXITING in
>> drm_sched_entity_do_release
>>
>> -       if ((current->flags & PF_SIGNALED) && current->exit_code == SIGKILL)
>> +      if ((current->flags & PF_EXITING) && current->exit_code == SIGKILL)
> let me repeat, please don't use task->exit_code. And in fact this check is racy
>
> But this doesn't matter. Say, we can trivially add SIGNAL_GROUP_KILLED_BY_SIGKILL,
> or do something else,


Can you explain where is the race and what is a possible alternative then ?

>   but I fail to understand what are you trying to do. Suppose
> that the check above is correct in that it is true iff the task is exiting and
> it was killed by SIGKILL. What about the "else" branch which does
>
> 	r = wait_event_killable(sched->job_scheduled, ...)
>
> ?
>
> Once again, fatal_signal_pending() (or even signal_pending()) is not well defined
> after the exiting task passes exit_signals().
>
> So wait_event_killable() can fail because fatal_signal_pending() is true; and this
> can happen even if it was not killed.
>
> Or it can block and SIGKILL won't be able to wake it up.
>
>> If SIGINT was sent then it's SIGINT,
> Yes, but see above. in this case fatal_signal_pending() will be likely true so
> wait_event_killable() will fail unless condition is already true.

My bad, I didn't show the full intended fix, it was just a snippet to 
address the differentiation between exiting
do to SIGKILL and any other exit, I also intended to change 
wait_event_killable to wait_event_timeout.

Andrey

>
> Oleg.
>