linux-kernel - Re: [PATCH 3/3] block: avoid hctx spinlock for plug with multiple queues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aAoiq1AsoD-k_kp3@infradead.org>
Date: Thu, 24 Apr 2025 04:38:19 -0700
From: Christoph Hellwig <hch@...radead.org>
To: Caleb Sander Mateos <csander@...estorage.com>
Cc: Jens Axboe <axboe@...nel.dk>, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] block: avoid hctx spinlock for plug with multiple
 queues

> +static void blk_mq_extract_queue_requests(struct rq_list *rqs,
> +					  struct rq_list *queue_rqs,
> +					  unsigned *queue_depth)
> +{
> +	struct rq_list matched_rqs = {}, unmatched_rqs = {};
> +	struct request *rq = rq_list_pop(rqs);
> +	struct request_queue *this_q = rq->q;
> +	unsigned depth = 1;
> +
> +	rq_list_add_tail(&matched_rqs, rq);
> +	while ((rq = rq_list_pop(rqs))) {
> +		if (rq->q == this_q) {
> +			rq_list_add_tail(&matched_rqs, rq);
> +			depth++;
> +		} else {
> +			rq_list_add_tail(&unmatched_rqs, rq);
> +		}

This might be moe efficient if you keep an extra iterator and never
mode the request for another queue.

> +	}
> +
> +	*queue_rqs = matched_rqs;
> +	*rqs = unmatched_rqs;
> +	*queue_depth = depth;

.. and I'd return the queue depth here instead of making it another
output argument.

> +static void blk_mq_dispatch_multiple_queue_requests(struct rq_list *rqs)
> +{
> +	do {
> +		struct rq_list queue_rqs;
> +		unsigned depth;
> +
> +		blk_mq_extract_queue_requests(rqs, &queue_rqs, &depth);
> +		blk_mq_dispatch_queue_requests(&queue_rqs, depth);
> +		while (!rq_list_empty(&queue_rqs)) {
> +			blk_mq_dispatch_list(&queue_rqs, false);
> +		}

No need for the braces in the inner while loop here.

The other caller of blk_mq_dispatch_list loops until the list is empty,
why don't we need that here?