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: signature.asc
Description: PGP signature

Reply via email to