From: Pavel Begunkov <asml.silence@gmail.com>
To: Olivier Langlois <olivier@trillion01.com>,
Jens Axboe <axboe@kernel.dk>,
Tony Battersby <tonyb@cybernetics.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Oleg Nesterov <oleg@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
io-uring <io-uring@vger.kernel.org>,
Alexander Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH] coredump: Limit what can interrupt coredumps
Date: Mon, 16 Aug 2021 14:02:30 +0100 [thread overview]
Message-ID: <b9f92bf3-77aa-8cdd-6db7-95c86e5a6946@gmail.com> (raw)
In-Reply-To: <b36eb4a26b6aff564c6ef850a3508c5b40141d46.camel@trillion01.com>
On 8/15/21 9:42 PM, Olivier Langlois wrote:
[...]
> When I have first encountered the issue, the very first thing that I
> did try was to create a simple test program that would synthetize the
> problem.
>
> After few time consumming failed attempts, I just gave up the idea and
> simply settle to my prod program that showcase systematically the
> problem every time that I kill the process with a SEGV signal.
>
> In a nutshell, all the program does is to issue read operations with
> io_uring on a TCP socket on which there is a constant data stream.
>
> Now that I have a better understanding of what is going on, I think
> that one way that could reproduce the problem consistently could be
> along those lines:
>
> 1. Create a pipe
> 2. fork a child
> 3. Initiate a read operation on the pipe with io_uring from the child
> 4. Let the parent kill its child with a core dump generating signal.
> 5. Write something in the pipe from the parent so that the io_uring
> read operation completes while the core dump is generated.
>
> I guess that I'll end up doing that if I cannot fix the issue with my
> current setup but here is what I have attempted so far:
>
> 1. Call io_uring_files_cancel from do_coredump
> 2. Same as #1 but also make sure that TIF_NOTIFY_SIGNAL is cleared on
> returning from io_uring_files_cancel
>
> Those attempts didn't work but lurking in the io_uring dev mailing list
> is starting to pay off. I thought that I did reach the bottom of the
> rabbit hole in my journey of understanding io_uring but the recent
> patch set sent by Hao Xu
>
> https://lore.kernel.org/io-uring/90fce498-968e-6812-7b6a-fdf8520ea8d9@kernel.dk/T/#t
>
> made me realize that I still haven't assimilated all the small io_uring
> nuances...
>
> Here is my feedback. From my casual io_uring code reader point of view,
> it is not 100% obvious what the difference is between
> io_uring_files_cancel and io_uring_task_cancel
As you mentioned, io_uring_task_cancel() cancels and waits for all
requests submitted by current task, used in exec() and SQPOLL because
of potential races.
io_uring_task_cancel() cancels only selected ones and
io_uring_files_cancel()
cancels and waits only some specific requests that we absolutely have
to, e.g. in 5.15 it'll be only requests referencing the ring itself.
It's used on normal task exit.
io_uring_task_cancel() cancels and waits all requests submitted by
current task, used on exec() because of races.
As you mentioned
>
> It seems like io_uring_files_cancel is cancelling polls only if they
> have the REQ_F_INFLIGHT flag set.
>
> I have no idea what an inflight request means and why someone would
> want to call io_uring_files_cancel over io_uring_task_cancel.
>
> I guess that if I was to meditate on the question for few hours, I
> would at some point get some illumination strike me but I believe that
> it could be a good idea to document in the code those concepts for
> helping casual readers...
>
> Bottomline, I now understand that io_uring_files_cancel does not cancel
> all the requests. Therefore, without fully understanding what I am
> doing, I am going to replace my call to io_uring_files_cancel from
> do_coredump with io_uring_task_cancel and see if this finally fix the
> issue for good.
>
> What I am trying to do is to cancel pending io_uring requests to make
> sure that TIF_NOTIFY_SIGNAL isn't set while core dump is generated.
>
> Maybe another solution would simply be to modify __dump_emit to make it
> resilient to TIF_NOTIFY_SIGNAL as Eric W. Biederman originally
> suggested.
>
> or maybe do both...
>
> Not sure which approach is best. If someone has an opinion, I would be
> curious to hear it.
>
> Greetings,
>
>
--
Pavel Begunkov
next prev parent reply other threads:[~2021-08-16 13:03 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <192c9697e379bf084636a8213108be6c3b948d0b.camel@trillion01.com>
[not found] ` <9692dbb420eef43a9775f425cb8f6f33c9ba2db9.camel@trillion01.com>
[not found] ` <87h7i694ij.fsf_-_@disp2133>
2021-06-09 20:33 ` [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL Linus Torvalds
2021-06-09 20:48 ` Eric W. Biederman
2021-06-09 20:52 ` Linus Torvalds
2021-06-09 21:02 ` Olivier Langlois
2021-06-09 21:05 ` Eric W. Biederman
2021-06-09 21:26 ` Olivier Langlois
2021-06-09 21:56 ` Olivier Langlois
2021-06-10 14:26 ` Eric W. Biederman
2021-06-10 15:17 ` Olivier Langlois
2021-06-10 18:58 ` [CFT}[PATCH] coredump: Limit what can interrupt coredumps Eric W. Biederman
2021-06-10 19:10 ` Linus Torvalds
2021-06-10 19:18 ` Eric W. Biederman
2021-06-10 19:50 ` Linus Torvalds
2021-06-10 20:11 ` [PATCH] " Eric W. Biederman
2021-06-10 21:04 ` Linus Torvalds
2021-06-12 14:36 ` Olivier Langlois
2021-06-12 16:26 ` Jens Axboe
2021-06-14 14:10 ` Oleg Nesterov
2021-06-14 16:37 ` Eric W. Biederman
2021-06-14 16:59 ` Oleg Nesterov
2021-06-15 22:08 ` Eric W. Biederman
2021-06-16 19:23 ` Olivier Langlois
2021-06-16 20:00 ` Eric W. Biederman
2021-06-18 20:05 ` Olivier Langlois
2021-08-05 13:06 ` Olivier Langlois
2021-08-10 21:48 ` Tony Battersby
2021-08-11 20:47 ` Olivier Langlois
2021-08-12 1:55 ` Jens Axboe
2021-08-12 13:53 ` Tony Battersby
2021-08-15 20:42 ` Olivier Langlois
2021-08-16 13:02 ` Pavel Begunkov [this message]
2021-08-16 13:06 ` Pavel Begunkov
2021-08-17 18:15 ` Jens Axboe
2021-08-17 18:24 ` Jens Axboe
2021-08-17 19:29 ` Tony Battersby
2021-08-17 19:59 ` Jens Axboe
2021-08-17 21:28 ` Jens Axboe
2021-08-17 21:39 ` Tony Battersby
2021-08-17 22:05 ` Jens Axboe
2021-08-18 14:37 ` Tony Battersby
2021-08-18 14:46 ` Jens Axboe
2021-08-18 2:57 ` Jens Axboe
2021-08-18 2:58 ` Jens Axboe
2021-08-21 10:08 ` Olivier Langlois
2021-08-21 16:47 ` Olivier Langlois
2021-08-21 16:51 ` Jens Axboe
2021-08-21 17:21 ` Olivier Langlois
2021-08-21 9:52 ` Olivier Langlois
2021-08-21 9:48 ` Olivier Langlois
2021-10-22 14:13 ` [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL Pavel Begunkov
2021-12-24 1:34 ` Olivier Langlois
2021-12-24 10:37 ` Pavel Begunkov
2021-12-24 19:52 ` Eric W. Biederman
2021-12-28 11:24 ` Pavel Begunkov
2022-03-14 23:58 ` Eric W. Biederman
[not found] ` <8218f1a245d054c940e25142fd00a5f17238d078.camel@trillion01.com>
2022-06-01 3:15 ` Jens Axboe
2022-07-20 16:49 ` [PATCH 0/2] coredump: Allow io_uring using apps to dump to pipes Eric W. Biederman
2022-07-20 16:50 ` [PATCH 1/2] signal: Move stopping for the coredump from do_exit into get_signal Eric W. Biederman
2022-07-20 16:51 ` [PATCH 2/2] coredump: Allow coredumps to pipes to work with io_uring Eric W. Biederman
2022-08-22 21:16 ` Olivier Langlois
2022-08-23 3:35 ` Olivier Langlois
2022-08-23 18:22 ` Eric W. Biederman
2022-08-23 18:27 ` Jens Axboe
2022-08-24 15:11 ` Eric W. Biederman
2022-08-24 15:51 ` Jens Axboe
2022-01-05 19:39 ` [RFC] coredump: Do not interrupt dump for TIF_NOTIFY_SIGNAL Olivier Langlois
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=b9f92bf3-77aa-8cdd-6db7-95c86e5a6946@gmail.com \
--to=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=ebiederm@xmission.com \
--cc=io-uring@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=olivier@trillion01.com \
--cc=tonyb@cybernetics.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.