On 3/26/21 2:43 PM, Eric W. Biederman wrote: > Jens Axboe <axboe@xxxxxxxxx> writes: > >> Right now we're never calling get_signal() from PF_IO_WORKER threads, but >> in preparation for doing so, don't handle a fatal signal for them. The >> workers have state they need to cleanup when exiting, and they don't do >> coredumps, so just return instead of performing either a dump or calling >> do_exit() on their behalf. The threads themselves will detect a fatal >> signal and do proper shutdown. >> >> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> >> --- >> kernel/signal.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/kernel/signal.c b/kernel/signal.c >> index f2a1b898da29..e3e1b8fbfe8a 100644 >> --- a/kernel/signal.c >> +++ b/kernel/signal.c >> @@ -2756,6 +2756,15 @@ bool get_signal(struct ksignal *ksig) >> */ >> current->flags |= PF_SIGNALED; >> >> + /* >> + * PF_IO_WORKER threads will catch and exit on fatal signals >> + * themselves. They have cleanup that must be performed, so >> + * we cannot call do_exit() on their behalf. coredumps also >> + * do not apply to them. >> + */ >> + if (current->flags & PF_IO_WORKER) >> + return false; >> + > > Returning false when get_signal needs the caller to handle a signal > adds a very weird and awkward special case to how get_signal returns > arguments. > > Instead you should simply break and let get_signal return SIGKILL like > any other signal that has a handler that the caller of get_signal needs > to handle. > > Something like: >> + /* >> + * PF_IO_WORKER have cleanup that must be performed, >> + * before calling do_exit(). >> + */ >> + if (current->flags & PF_IO_WORKER) >> + break; > > > As do_coredump does not call do_exit there is no reason to skip calling into > the coredump handling either. And allowing it will remove yet another > special case from the io worker code. Thanks, I'll turn it into a break, that does seem like a better idea in general. Actually it wants to be a goto or similar, as a break will assume that we have the sighand lock held. With the coredump being irrelevant, I'll just it before the do_exit() call. -- Jens Axboe
