[PATCH 1/5] nvme-fc: don't hold rport lock when putting ctrl

[Justin Tee]

Hi Daniel,

> nvme_fc_ctrl_put can acquire the rport lock when freeing the
> ctrl object:
>
> nvme_fc_ctrl_put
>   nvme_fc_ctrl_free
>     spin_lock_irqsave(rport->lock)
>
> Thus we can't hold the rport lock when calling nvme_fc_ctrl_put.

While I agree that we can’t hold the rport lock when calling
nvme_fc_ctrl_put, nvme_fc_ctrl_free also does a nvme_fc_rport_put,
which could also trigger nvme_fc_free_rport, making rport invalid.
Should we also add kref get on the rport before entering the
list_for_each_entry loop?

Also, because nvme_fc_ctrl_free removes itself from the
rport->ctrl_list, should we also start using list_for_each_entry_safe?

So, something like this?

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 03987f497a5b..fcd30801921b 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1468,14 +1468,16 @@ nvme_fc_match_disconn_ls(struct nvme_fc_rport *rport,
 {
        struct fcnvme_ls_disconnect_assoc_rqst *rqst =
                                        &lsop->rqstbuf->rq_dis_assoc;
-       struct nvme_fc_ctrl *ctrl, *ret = NULL;
+       struct nvme_fc_ctrl *ctrl, *tmp, *ret = NULL;
        struct nvmefc_ls_rcv_op *oldls = NULL;
        u64 association_id = be64_to_cpu(rqst->associd.association_id);
        unsigned long flags;

+       nvme_fc_rport_get(rport);
+
        spin_lock_irqsave(&rport->lock, flags);

-       list_for_each_entry(ctrl, &rport->ctrl_list, ctrl_list) {
+       list_for_each_entry_safe(ctrl, tmp, &rport->ctrl_list, ctrl_list) {
                if (!nvme_fc_ctrl_get(ctrl))
                        continue;
                spin_lock(&ctrl->lock);
@@ -1488,11 +1490,15 @@ nvme_fc_match_disconn_ls(struct nvme_fc_rport *rport,
                if (ret)
                        /* leave the ctrl get reference */
                        break;
+               spin_unlock_irqrestore(&rport->lock, flags);
                nvme_fc_ctrl_put(ctrl);
+               spin_lock_irqsave(&rport->lock, flags);
        }

        spin_unlock_irqrestore(&rport->lock, flags);

+       nvme_fc_rport_put(rport);
+
        /* transmit a response for anything that was pending */
        if (oldls) {
                dev_info(rport->lport->dev,

Regards,
Justin