public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Xue He <xue01.he@samsung.com>
Cc: axboe@kernel.dk, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, yukuai@fnnas.com
Subject: Re: [PATCH v6] block: plug attempts to batch allocate tags multiple times
Date: Mon, 17 Nov 2025 18:19:51 +0800	[thread overview]
Message-ID: <aRr2x89ShJa08jNk@fedora> (raw)
In-Reply-To: <20251117084845.254680-1-xue01.he@samsung.com>

On Mon, Nov 17, 2025 at 08:48:45AM +0000, Xue He wrote:
> This patch aims to enable batch allocation of sufficient tags after
> batch IO submission with plug mechanism, thereby avoiding the need for
> frequent individual requests when the initial allocation is
> insufficient.
> -----------------------------------------------------------
> HW:
> 16 CPUs/16 poll queues
> Disk: Samsung PM9A3 Gen4 3.84T
> 
> CMD:
> [global]
> ioengine=io_uring
> group_reporting=1
> time_based=1
> runtime=1m
> refill_buffers=1
> norandommap=1
> randrepeat=0
> fixedbufs=1
> registerfiles=1
> rw=randread
> iodepth=128
> iodepth_batch_submit=32
> iodepth_batch_complete_min=32
> iodepth_batch_complete_max=128
> iodepth_low=32
> bs=4k
> numjobs=1
> direct=1
> hipri=1
> 
> [job1]
> filename=/dev/nvme0n1
> name=batch_test
> ------------------------------------------------------------
> Perf:
> base code: __blk_mq_alloc_requests() 1.47%
> patch: __blk_mq_alloc_requests() 0.78%
> ------------------------------------------------------------
> 
> ---
> changes since v1:
> - Modify multiple batch registrations into a single loop to achieve
>   the batch quantity
> 
> changes since v2:
> - Modify the call location of remainder handling
> - Refactoring sbitmap cleanup time
> 
> changes since v3:
> - Add handle operation in loop
> - Add helper sbitmap_find_bits_in_word
> 
> changes since v4:
> - Split blk-mq.c changes from sbitmap
> 
> changes since v5:
> - Add workload with perf
> - Modify over-counting bug
> 
> Signed-off-by: hexue <xue01.he@samsung.com>
> ---
>  block/blk-mq.c | 39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index d626d32f6e57..9e6fca1b5fb7 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -467,26 +467,31 @@ __blk_mq_alloc_requests_batch(struct blk_mq_alloc_data *data)
>  	unsigned long tag_mask;
>  	int i, nr = 0;
>  
> -	tag_mask = blk_mq_get_tags(data, data->nr_tags, &tag_offset);
> -	if (unlikely(!tag_mask))
> -		return NULL;
> +	do {
> +		tag_mask = blk_mq_get_tags(data, data->nr_tags, &tag_offset);

You may keep the original batch update on `data->nr_tags` by passing
`data->nr_tags - nr`.

> +		if (unlikely(!tag_mask)) {
> +			if (nr == 0)
> +				return NULL;
> +			break;
> +		}
> +		tags = blk_mq_tags_from_data(data);
> +		for (i = 0; tag_mask; i++) {
> +			if (!(tag_mask & (1UL << i)))
> +				continue;
> +			tag = tag_offset + i;
> +			prefetch(tags->static_rqs[tag]);
> +			tag_mask &= ~(1UL << i);
> +			rq = blk_mq_rq_ctx_init(data, tags, tag);
> +			rq_list_add_head(data->cached_rqs, rq);
> +			data->nr_tags--;

data->nr_tags-- can be killed.

> +			nr++;
> +		}
> +	} while (data->nr_tags);

Replace the above check with `data->nr_tags > nr`

>  
> -	tags = blk_mq_tags_from_data(data);
> -	for (i = 0; tag_mask; i++) {
> -		if (!(tag_mask & (1UL << i)))
> -			continue;
> -		tag = tag_offset + i;
> -		prefetch(tags->static_rqs[tag]);
> -		tag_mask &= ~(1UL << i);
> -		rq = blk_mq_rq_ctx_init(data, tags, tag);
> -		rq_list_add_head(data->cached_rqs, rq);
> -		nr++;
> -	}
> -	if (!(data->rq_flags & RQF_SCHED_TAGS))
> -		blk_mq_add_active_requests(data->hctx, nr);
>  	/* caller already holds a reference, add for remainder */
>  	percpu_ref_get_many(&data->q->q_usage_counter, nr - 1);
> -	data->nr_tags -= nr;

The above line needs to keep.

> +	if (!(data->rq_flags & RQF_SCHED_TAGS))
> +		blk_mq_add_active_requests(data->hctx, nr);

The above two lines can be kept in original position, then the change
outside loop can be avoided.

With above update, feel free to add:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming


  reply	other threads:[~2025-11-17 10:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20251117085321epcas5p3de0647f01a818db024bea32870f223f4@epcas5p3.samsung.com>
2025-11-17  8:48 ` [PATCH v6] block: plug attempts to batch allocate tags multiple times Xue He
2025-11-17 10:19   ` Ming Lei [this message]
2025-11-17 10:22     ` Xue He

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=aRr2x89ShJa08jNk@fedora \
    --to=ming.lei@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=xue01.he@samsung.com \
    --cc=yukuai@fnnas.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