All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Begunkov <asml.silence@gmail.com>
To: Ming Lei <ming.lei@redhat.com>, Jens Axboe <axboe@kernel.dk>,
	io-uring@vger.kernel.org
Cc: Kanchan Joshi <joshi.k@samsung.com>
Subject: Re: [PATCH] io_uring: complete request via task work in case of DEFER_TASKRUN
Date: Fri, 14 Apr 2023 14:01:26 +0100	[thread overview]
Message-ID: <68ddddc0-fb0e-47b4-9318-9dd549d851a1@gmail.com> (raw)
In-Reply-To: <20230414075313.373263-1-ming.lei@redhat.com>

On 4/14/23 08:53, Ming Lei wrote:
> So far io_req_complete_post() only covers DEFER_TASKRUN by completing
> request via task work when the request is completed from IOWQ.
> 
> However, uring command could be completed from any context, and if io
> uring is setup with DEFER_TASKRUN, the command is required to be
> completed from current context, otherwise wait on IORING_ENTER_GETEVENTS
> can't be wakeup, and may hang forever.

fwiw, there is one legit exception, when the task is half dead
task_work will be executed by a kthread. It should be fine as it
locks the ctx down, but I can't help but wonder whether it's only
ublk_cancel_queue() affected or there are more places in ublk?

One more thing, cmds should not be setting issue_flags but only
forwarding what the core io_uring code passed, it'll get tons of
bugs in no time otherwise.

static void ublk_cancel_queue(struct ublk_queue *ubq)
{
     ...
     io_uring_cmd_done(io->cmd, UBLK_IO_RES_ABORT, 0,
                       IO_URING_F_UNLOCKED);
}

Can we replace it with task_work? It should be cold, and I
assume ublk_cancel_queue() doesn't assume that all requests will
put down by the end of the function as io_uring_cmd_done()
can offload it in any case.

-- 
Pavel Begunkov

  parent reply	other threads:[~2023-04-14 13:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20230414075422epcas5p3ae5de53e643a448f19df82a7a1d5cd1c@epcas5p3.samsung.com>
2023-04-14  7:53 ` [PATCH] io_uring: complete request via task work in case of DEFER_TASKRUN Ming Lei
2023-04-14 11:52   ` Kanchan Joshi
2023-04-14 12:39   ` Jens Axboe
2023-04-14 13:01   ` Pavel Begunkov [this message]
2023-04-14 13:53     ` Ming Lei
2023-04-14 14:13       ` Kanchan Joshi
2023-04-14 14:53         ` Ming Lei
2023-04-14 15:07       ` Pavel Begunkov
2023-04-14 15:42         ` Ming Lei
2023-04-15 23:15           ` Pavel Begunkov
2023-04-16 10:05             ` Ming Lei

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=68ddddc0-fb0e-47b4-9318-9dd549d851a1@gmail.com \
    --to=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=joshi.k@samsung.com \
    --cc=ming.lei@redhat.com \
    /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.