Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for signaled process.
From: Andrey Grodzovsky
Date: Wed May 23 2018 - 10:14:57 EST
On 05/01/2018 10:35 AM, Oleg Nesterov wrote:
On 04/30, Andrey Grodzovsky wrote:
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 ?
Oh. I mentioned this race automatically, because I am pedant ;) Let me repeat
that this doesn't really matter, and let me remind that the caller of fop->release
can be completely unrelated process, say $cat /proc/pid/fdinfo. And in any case
->exit_code should not be used outside of ptrace/exit paths.
OK, the race. Consider a process P with a main thread M and a sub-thread T.
T does pthread_exit(), enters do_exit() and gets a preemption before exit_files().
The process is killed by SIGKILL. M calls do_group_exit(), do_exit() and passes
exit_files(). However, it doesn't call close_files() because T has another reference.
T resumes, calls close_files(), fput(), etc, and then exit_task_work(), so
it can finally call ->release() with current->exit_code == 0 desptite the fact
the process was killed.
Again, again, this doesn't matter. We can distinguish killed-or-not, by SIGKILL-
or-not. But I still do not think we actually need this. At least in ->release()
paths, ->flush() may differ.
Hi Oleg, reworked the code to use .flush hook and eliminated
wait_event_killable.
So in case of .flush is this OK to look at task->exit_code == SIGKILL to
distinguish
exit by signal ?
Andrey
Oleg.