On Wed, 2017-05-03 at 11:24 -0600, Jens Axboe wrote:
> 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
> [ ... ]
> @@ -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);
>  
>       /*

Hello Jens,

How about replacing the blk_freeze_queue_start() and blk_mq_stop_hw_queues()
calls by a single call to blk_set_queue_dying()? I think that would be more
appropriate in the context of mtip_block_remove().

> 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");

A similar comment here: since nbd_clear_que() is called just before shutting
down the block layer queue in the NBD driver, how about replacing the
blk_mq_stop_hw_queues() / blk_mq_start_hw_queues() pair by a single 
blk_set_queue_dying() call?

> 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;

Since the above blk_mq_stop_hw_queues() call is followed by a .del_vqs() call,
shouldn't the blk_mq_stop_hw_queues() / blk_mq_start_hw_queues() pair in the
virtio_blk driver be changed into a blk_mq_freeze_queue() / 
blk_mq_unfreeze_queue()
pair?

Thanks,

Bart.

Reply via email to