nvmet/fc.c acquiring qlock after kref_put

Engel, Amit Amit.Engel at Dell.com
Tue Jun 18 03:36:43 PDT 2024


Hi James,

Have a question about nvmet/fc.c code that was committed by you:
In nvmet/fc.c function nvmet_fc_free_fcp_iod(), there is a call to spin_lock_irqsave(&queue->qlock, flags) right after a call to nvmet_fc_tgt_q_put(queue); 
Which means that, as a result of nvmet_fc_tgt_q_put() the queue might be freed (kref_put)
At this point, trying to acquire the queue->qlock does not make sense.
Am I missing something or its really an issue?

BEFORE the below commit the nvmet_fc_tgt_q_put(queue); was called only AFTER spin_unlock_irqrestore(&queue->qlock, flags); - which does make sense to me.

Thanks
Amit E

commit 619c62dcc62b957d17cccde2081cad527b020883
Author: James Smart mailto:jsmart2021 at gmail.com
Date:   Fri Nov 10 15:38:45 2017 -0800

    nvmet-fc: correct ref counting error when deferred rcv used
    
    Whenever a cmd is received a reference is taken while looking up the
    queue. The reference is removed after the cmd is done as the iod is
    returned for reuse. The fod may be reused for a deferred (recevied but
    no job context) cmd.  Existing code removes the reference only if the
    fod is not reused for another command. Given the fod may be used for
    one or more ios, although a reference was taken per io, it won't be
    matched on the frees.
    
    Remove the reference on every fod free. This pairs the references to
    each io.
    
    Signed-off-by: James Smart mailto:james.smart at broadcom.com
    Reviewed-by: Sagi Grimberg mailto:sagi at grimberg.me
    Signed-off-by: Christoph Hellwig mailto:hch at lst.de

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 664d3013f68f..5fd86039e353 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -533,15 +533,15 @@ nvmet_fc_free_fcp_iod(struct nvmet_fc_tgt_queue *queue,
 
        tgtport->ops->fcp_req_release(&tgtport->fc_target_port, fcpreq);
 
+       /* release the queue lookup reference on the completed IO */
+       nvmet_fc_tgt_q_put(queue);
+
        spin_lock_irqsave(&queue->qlock, flags);
        deferfcp = list_first_entry_or_null(&queue->pending_cmd_list,
                                struct nvmet_fc_defer_fcp_req, req_list);
        if (!deferfcp) {
                list_add_tail(&fod->fcp_list, &fod->queue->fod_list);
                spin_unlock_irqrestore(&queue->qlock, flags);
-
-               /* Release reference taken at queue lookup and fod allocation */
-               nvmet_fc_tgt_q_put(queue);
                return;
        }
 
@@ -760,6 +760,9 @@ nvmet_fc_delete_target_queue(struct nvmet_fc_tgt_queue *queue)
                tgtport->ops->fcp_req_release(&tgtport->fc_target_port,
                                deferfcp->fcp_req);
 
+               /* release the queue lookup reference */
+               nvmet_fc_tgt_q_put(queue);
+
                kfree(deferfcp);
 
                spin_lock_irqsave(&queue->qlock, flags);


More information about the Linux-nvme mailing list