Re: [PATCH 0/4] blk-mq: support to use hw tag for scheduling

Jens Axboe
2017-05-03T17:24:56Z

On 05/03/2017 11:15 AM, Bart Van Assche wrote:
> On Wed, 2017-05-03 at 11:07 -0600, Jens Axboe wrote:
>> +void blk_mq_stop_hw_queues(struct request_queue *q)
>> +{
>> + __blk_mq_stop_hw_queues(q, false);
>> }
>> EXPORT_SYMBOL(blk_mq_stop_hw_queues);
>
> Hello Jens,
>
> So the approach of this patch is to make all blk_mq_stop_hw_queue()
> and blk_mq_stop_hw_queues() callers cancel run_work without waiting?
> That should make the BUG reported by Ming disappear. However, I think
> we may want to review all calls from block drivers to
> blk_mq_stop_hw_queues(). There are drivers that call this function to
> quiesce I/O so I think these need the synchronous work cancellation
> ...

I took a look at the drivers, and it's trivial enough to do. Most of
them will work fine with a sync block for stopping _all_ queues. See
below.

diff --git a/block/blk-mq.c b/block/blk-mq.c
index e339247a2570..9dcb9592dbf9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -166,7 +166,7 @@ void blk_mq_quiesce_queue(struct request_queue *q)
unsigned int i;
bool rcu = false;

- blk_mq_stop_hw_queues(q);
+ blk_mq_stop_hw_queues(q, true);

queue_for_each_hw_ctx(q, hctx, i) {
if (hctx->flags & BLK_MQ_F_BLOCKING)
@@ -1218,20 +1218,29 @@ bool blk_mq_queue_stopped(struct request_queue *q)
}
EXPORT_SYMBOL(blk_mq_queue_stopped);

-void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx)
+static void __blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx, bool sync)
{
- cancel_delayed_work_sync(&hctx->run_work);
+ if (sync)
+ cancel_delayed_work_sync(&hctx->run_work);
+ else
+ cancel_delayed_work(&hctx->run_work);
+
set_bit(BLK_MQ_S_STOPPED, &hctx->state);
}
+
+void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx)
+{
+ __blk_mq_stop_hw_queue(hctx, false);
+}
EXPORT_SYMBOL(blk_mq_stop_hw_queue);

-void blk_mq_stop_hw_queues(struct request_queue *q)
+void blk_mq_stop_hw_queues(struct request_queue *q, bool sync)
{
struct blk_mq_hw_ctx *hctx;
int i;

queue_for_each_hw_ctx(q, hctx, i)
- blk_mq_stop_hw_queue(hctx);
+ __blk_mq_stop_hw_queue(hctx, sync);
}
EXPORT_SYMBOL(blk_mq_stop_hw_queues);

diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 3a779a4f5653..33b5d1382c45 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -950,7 +950,7 @@ static int mtip_quiesce_io(struct mtip_port *port, unsigned long timeout)
unsigned long to;
bool active = true;

- blk_mq_stop_hw_queues(port->dd->queue);
+ blk_mq_stop_hw_queues(port->dd->queue, true);

to = jiffies + msecs_to_jiffies(timeout);
do {
@@ -4009,7 +4009,7 @@ static int mtip_block_remove(struct driver_data *dd)
dd->disk->disk_name);

blk_freeze_queue_start(dd->queue);
- blk_mq_stop_hw_queues(dd->queue);
+ blk_mq_stop_hw_queues(dd->queue, true);
blk_mq_tagset_busy_iter(&dd->tags, mtip_no_dev_cleanup, dd);

/*
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 6b98ec2a3824..ce5490938232 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -656,7 +656,7 @@ static void nbd_clear_req(struct request *req, void *data, bool reserved)

static void nbd_clear_que(struct nbd_device *nbd)
{
- blk_mq_stop_hw_queues(nbd->disk->queue);
+ blk_mq_stop_hw_queues(nbd->disk->queue, true);
blk_mq_tagset_busy_iter(&nbd->tag_set, nbd_clear_req, NULL);
blk_mq_start_hw_queues(nbd->disk->queue);
dev_dbg(disk_to_dev(nbd->disk), "queue cleared\n");
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 94173de1efaa..a73d2823cdb2 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -844,7 +844,7 @@ static int virtblk_freeze(struct virtio_device *vdev)
/* Make sure no work handler is accessing the device. */
flush_work(&vblk->config_work);

- blk_mq_stop_hw_queues(vblk->disk->queue);
+ blk_mq_stop_hw_queues(vblk->disk->queue, true);

