diff mbox series

[2/5] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync

Message ID 20200217210839.28535-3-bvanassche@acm.org (mailing list archive)
State New, archived
Headers show
Series Five patches related to changing the number of hardware queues | expand

Commit Message

Bart Van Assche Feb. 17, 2020, 9:08 p.m. UTC
This patch fixes the following kernel warning:

WARNING: CPU: 0 PID: 2501 at include/linux/cpumask.h:137
Call Trace:
 blk_mq_run_hw_queue+0x19d/0x350 block/blk-mq.c:1508
 blk_mq_run_hw_queues+0x112/0x1a0 block/blk-mq.c:1525
 blk_mq_requeue_work+0x502/0x780 block/blk-mq.c:775
 process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
 worker_thread+0x98/0xe40 kernel/workqueue.c:2415
 kthread+0x361/0x430 kernel/kthread.c:255

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Reported-by: syzbot+d44e1b26ce5c3e77458d@syzkaller.appspotmail.com
Fixes: ed76e329d74a ("blk-mq: abstract out queue map") # v5.0
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Ming Lei Feb. 18, 2020, 3:16 a.m. UTC | #1
On Mon, Feb 17, 2020 at 01:08:36PM -0800, Bart Van Assche wrote:
> This patch fixes the following kernel warning:
> 
> WARNING: CPU: 0 PID: 2501 at include/linux/cpumask.h:137
> Call Trace:
>  blk_mq_run_hw_queue+0x19d/0x350 block/blk-mq.c:1508
>  blk_mq_run_hw_queues+0x112/0x1a0 block/blk-mq.c:1525
>  blk_mq_requeue_work+0x502/0x780 block/blk-mq.c:775
>  process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
>  worker_thread+0x98/0xe40 kernel/workqueue.c:2415
>  kthread+0x361/0x430 kernel/kthread.c:255
> 
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Johannes Thumshirn <jth@kernel.org>
> Reported-by: syzbot+d44e1b26ce5c3e77458d@syzkaller.appspotmail.com
> Fixes: ed76e329d74a ("blk-mq: abstract out queue map") # v5.0
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  block/blk-mq.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index f298500e6dda..2b9f490f5a64 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3050,6 +3050,16 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
>  	}
>  }
>  
> +static void blk_mq_set_nr_hw_queues(struct blk_mq_tag_set *set,
> +				    int new_nr_hw_queues)
> +{
> +	int i;
> +
> +	set->nr_hw_queues = new_nr_hw_queues;
> +	for (i = 0; i < set->nr_maps; i++)
> +		set->map[i].nr_queues = new_nr_hw_queues;
> +}
> +
>  static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
>  				  int cur_nr_hw_queues, int new_nr_hw_queues)
>  {
> @@ -3068,7 +3078,7 @@ static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
>  		       sizeof(*set->tags));
>  	kfree(set->tags);
>  	set->tags = new_tags;
> -	set->nr_hw_queues = new_nr_hw_queues;
> +	blk_mq_set_nr_hw_queues(set, new_nr_hw_queues);
>  
>  	return 0;
>  }
> @@ -3330,7 +3340,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>  		goto reregister;
>  
>  	prev_nr_hw_queues = set->nr_hw_queues;
> -	set->nr_hw_queues = nr_hw_queues;
> +	blk_mq_set_nr_hw_queues(set, nr_hw_queues);
>  	blk_mq_update_queue_map(set);
>  fallback:
>  	list_for_each_entry(q, &set->tag_list, tag_set_list) {
> @@ -3338,7 +3348,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>  		if (q->nr_hw_queues != set->nr_hw_queues) {
>  			pr_warn("Increasing nr_hw_queues to %d fails, fallback to %d\n",
>  					nr_hw_queues, prev_nr_hw_queues);
> -			set->nr_hw_queues = prev_nr_hw_queues;
> +			blk_mq_set_nr_hw_queues(set, prev_nr_hw_queues);
>  			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
>  			goto fallback;
>  		}
> 

I guess the issue can be fixed by the following change, and we do not
need to touch each queue map:

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 5f5c43ae3792..f7340afb89ec 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3046,6 +3046,7 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
                return set->ops->map_queues(set);
        } else {
                BUG_ON(set->nr_maps > 1);
+               set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
                return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
        }
 }


