From: Ming Lei <ming.lei@redhat.com>
To: Uday Shankar <ushankar@purestorage.com>
Cc: Jens Axboe <axboe@kernel.dk>,
Caleb Sander Mateos <csander@purestorage.com>,
linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/4] ublk: require unique task per io instead of unique task per hctx
Date: Fri, 18 Apr 2025 09:30:11 +0800 [thread overview]
Message-ID: <aAGrI8wqLLCWzqNe@fedora> (raw)
In-Reply-To: <20250416-ublk_task_per_io-v5-1-9261ad7bff20@purestorage.com>
On Wed, Apr 16, 2025 at 01:46:05PM -0600, Uday Shankar wrote:
> Currently, ublk_drv associates to each hardware queue (hctx) a unique
> task (called the queue's ubq_daemon) which is allowed to issue
> COMMIT_AND_FETCH commands against the hctx. If any other task attempts
> to do so, the command fails immediately with EINVAL. When considered
> together with the block layer architecture, the result is that for each
> CPU C on the system, there is a unique ublk server thread which is
> allowed to handle I/O submitted on CPU C. This can lead to suboptimal
> performance under imbalanced load generation. For an extreme example,
> suppose all the load is generated on CPUs mapping to a single ublk
> server thread. Then that thread may be fully utilized and become the
> bottleneck in the system, while other ublk server threads are totally
> idle.
>
> This issue can also be addressed directly in the ublk server without
> kernel support by having threads dequeue I/Os and pass them around to
> ensure even load. But this solution requires inter-thread communication
> at least twice for each I/O (submission and completion), which is
> generally a bad pattern for performance. The problem gets even worse
> with zero copy, as more inter-thread communication would be required to
> have the buffer register/unregister calls to come from the correct
> thread.
>
> Therefore, address this issue in ublk_drv by requiring a unique task per
> I/O instead of per queue/hctx. Imbalanced load can then be balanced
> across all ublk server threads by having threads issue FETCH_REQs in a
> round-robin manner. As a small toy example, consider a system with a
> single ublk device having 2 queues, each of queue depth 4. A ublk server
> having 4 threads could issue its FETCH_REQs against this device as
> follows (where each entry is the qid,tag pair that the FETCH_REQ
> targets):
>
> poller thread: T0 T1 T2 T3
> 0,0 0,1 0,2 0,3
> 1,3 1,0 1,1 1,2
>
> Since tags appear to be allocated in sequential chunks, this setup
> provides a rough approximation to distributing I/Os round-robin across
> all ublk server threads, while letting I/Os stay fully thread-local.
>
> Signed-off-by: Uday Shankar <ushankar@purestorage.com>
> Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
I guess this patch need to rebase against yesterday Jens's merge.
Given this change is big from ublk serer viewpoint, it should aim at
v6.16
As I suggested on V3[1]:
- it is nice to make it as one feature, so you can make any optimization
for this feature only, such as applying BLK_MQ_F_TAG_RR for improving
IO locality
- add selftest for this usage, which is helpful for
- verify this usage
- avoid to break the feature with new change
- and evaluate performance effect
[1] https://lore.kernel.org/linux-block/aABZl4Yxdf3yew4q@fedora/
thanks,
Ming
next prev parent reply other threads:[~2025-04-18 1:30 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-16 19:46 [PATCH v5 0/4] ublk: decouple server threads from hctxs Uday Shankar
2025-04-16 19:46 ` [PATCH v5 1/4] ublk: require unique task per io instead of unique task per hctx Uday Shankar
2025-04-18 1:30 ` Ming Lei [this message]
2025-04-18 2:30 ` Jens Axboe
2025-04-16 19:46 ` [PATCH v5 2/4] ublk: mark ublk_queue as const for ublk_commit_and_fetch Uday Shankar
2025-04-18 1:35 ` Ming Lei
2025-04-16 19:46 ` [PATCH v5 3/4] ublk: mark ublk_queue as const for ublk_register_io_buf Uday Shankar
2025-04-18 1:36 ` Ming Lei
2025-04-16 19:46 ` [PATCH v5 4/4] ublk: mark ublk_queue as const for ublk_handle_need_get_data Uday Shankar
2025-04-16 19:55 ` Caleb Sander Mateos
2025-04-18 1:39 ` 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=aAGrI8wqLLCWzqNe@fedora \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=csander@purestorage.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ushankar@purestorage.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 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).