Re: [GIT PULL] io_uring fixes for 5.15-rc3

[Jens Axboe <axboe@kernel.dk>]

On 9/27/21 7:51 AM, Eric W. Biederman wrote:
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> On 9/25/21 5:05 PM, Linus Torvalds wrote:
>>> On Sat, Sep 25, 2021 at 1:32 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> - io-wq core dump exit fix (me)
>>>
>>> Hmm.
>>>
>>> That one strikes me as odd.
>>>
>>> I get the feeling that if the io_uring thread needs to have that
>>> signal_group_exit() test, something is wrong in signal-land.
>>>
>>> It's basically a "fatal signal has been sent to another thread", and I
>>> really get the feeling that "fatal_signal_pending()" should just be
>>> modified to handle that case too.
>>
>> It did surprise me as well, which is why that previous change ended up
>> being broken for the coredump case... You could argue that the io-wq
>> thread should just exit on signal_pending(), which is what we did
>> before, but that really ends up sucking for workloads that do use
>> signals for communication purposes. postgres was the reporter here.
> 
> The primary function get_signal is to make signals not pending.  So I
> don't understand any use of testing signal_pending after a call to
> get_signal.
> 
> My confusion doubles when I consider the fact io_uring threads should
> only be dequeuing SIGSTOP and SIGKILL.
> 
> I am concerned that an io_uring thread that dequeues SIGKILL won't call
> signal_group_exit and thus kill the other threads in the thread group.
> 
> What motivated removing the break and adding the fatal_signal_pending
> test?

I played with this a bit this morning, and I agree it doesn't seem to be
needed at all. The original issue was with postgres, I'll give that a
whirl as well and see if we run into any unwarranted exits. My simpler
test case did not.

-- 
Jens Axboe