Re: [PATCH v2] blk-mq: use a housekeeping CPU for passthrough requests on isolated hctxs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] blk-mq: blk_mq_alloc_request_hctx() allocates on offline hctxs when BLK_MQ_S_INACTIVE is unset
--

commit f6824e249591ee431516fa47caa778074d8f4d22
Author: Daniel Wagner <dwagner@suse.de>

blk-mq: use a housekeeping CPU for passthrough requests on isolated hctxs

When hctx->cpumask contains no online CPUs because all CPUs mapped to the
hardware context are isolated, this commit allows passthrough requests in
blk_mq_alloc_request_hctx() to fall back to the software context of an
online housekeeping CPU unless BLK_MQ_S_INACTIVE is set on the hctx.

> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 5fca78778c31c..77685e6283fa1 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c

[ ... ]

> @@ -767,8 +786,22 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
>  	if (!blk_mq_hw_queue_mapped(data.hctx))
>  		goto out_queue_exit;
>  	cpu = cpumask_first_and(data.hctx->cpumask, cpu_online_mask);
> -	if (cpu >= nr_cpu_ids)
> -		goto out_queue_exit;
> +	if (cpu >= nr_cpu_ids) {
> +		/*
> +		 * Isolated CPUs are not part of hctx->cpumask, so it can be
> +		 * empty although online CPUs are mapped to this hardware
> +		 * context. Passthrough requests are inserted directly into
> +		 * hctx->dispatch, so use the software context of a
> +		 * housekeeping CPU for them. Inactive hardware contexts have
> +		 * no online CPU left.
> +		 */
> +		if (!blk_op_is_passthrough(opf) ||
> +		    test_bit(BLK_MQ_S_INACTIVE, &data.hctx->state))
> +			goto out_queue_exit;
> +		cpu = blk_mq_housekeeping_cpu();
> +		if (cpu >= nr_cpu_ids)
> +			goto out_queue_exit;
> +	}

[Severity: Medium]
Can this allocate a passthrough request on a hardware context that has no
online CPUs mapped to it instead of returning ERR_PTR(-EXDEV)?

BLK_MQ_S_INACTIVE is only set in the CPU hotplug callback
blk_mq_hctx_notify_offline() when the last online CPU mapped to an hctx
transitions to offline:

block/blk-mq.c:blk_mq_hctx_notify_offline() {
	...
	if (!hctx->nr_ctx || blk_mq_hctx_has_online_cpu(hctx, cpu))
		return 0;

	set_bit(BLK_MQ_S_INACTIVE, &hctx->state);
	...
}

However, during queue initialization in blk_mq_init_allocated_queue() and
queue reconfiguration in blk_mq_update_nr_hw_queues(), blk_mq_init_hctx()
unconditionally clears BLK_MQ_S_INACTIVE, __blk_mq_add_cpuhp() registers the
hotplug state via cpuhp_state_add_instance_nocalls() (and skips registration
for BLK_MQ_F_STACKING), and blk_mq_map_swqueue() maps all present and
possible CPUs to hardware contexts without setting BLK_MQ_S_INACTIVE on
hctxs whose mapped CPUs are all currently offline.

If a queue is initialized or remapped while all CPUs assigned to data.hctx
are already offline, blk_mq_hw_queue_mapped(data.hctx) is true and
BLK_MQ_S_INACTIVE remains clear. When blk_mq_alloc_request_hctx() is called
for that hctx_idx with a passthrough operation, test_bit(BLK_MQ_S_INACTIVE,
&data.hctx->state) evaluates to false and the request is allocated using
blk_mq_housekeeping_cpu() on an offline hardware queue instead of failing
with -EXDEV.

Should this check !blk_mq_hctx_has_online_cpu(data.hctx, nr_cpu_ids) (or
ensure blk_mq_map_swqueue() sets BLK_MQ_S_INACTIVE when an hctx has no
online CPUs mapped)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-nvme-fabrics-isolcpu-v2-1-95ec70d58254@suse.de?part=1