io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jens Axboe <axboe@kernel.dk>, Oleg Nesterov <oleg@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	io-uring <io-uring@vger.kernel.org>
Subject: Re: [GIT PULL] io_uring fixes for 5.15-rc3
Date: Sat, 25 Sep 2021 23:31:35 -0500	[thread overview]
Message-ID: <87ee9cdkk8.fsf@disp2133> (raw)
In-Reply-To: <CAHk-=whi3UxvY1C1LQNCO9d2xzX5A69qfzNGbBVGpRE_6gv=9Q@mail.gmail.com> (Linus Torvalds's message of "Sat, 25 Sep 2021 16:05:07 -0700")

Linus Torvalds <torvalds@linux-foundation.org> writes:

> 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.
>
> Because what about a number of other situations where we have that
> "killable" logic (ie "stop waiting for locks or IO if you're just
> going to get killed anyway" - things like lock_page_killable() and
> friends)
>
> Adding Eric, Oleg and Al to the participants, so that somebody else can pipe up.
>
> That piping up may quite possibly be to just tell me I'm being stupid,
> and that this is just a result of some io_uring thread thing, and
> nobody else has this problem.
>
> It's commit 87c169665578 ("io-wq: ensure we exit if thread group is
> exiting") in my tree.
>
> Comments?

I agree it smells.

It smells that there needs to be a test after get_signal returns with a
signal from an io_uring thread.

As I recall io-wq threads block all signals except for SIGSTOP and
SIGKILL.  The signal SIGSTOP is never returned from get_signal.  So at
a first glance every return from get_signal should be SIGKILL and thus
fatal.

So I don't understand why there needs to be a test at all after
get_signal.

Further the SIGKILL should have been delivered by get_signal so it
should not be pending so I am not certain when fatal_signal_pending.

Can someone please explain commit 15e20db2e0ce ("io-wq: only exit on
fatal signals") to me?

What signals is get_signal returning to be delivered to userspace that
are not fatal and that are ok to ignore?

I think I am missing something.

Eric

  parent reply	other threads:[~2021-09-26  4:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-25 20:32 [GIT PULL] io_uring fixes for 5.15-rc3 Jens Axboe
2021-09-25 23:05 ` Linus Torvalds
2021-09-26  1:20   ` Jens Axboe
2021-09-27 13:51     ` Eric W. Biederman
2021-09-27 14:29       ` Jens Axboe
2021-09-27 14:59         ` Jens Axboe
2021-09-27 15:13           ` Eric W. Biederman
2021-09-27 15:41             ` Jens Axboe
2021-09-27 15:52               ` Eric W. Biederman
2021-09-27 16:03                 ` Jens Axboe
2021-09-26  4:31   ` Eric W. Biederman [this message]
2021-09-25 23:05 ` pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ee9cdkk8.fsf@disp2133 \
    --to=ebiederm@xmission.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).