[PATCH 1/4] nvmet: pci-epf: Clear completion queue IRQ flag on delete

[Damien Le Moal]

On 5/8/25 10:07 PM, Niklas Cassel wrote:
> On Thu, May 08, 2025 at 03:57:42PM +0900, Damien Le Moal wrote:
>> The function nvmet_pci_epf_delete_cq() unconditionnally calls
> 
> s/unconditionnally/unconditionally/
> 
> 
>> nvmet_pci_epf_remove_irq_vector() even for completion queues that do not
>> have interrupts enabled. Furthermore, for completion queues that do
>> have IRQ enabled, deleting and re-creating the completion queue leaves
>> the flag NVMET_PCI_EPF_Q_IRQ_ENABLED set, even if the completion queue
>> is being re-created with IRQ disabled.
>>
>> Fix these issues by calling nvmet_pci_epf_remove_irq_vector() only if
>> NVMET_PCI_EPF_Q_IRQ_ENABLED is set and make sure to always clear that
>> flag.
>>
>> Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")
>> Cc: stable at vger.kernel.org
>> Signed-off-by: Damien Le Moal <dlemoal at kernel.org>
>> ---
>>  drivers/nvme/target/pci-epf.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
>> index 7fab7f3d79b7..d5442991f2fb 100644
>> --- a/drivers/nvme/target/pci-epf.c
>> +++ b/drivers/nvme/target/pci-epf.c
>> @@ -1344,7 +1344,8 @@ static u16 nvmet_pci_epf_delete_cq(struct nvmet_ctrl *tctrl, u16 cqid)
>>  
>>  	cancel_delayed_work_sync(&cq->work);
>>  	nvmet_pci_epf_drain_queue(cq);
>> -	nvmet_pci_epf_remove_irq_vector(ctrl, cq->vector);
>> +	if (test_and_clear_bit(NVMET_PCI_EPF_Q_IRQ_ENABLED, &cq->flags))
>> +		nvmet_pci_epf_remove_irq_vector(ctrl, cq->vector);
> 
> I agree that we should only call nvmet_pci_epf_remove_irq_vector() if
> NVMET_PCI_EPF_Q_IRQ_ENABLED is set.
> 
> However, it looks a bit weird to explicitly only clear only this flag bit.
> What about the other flag bits?

NVMET_PCI_EPF_Q_LIVE is cleared in the same function in the same manner. The
only other flag is NVMET_PCI_EPF_Q_IRQ_ENABLED, which this patch clears.

> I would have expected a memset() of flags, or flags = 0; in either
> .create_cq/.create_sq, or in .delete_cq/.delete_sq.
> 
> And it also seems that flag NVMET_PCI_EPF_Q_IS_SQ is unused, so that flag
> can probably be dropped.

Good catch. Will add a patch to drop it.


-- 
Damien Le Moal
Western Digital Research