linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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).