> +static void nvme_rdma_destroy_admin_queue(struct nvme_rdma_ctrl *ctrl, bool 
> remove)
>  {
> +     nvme_rdma_stop_queue(&ctrl->queues[0]);
> +     if (remove) {
> +             blk_cleanup_queue(ctrl->ctrl.admin_connect_q);
> +             blk_cleanup_queue(ctrl->ctrl.admin_q);
> +             blk_mq_free_tag_set(&ctrl->admin_tag_set);
> +             nvme_rdma_dev_put(ctrl->device);
> +     }
> +
>       nvme_rdma_free_qe(ctrl->queues[0].device->dev, &ctrl->async_event_sqe,
>                       sizeof(struct nvme_command), DMA_TO_DEVICE);
> +     nvme_rdma_free_queue(&ctrl->queues[0]);

I don't like the calling convention.  We only have have two callers
anyway.  So I'd much rather only keep the code inside the if above
in the new nvme_rdma_destroy_admin_queue that is only called at shutdown
time, and opencode the calls to nvme_rdma_stop_queue, nvme_rdma_free_qe
and nvme_rdma_free_queue in the callers.

> -static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl)
> +static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl, bool 
> new)

PCIe is just checking for a non-null admin_q.  But I think we should
jsut split this into two functions, one for the shared code at the end
and one just for the first-time setup, with the nvme_rdma_init_queue
call open coded.

>       error = nvmf_connect_admin_queue(&ctrl->ctrl);
> @@ -1596,6 +1601,8 @@ static int nvme_rdma_configure_admin_queue(struct 
> nvme_rdma_ctrl *ctrl)
>  
>       set_bit(NVME_RDMA_Q_LIVE, &ctrl->queues[0].flags);
>  
> +     blk_mq_start_stopped_hw_queues(ctrl->ctrl.admin_q, true);
> +

Where does this come from?

Reply via email to