[PATCH 2/5] nvmet: cq: prepare for completion queue sharing

[Wilfred Mallawa]

On Wed, 2025-05-07 at 16:25 +0900, Damien Le Moal wrote:
> On 4/24/25 2:13 PM, Wilfred Mallawa wrote:
> > From: Wilfred Mallawa <wilfred.mallawa at wdc.com>
> > 
> > For the PCI transport, the NVMe specification allows submission
> > queues to
> > share completion queues, however, this is not supported in the
> > current
> > NVMe target implementation. This is a preparatory patch to allow
> > for
> > completion queue (CQ) sharing between different submission queues
> > (SQ).
> > 
> > To support queue sharing, reference counting completion queues is
> > required. This patch adds the refcount_t field ref to struct
> > nvmet_cq
> > coupled with respective nvmet_cq_init(), nvmet_cq_get(),
> > nvmet_cq_put(),
> > nvmet_cq_is_deletable() and nvmet_cq_destroy() functions.
> > 
> > A CQ reference count is initialized with nvmet_cq_init() when a CQ
> > is
> > created. Using nvmet_cq_get(), a reference to a CQ is taken when an
> > SQ is
> > created that uses the respective CQ. Similarly. when an SQ is
> > destroyed,
> > the reference count to the respective CQ from the SQ being
> > destroyed is
> > decremented with nvmet_cq_put(). The last reference to a CQ is
> > dropped
> > on a CQ deletion using nvmet_cq_put(), which invokes
> > nvmet_cq_destroy()
> > to fully cleanup after the CQ. The helper function
> > nvmet_cq_in_use() is
> > used to determine if any SQs are still using the CQ pending
> > deletion.
> > In which case, the CQ must not be deleted. This should protect
> > scenarios
> > where a bad host may attempt to delete a CQ without first having
> > deleted
> > SQ(s) using that CQ.
> > 
> > Additionally, this patch adds an array of struct nvmet_cq to the
> > nvmet_ctrl structure. This allows for the controller to keep track
> > of CQs
> > as they are created and destroyed, similar to the current tracking
> > done
> > for SQs. The memory for this array is freed when the controller is
> > freed.
> > A struct nvmet_ctrl reference is also added to the nvmet_cq
> > structure to
> > allow for CQs to be removed from the controller whilst keeping the
> > new API
> > similar to the existing API for SQs.
> 
> Looks good to me, modulo one nit below.
> 
> Reviewed-by: Damien Le Moal <dlemoal at kernel.org>
> 
> > diff --git a/drivers/nvme/target/pci-epf.c
> > b/drivers/nvme/target/pci-epf.c
> > index 7fab7f3d79b7..7dda4156d86c 100644
> > --- a/drivers/nvme/target/pci-epf.c
> > +++ b/drivers/nvme/target/pci-epf.c
> > @@ -1346,6 +1346,7 @@ static u16 nvmet_pci_epf_delete_cq(struct
> > nvmet_ctrl *tctrl, u16 cqid)
> >  	nvmet_pci_epf_drain_queue(cq);
> >  	nvmet_pci_epf_remove_irq_vector(ctrl, cq->vector);
> >  	nvmet_pci_epf_mem_unmap(ctrl->nvme_epf, &cq->pci_map);
> > +	tctrl->cqs[cqid] = NULL;
> 
> I do not think we need for this hunk in this patch as we are not yet
> using the
> cqs array, and patch 4 removes this line to change it with a call to
> nvmet_cq_put().
> 
Hey Damien,

This is added to undo the effects of `nvmet_cq_setup()`. If we don't,
when a CQ is deleted and created (same id) it would incorrectly fail
the `create && ctrl->cqs[cqid]` check in nvmet_check_cqid().

Wilfred