From: Jens Axboe <axboe@kernel.dk>
To: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>,
io-uring@vger.kernel.org
Subject: Re: [PATCH 8/8] io_uring: enable IORING_SETUP_ATTACH_WQ to attach to SQPOLL thread too
Date: Mon, 7 Sep 2020 10:14:27 -0600 [thread overview]
Message-ID: <13bd91f7-9eef-cc38-d892-d28e5d068421@kernel.dk> (raw)
In-Reply-To: <c6562c28-7631-d593-d3e5-cde158337337@linux.alibaba.com>
On 9/7/20 2:56 AM, Xiaoguang Wang wrote:
> 3. When it's appropriate to set ctx's IORING_SQ_NEED_WAKEUP flag? In
> your current implementation, if a ctx is marked as SQT_IDLE, this ctx
> will be set IORING_SQ_NEED_WAKEUP flag, but if other ctxes have work
> to do, then io_sq_thread is still running and does not need to be
> waken up, then a later wakeup form userspace is unnecessary. I think
> it maybe appropriate to set IORING_SQ_NEED_WAKEUP when all ctxes have
> no work to do, you can have a look at my attached codes:)
That's a good point, any chance I can get you to submit a patch to fix
that up?
> 4. Is io_attach_sq_data really safe? sqd_list is a global list, but
> its search key is a fd local to process, different processes may have
> same fd, then this codes looks odd, seems that your design is to make
> io_sq_thread shared inside process.
It's really meant for thread sharing, or you could pass the fd and use
it across a process too. See the incremental I just sent in a reply to
Pavel.
That said, I do think the per-cpu approach has merrit, and I also think
it should be possible to layer it on top of the existing code in
for-5.10/io_uring. So I'd strongly encourage you to try and do that, so
you can get rid of using that private patch and just have it upstream
instead.
--
Jens Axboe
next prev parent reply other threads:[~2020-09-07 16:17 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-03 2:20 [PATCHSET for-next 0/8] io_uring SQPOLL improvements Jens Axboe
2020-09-03 2:20 ` [PATCH 1/8] io_uring: io_sq_thread() doesn't need to flush signals Jens Axboe
2020-09-03 2:20 ` [PATCH 2/8] io_uring: allow SQPOLL with CAP_SYS_NICE privileges Jens Axboe
2020-09-03 2:20 ` [PATCH 3/8] io_uring: use private ctx wait queue entries for SQPOLL Jens Axboe
2020-09-03 2:20 ` [PATCH 4/8] io_uring: move SQPOLL post-wakeup ring need wakeup flag into wake handler Jens Axboe
2020-09-03 2:20 ` [PATCH 5/8] io_uring: split work handling part of SQPOLL into helper Jens Axboe
2020-09-03 2:20 ` [PATCH 6/8] io_uring: split SQPOLL data into separate structure Jens Axboe
2020-09-03 2:20 ` [PATCH 7/8] io_uring: base SQPOLL handling off io_sq_data Jens Axboe
2020-09-03 2:20 ` [PATCH 8/8] io_uring: enable IORING_SETUP_ATTACH_WQ to attach to SQPOLL thread too Jens Axboe
2020-09-07 8:56 ` Xiaoguang Wang
2020-09-07 14:00 ` Pavel Begunkov
2020-09-07 16:11 ` Jens Axboe
2020-09-07 16:14 ` Jens Axboe [this message]
2020-09-07 16:18 ` Jens Axboe
2020-09-08 2:28 ` Xiaoguang Wang
2020-09-08 2:53 ` Xiaoguang Wang
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=13bd91f7-9eef-cc38-d892-d28e5d068421@kernel.dk \
--to=axboe@kernel.dk \
--cc=io-uring@vger.kernel.org \
--cc=xiaoguang.wang@linux.alibaba.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.