Re: [PATCH v9 10/13] blk-mq: use hk cpus only when isolcpus=io_queue is enabled

Aaron Tomlin
2026-04-01T17:16:09Z

[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]

On Tue, Mar 31, 2026 at 05:05:07PM -0600, Keith Busch wrote:
> > +static bool blk_mq_validate(struct blk_mq_queue_map *qmap,
> > + const struct cpumask *active_hctx)
> > +{
> > + /*
> > + * Verify if the mapping is usable when housekeeping
> > + * configuration is enabled
> > + */
> > +
> > + for (int queue = 0; queue < qmap->nr_queues; queue++) {
> > + int cpu;

Hi Keith,

Thank you for taking the time to review and test.

> > +
> > + if (cpumask_test_cpu(queue, active_hctx)) {
> > + /*
> > + * This htcx has at least one online CPU thus it
>
> Typo, should say "hctx".

Acknowledged.

> > + * is able to serve any assigned isolated CPU.
> > + */
> > + continue;
> > + }
> > +
> > + /*
> > + * There is no housekeeping online CPU for this hctx, all
> > + * good as long as all non houskeeping CPUs are also
>
> Typo, "housekeeping".

Acknowledged.

> ...
>
> > void blk_mq_map_queues(struct blk_mq_queue_map *qmap)
> > {
> > - const struct cpumask *masks;
> > + struct cpumask *masks __free(kfree) = NULL;
> > + const struct cpumask *constraint;
> > unsigned int queue, cpu, nr_masks;
> > + cpumask_var_t active_hctx;
> >
> > - masks = group_cpus_evenly(qmap->nr_queues, &nr_masks);
> > - if (!masks) {
> > - for_each_possible_cpu(cpu)
> > - qmap->mq_map[cpu] = qmap->queue_offset;
> > - return;
> > - }
> > + if (!zalloc_cpumask_var(&active_hctx, GFP_KERNEL))
> > + goto fallback;
> > +
> > + if (housekeeping_enabled(HK_TYPE_IO_QUEUE))
> > + constraint = housekeeping_cpumask(HK_TYPE_IO_QUEUE);
> > + else
> > + constraint = cpu_possible_mask;
> > +
> > + /* Map CPUs to the hardware contexts (hctx) */
> > + masks = group_mask_cpus_evenly(qmap->nr_queues, constraint, &nr_masks);
> > + if (!masks)
> > + goto free_fallback;
> >
> > for (queue = 0; queue < qmap->nr_queues; queue++) {
> > - for_each_cpu(cpu, &masks[queue % nr_masks])
> > - qmap->mq_map[cpu] = qmap->queue_offset + queue;
> > + unsigned int idx = (qmap->queue_offset + queue) % nr_masks;
> > +
> > + for_each_cpu(cpu, &masks[idx]) {
> > + qmap->mq_map[cpu] = idx;
>
> I think there's something off with this when we have multiple queue maps. The
> wrapping loses the offset when we've isolated CPUs, so I think the index would
> end up incorrect.
>
> Trying this series out when "nvme.poll_queues=2" with isolcpus set, I am
> getting a kernel panic:

To resolve this comprehensively, I have made the following adjustments:

1. qmap->mq_map now strictly stores the absolute hardware index
(qmap->queue_offset + queue) across both the primary assignment
loop and the unassigned CPU fallback loop.

2. The active_hctx cpumask now strictly tracks the relative index
(queue) for the current map being processed, preventing
out-of-bounds mask tracking.

3. The validation check within blk_mq_validate() has been updated to
correctly compare the absolute index stored in mq_map against the
offset-adjusted queue index.

I shall fold these corrections directly into the next series.

Best regards,
--
Aaron Tomlin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]