[PATCH 0/5] pci: nvmet: support completion queue sharing by multiple submission queues

[Damien Le Moal]

On 4/24/25 22:16, Niklas Cassel wrote:
> Hello Wilfred,
> 
> On Thu, Apr 24, 2025 at 03:13:48PM +1000, Wilfred Mallawa wrote:
>> From: Wilfred Mallawa <wilfred.mallawa at wdc.com>
>>
>> Hi all,
>>
>> For the NVMe PCI transport, the NVMe specification allows different
>> submission queues (SQs) to share completion queues (CQs), however,
>> this is not supported in the current NVMe target implementation.
>> Until now, the nvmet target implementation enforced a 1:1 relationship
>> between SQs and CQs, which is not specification compliant for the NVMe
>> PCI transport.
> 
> Perhaps it is a bit too harsh to say that we are non-spec compliant.
> We don't implement every feature in the NVMe spec in the Linux drivers
> on purpose, because there is a lot of feature creep in NVMe.
> 
> Perhaps rephrase this to something like:
> "While the nvmet target implementation only supports a 1:1 relationship
> between SQs and CQs, the NVMe over PCIe Transport Specification does not
> have this restriction."
> 
> 
>>
>> This patch series adds support for CQ sharing between multiple SQs in the
>> NVMe target driver, in line with the NVMe PCI transport specification.
>> This series implements reference counting for completion queues to ensure
>> proper lifecycle management when shared across multiple submission queues.
>> This ensures that we retain CQs until all referencing SQs are deleted
>> first, thereby avoiding premature CQ deletions.
> 
> The patches themselves look nice, however, I do think this cover letter is
> missing the answer to the question "why?".
> 
> If you need to modify the host-side Linux kernel even to test this feature,
> why are we doing this?
> 
> Is because we want to be nice to other host-side operating systems or NVMe
> stacks, e.g. SPDK, with might actually use this feature?

Yes. The Linux nvme pci host driver does not use CQ sharing, but other OSes may.

There is no urgency with this as no-one complained about this mandatory feature
not being supported. But it does not hurt to have this as it also adds nice
checks on the fabrics side to make sure that IO queues are being created and
deleted correctly.

-- 
Damien Le Moal
Western Digital Research