Thanks,
Ming
Bart Van Assche Feb. 19, 2020, 4:24 a.m. UTC | #2
On 2020-02-17 19:16, Ming Lei wrote:
> I guess the issue can be fixed by the following change, and we do not
> need to touch each queue map:
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 5f5c43ae3792..f7340afb89ec 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3046,6 +3046,7 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
>                 return set->ops->map_queues(set);
>         } else {
>                 BUG_ON(set->nr_maps > 1);
> +               set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
>                 return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
>         }
>  }

Every caller of blk_mq_map_queues() needs to set the number of queues
in the default queue map so I would like to make the number of queues an
argument. How about the (untested) patch below?

Thanks,

Bart.

---
 block/blk-mq-cpumap.c                 |  5 +++--
 block/blk-mq-rdma.c                   |  2 +-
 block/blk-mq-virtio.c                 |  2 +-
 block/blk-mq.c                        |  6 ++++--
 drivers/block/virtio_blk.c            |  5 +++--
 drivers/nvme/host/pci.c               |  2 +-
 drivers/nvme/host/rdma.c              |  5 ++---
 drivers/nvme/host/tcp.c               | 11 ++++++-----
 drivers/scsi/qla2xxx/qla_os.c         |  8 +++++---
 drivers/scsi/scsi_lib.c               |  3 ++-
 drivers/scsi/smartpqi/smartpqi_init.c |  5 +++--
 drivers/scsi/virtio_scsi.c            |  1 +
 include/linux/blk-mq.h                |  2 +-
 13 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 0157f2b3485a..4c0d5768305a 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -32,12 +32,13 @@ static int get_first_sibling(unsigned int cpu)
 	return cpu;
 }

-int blk_mq_map_queues(struct blk_mq_queue_map *qmap)
+int blk_mq_map_queues(struct blk_mq_queue_map *qmap, unsigned int nr_queues)
 {
 	unsigned int *map = qmap->mq_map;
-	unsigned int nr_queues = qmap->nr_queues;
 	unsigned int cpu, first_sibling, q = 0;

+	qmap->nr_queues = nr_queues;
+
 	for_each_possible_cpu(cpu)
 		map[cpu] = -1;

diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c
index 14f968e58b8f..c32af1231244 100644
--- a/block/blk-mq-rdma.c
+++ b/block/blk-mq-rdma.c
@@ -39,6 +39,6 @@ int blk_mq_rdma_map_queues(struct blk_mq_queue_map *map,
 	return 0;

 fallback:
-	return blk_mq_map_queues(map);
+	return blk_mq_map_queues(map, map->nr_queues);
 }
 EXPORT_SYMBOL_GPL(blk_mq_rdma_map_queues);
diff --git a/block/blk-mq-virtio.c b/block/blk-mq-virtio.c
index 488341628256..b5911cb43974 100644
--- a/block/blk-mq-virtio.c
+++ b/block/blk-mq-virtio.c
@@ -41,6 +41,6 @@ int blk_mq_virtio_map_queues(struct blk_mq_queue_map *qmap,

 	return 0;
 fallback:
-	return blk_mq_map_queues(qmap);
+	return blk_mq_map_queues(qmap, qmap->nr_queues);
 }
 EXPORT_SYMBOL_GPL(blk_mq_virtio_map_queues);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f298500e6dda..bbd9edde2b6f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3046,7 +3046,8 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
 		return set->ops->map_queues(set);
 	} else {
 		BUG_ON(set->nr_maps > 1);
-		return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
+		return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+					 set->nr_hw_queues);
 	}
 }

@@ -3339,7 +3340,8 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 			pr_warn("Increasing nr_hw_queues to %d fails, fallback to %d\n",
 					nr_hw_queues, prev_nr_hw_queues);
 			set->nr_hw_queues = prev_nr_hw_queues;
