linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Begunkov <asml.silence@gmail.com>
To: Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [BUG][RFC] Miscalculated inflight counter in io_uring
Date: Fri, 25 Oct 2019 12:48:07 +0300	[thread overview]
Message-ID: <bfecc5ba-274b-b2f7-52dc-8ac6e0fab352@gmail.com> (raw)


[-- Attachment #1.1: Type: text/plain, Size: 1591 bytes --]

In case of IORING_SETUP_SQPOLL | IORING_SETUP_IOPOLL:

@inflight count returned by io_submit_sqes() is the number of entries
picked up from a sqring including already completed/failed. And
completed but not failed requests will be placed into @poll_list.

Then io_sq_thread() tries to poll @inflight events, even though failed
won't appear in @poll_list. Thus, it will think that there are always
something to poll (i.e. @inflight > 0)

There are several issues with this:
1. io_sq_thread won't ever sleep
2. io_sq_thread() may be left running and actively polling even after
user process is destroyed
3. the same goes for mm_struct with all vmas of the user process
TL;DR;
awhile @inflight > 0, io_sq_thread won't put @cur_mm, so locking
recycling of vmas used for rings' mapping, which hold refcount of
io_uring's struct file. Thus, io_uring_release() won't be called, as
well as kthread_{park,stop}(). That's all in case when the user process
haven't unmapped rings.


I'm not sure how to fix it better:
1. try to put failed into poll_list (grabbing mutex).

2. test for zero-inflight case with comparing sq and cq. something like
```
if (nr_polled == 0) {
	lock(comp_lock);
	if (cached_cq_head == cached_sq_tail)
		inflight = 0;
	unlock(comp_lock);
}
```
But that's adds extra spinlock locking in fast-path. And that's unsafe
to use non-cached heads/tails, as it could be maliciously changed by
userspace.

3. Do some counting of failed (probably needs atomic or synchronisation)

4. something else?


-- 
Yours sincerely,
Pavel Begunkov


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

             reply	other threads:[~2019-10-25  9:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-25  9:48 Pavel Begunkov [this message]
2019-10-25 15:32 ` [BUG][RFC] Miscalculated inflight counter in io_uring Jens Axboe
2019-10-25 16:08   ` Pavel Begunkov
2019-10-25 16:18     ` Jens Axboe

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=bfecc5ba-274b-b2f7-52dc-8ac6e0fab352@gmail.com \
    --to=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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).