vdev->config->del_vqs(vdev);
return 0;
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 70e689bf1cad..b6891e4af837 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1969,7 +1969,7 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
return BLK_MQ_RQ_QUEUE_ERROR;

if (op->rq) {
- blk_mq_stop_hw_queues(op->rq->q);
+ blk_mq_stop_hw_queues(op->rq->q, false);
blk_mq_delay_queue(queue->hctx, NVMEFC_QUEUE_DELAY);
}
return BLK_MQ_RQ_QUEUE_BUSY;
@@ -2478,7 +2478,7 @@ nvme_fc_delete_association(struct nvme_fc_ctrl *ctrl)
* use blk_mq_tagset_busy_itr() and the transport routine to
* terminate the exchanges.
*/
- blk_mq_stop_hw_queues(ctrl->ctrl.admin_q);
+ blk_mq_stop_hw_queues(ctrl->ctrl.admin_q, true);
blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
nvme_fc_terminate_exchange, &ctrl->ctrl);

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 56a315bd4d96..6e87854eddd5 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1084,7 +1084,7 @@ static int nvme_suspend_queue(struct nvme_queue *nvmeq)
spin_unlock_irq(&nvmeq->q_lock);

if (!nvmeq->qid && nvmeq->dev->ctrl.admin_q)
- blk_mq_stop_hw_queues(nvmeq->dev->ctrl.admin_q);
+ blk_mq_stop_hw_queues(nvmeq->dev->ctrl.admin_q, true);

free_irq(vector, nvmeq);

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index dd1c6deef82f..b478839c5d79 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -792,7 +792,7 @@ static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work)
return;

stop_admin_q:
- blk_mq_stop_hw_queues(ctrl->ctrl.admin_q);
+ blk_mq_stop_hw_queues(ctrl->ctrl.admin_q, true);
requeue:
dev_info(ctrl->ctrl.device, "Failed reconnect attempt %d\n",
ctrl->ctrl.opts->nr_reconnects);
@@ -814,7 +814,7 @@ static void nvme_rdma_error_recovery_work(struct work_struct *work)

if (ctrl->queue_count > 1)
nvme_stop_queues(&ctrl->ctrl);
- blk_mq_stop_hw_queues(ctrl->ctrl.admin_q);
+ blk_mq_stop_hw_queues(ctrl->ctrl.admin_q, true);

/* We must take care of fastfail/requeue all our inflight requests */
if (ctrl->queue_count > 1)
@@ -1651,7 +1651,7 @@ static void nvme_rdma_shutdown_ctrl(struct nvme_rdma_ctrl *ctrl)
if (test_bit(NVME_RDMA_Q_CONNECTED, &ctrl->queues[0].flags))
nvme_shutdown_ctrl(&ctrl->ctrl);

- blk_mq_stop_hw_queues(ctrl->ctrl.admin_q);
+ blk_mq_stop_hw_queues(ctrl->ctrl.admin_q, true);
blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
nvme_cancel_request, &ctrl->ctrl);
nvme_rdma_destroy_admin_queue(ctrl);
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index feb497134aee..ea0911e36f2d 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -446,7 +446,7 @@ static void nvme_loop_shutdown_ctrl(struct nvme_loop_ctrl *ctrl)
if (ctrl->ctrl.state == NVME_CTRL_LIVE)
nvme_shutdown_ctrl(&ctrl->ctrl);

- blk_mq_stop_hw_queues(ctrl->ctrl.admin_q);
+ blk_mq_stop_hw_queues(ctrl->ctrl.admin_q, true);
blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
nvme_cancel_request, &ctrl->ctrl);
nvme_loop_destroy_admin_queue(ctrl);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 327b10206d63..64100c6b5811 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2976,7 +2976,7 @@ scsi_internal_device_block(struct scsi_device *sdev, bool wait)
if (wait)
blk_mq_quiesce_queue(q);
else
- blk_mq_stop_hw_queues(q);
+ blk_mq_stop_hw_queues(q, true);
} else {
spin_lock_irqsave(q->queue_lock, flags);
blk_stop_queue(q);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index a104832e7ae5..1f6684aa465f 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -239,7 +239,7 @@ void blk_mq_complete_request(struct request *rq);
bool blk_mq_queue_stopped(struct request_queue *q);
void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx);
void blk_mq_start_hw_queue(struct blk_mq_hw_ctx *hctx);
-void blk_mq_stop_hw_queues(struct request_queue *q);
+void blk_mq_stop_hw_queues(struct request_queue *q, bool sync);
void blk_mq_start_hw_queues(struct request_queue *q);
void blk_mq_start_stopped_hw_queue(struct blk_mq_hw_ctx *hctx, bool async);
void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async);

--
Jens Axboe