-			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
+			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+					  set->nr_hw_queues);
 			goto fallback;
 		}
 		blk_mq_map_swqueue(q);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 54158766334b..61d6482a0b9e 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -595,9 +595,10 @@ static int virtblk_init_request(struct blk_mq_tag_set *set, struct request *rq,
 static int virtblk_map_queues(struct blk_mq_tag_set *set)
 {
 	struct virtio_blk *vblk = set->driver_data;
+	struct blk_mq_queue_map *qmap = &set->map[HCTX_TYPE_DEFAULT];

-	return blk_mq_virtio_map_queues(&set->map[HCTX_TYPE_DEFAULT],
-					vblk->vdev, 0);
+	qmap->nr_queues = set->nr_hw_queues;
+	return blk_mq_virtio_map_queues(qmap, vblk->vdev, 0);
 }

 static const struct blk_mq_ops virtio_mq_ops = {
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index da392b50f73e..7531a6950736 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -438,7 +438,7 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set)
 		if (i != HCTX_TYPE_POLL && offset)
 			blk_mq_pci_map_queues(map, to_pci_dev(dev->dev), offset);
 		else
-			blk_mq_map_queues(map);
+			blk_mq_map_queues(map, map->nr_queues);
 		qoff += map->nr_queues;
 		offset += map->nr_queues;
 	}
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 2a47c6c5007e..27d00bc7a746 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1844,12 +1844,11 @@ static int nvme_rdma_map_queues(struct blk_mq_tag_set *set)

 	if (opts->nr_poll_queues && ctrl->io_queues[HCTX_TYPE_POLL]) {
 		/* map dedicated poll queues only if we have queues left */
-		set->map[HCTX_TYPE_POLL].nr_queues =
-				ctrl->io_queues[HCTX_TYPE_POLL];
 		set->map[HCTX_TYPE_POLL].queue_offset =
 			ctrl->io_queues[HCTX_TYPE_DEFAULT] +
 			ctrl->io_queues[HCTX_TYPE_READ];
-		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL]);
+		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL],
+				  ctrl->io_queues[HCTX_TYPE_POLL]);
 	}

 	dev_info(ctrl->ctrl.device,
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 6d43b23a0fc8..c05ca7dec661 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2192,17 +2192,18 @@ static int nvme_tcp_map_queues(struct blk_mq_tag_set *set)
 			ctrl->io_queues[HCTX_TYPE_DEFAULT];
 		set->map[HCTX_TYPE_READ].queue_offset = 0;
 	}
-	blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
-	blk_mq_map_queues(&set->map[HCTX_TYPE_READ]);
+	blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+			  set->map[HCTX_TYPE_DEFAULT].nr_queues);
+	blk_mq_map_queues(&set->map[HCTX_TYPE_READ],
+			  set->map[HCTX_TYPE_READ].nr_queues);

 	if (opts->nr_poll_queues && ctrl->io_queues[HCTX_TYPE_POLL]) {
 		/* map dedicated poll queues only if we have queues left */
-		set->map[HCTX_TYPE_POLL].nr_queues =
-				ctrl->io_queues[HCTX_TYPE_POLL];
 		set->map[HCTX_TYPE_POLL].queue_offset =
 			ctrl->io_queues[HCTX_TYPE_DEFAULT] +
 			ctrl->io_queues[HCTX_TYPE_READ];
-		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL]);
+		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL],
+				  ctrl->io_queues[HCTX_TYPE_POLL]);
 	}

 	dev_info(ctrl->ctrl.device,
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index b520a980d1dc..71058bec7d98 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -7114,10 +7114,12 @@ static int qla2xxx_map_queues(struct Scsi_Host *shost)
 	scsi_qla_host_t *vha = (scsi_qla_host_t *)shost->hostdata;
 	struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];

