Now target is removed from nvme_fc_ctrl_free() which is the ctrl->ref release handler. And even admin queue is unquiesced there, this way is definitely wrong because the ctr->ref is grabbed when submitting command.
And Marco observed that nvme_fc_ctrl_free() can be called from request completion code path, and trigger kernel warning since request completes from softirq context.
Fix the issue by moveing target removal into nvme_fc_delete_ctrl(), which is also aligned with nvme-tcp and nvme-rdma.
Patch originally proposed by Ming Lei, then modified to move the tagset removal down to after nvme_fc_delete_association() after further testing.
Cc: Marco Patalano mpatalan@redhat.com Cc: Ewan Milne emilne@redhat.com Cc: James Smart james.smart@broadcom.com Cc: Sagi Grimberg sagi@grimberg.me Signed-off-by: Ming Lei ming.lei@redhat.com Cc: stable@vger.kernel.org Tested-by: Marco Patalano mpatalan@redhat.com Signed-off-by: Ewan D. Milne emilne@redhat.com --- drivers/nvme/host/fc.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c index b613fc5966a7..9e1841223e8a 100644 --- a/drivers/nvme/host/fc.c +++ b/drivers/nvme/host/fc.c @@ -2359,17 +2359,11 @@ nvme_fc_ctrl_free(struct kref *ref) container_of(ref, struct nvme_fc_ctrl, ref); unsigned long flags;
- if (ctrl->ctrl.tagset) - nvme_remove_io_tag_set(&ctrl->ctrl); - /* remove from rport list */ spin_lock_irqsave(&ctrl->rport->lock, flags); list_del(&ctrl->ctrl_list); spin_unlock_irqrestore(&ctrl->rport->lock, flags);
- nvme_unquiesce_admin_queue(&ctrl->ctrl); - nvme_remove_admin_tag_set(&ctrl->ctrl); - kfree(ctrl->queues);
put_device(ctrl->dev); @@ -3265,11 +3259,18 @@ nvme_fc_delete_ctrl(struct nvme_ctrl *nctrl)
cancel_work_sync(&ctrl->ioerr_work); cancel_delayed_work_sync(&ctrl->connect_work); + /* * kill the association on the link side. this will block * waiting for io to terminate */ nvme_fc_delete_association(ctrl); + + if (ctrl->ctrl.tagset) + nvme_remove_io_tag_set(&ctrl->ctrl); + + nvme_unquiesce_admin_queue(&ctrl->ctrl); + nvme_remove_admin_tag_set(&ctrl->ctrl); }
static void