-	if (USER_CTRL_IRQ(vha->hw) || !vha->hw->mqiobase)
-		rc = blk_mq_map_queues(qmap);
-	else
+	if (USER_CTRL_IRQ(vha->hw) || !vha->hw->mqiobase) {
+		rc = blk_mq_map_queues(qmap, shost->tag_set.nr_hw_queues);
+	} else {
+		qmap->nr_queues = shost->tag_set.nr_hw_queues;
 		rc = blk_mq_pci_map_queues(qmap, vha->hw->pdev, vha->irq_offset);
+	}
 	return rc;
 }

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 610ee41fa54c..45a4b115dadf 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1775,7 +1775,8 @@ static int scsi_map_queues(struct blk_mq_tag_set *set)

 	if (shost->hostt->map_queues)
 		return shost->hostt->map_queues(shost);
-	return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
+	return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+				 set->nr_hw_queues);
 }

 void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index b7492568e02f..18ae5ce129b2 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -5826,9 +5826,10 @@ static int pqi_slave_alloc(struct scsi_device *sdev)
 static int pqi_map_queues(struct Scsi_Host *shost)
 {
 	struct pqi_ctrl_info *ctrl_info = shost_to_hba(shost);
+	struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];

-	return blk_mq_pci_map_queues(&shost->tag_set.map[HCTX_TYPE_DEFAULT],
-					ctrl_info->pci_dev, 0);
+	qmap->nr_queues = shost->tag_set.nr_hw_queues;
+	return blk_mq_pci_map_queues(qmap, ctrl_info->pci_dev, 0);
 }

 static int pqi_getpciinfo_ioctl(struct pqi_ctrl_info *ctrl_info,
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index bfec84aacd90..9b9da0409643 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -705,6 +705,7 @@ static int virtscsi_map_queues(struct Scsi_Host *shost)
 	struct virtio_scsi *vscsi = shost_priv(shost);
 	struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];

+	qmap->nr_queues = shost->tag_set.nr_hw_queues;
 	return blk_mq_virtio_map_queues(qmap, vscsi->vdev, 2);
 }

diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 31344d5f83e2..c63b315ed0c8 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -516,7 +516,7 @@ void blk_mq_freeze_queue_wait(struct request_queue *q);
 int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
 				     unsigned long timeout);

-int blk_mq_map_queues(struct blk_mq_queue_map *qmap);
+int blk_mq_map_queues(struct blk_mq_queue_map *qmap, unsigned int nr_queues);
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);

 void blk_mq_quiesce_queue_nowait(struct request_queue *q);
diff mbox series

Patch

diff --git a/block/blk-mq.c b/block/blk-mq.c
index f298500e6dda..2b9f490f5a64 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3050,6 +3050,16 @@  static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
 	}
 }
 
+static void blk_mq_set_nr_hw_queues(struct blk_mq_tag_set *set,
+				    int new_nr_hw_queues)
+{
+	int i;
+
+	set->nr_hw_queues = new_nr_hw_queues;
+	for (i = 0; i < set->nr_maps; i++)
+		set->map[i].nr_queues = new_nr_hw_queues;
+}
+
 static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
 				  int cur_nr_hw_queues, int new_nr_hw_queues)
 {
@@ -3068,7 +3078,7 @@  static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
 		       sizeof(*set->tags));
 	kfree(set->tags);
 	set->tags = new_tags;
-	set->nr_hw_queues = new_nr_hw_queues;
+	blk_mq_set_nr_hw_queues(set, new_nr_hw_queues);
 
 	return 0;
 }
@@ -3330,7 +3340,7 @@  static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 		goto reregister;
 
 	prev_nr_hw_queues = set->nr_hw_queues;
-	set->nr_hw_queues = nr_hw_queues;
+	blk_mq_set_nr_hw_queues(set, nr_hw_queues);
 	blk_mq_update_queue_map(set);
 fallback:
 	list_for_each_entry(q, &set->tag_list, tag_set_list) {
@@ -3338,7 +3348,7 @@  static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 		if (q->nr_hw_queues != set->nr_hw_queues) {
 			pr_warn("Increasing nr_hw_queues to %d fails, fallback to %d\n",
 					nr_hw_queues, prev_nr_hw_queues);
-			set->nr_hw_queues = prev_nr_hw_queues;
+			blk_mq_set_nr_hw_queues(set, prev_nr_hw_queues);
 			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
 			goto fallback;
 		}