Re: [PATCH 07/13] pci: Provide sensible irq vector alloc/free routines
From: Christoph Hellwig <hch@lst.de>
Date: 2016-07-10 03:47:43
Also in:
linux-nvme, linux-pci, lkml
On Wed, Jul 06, 2016 at 10:05:45AM +0200, Alexander Gordeev wrote:
quoted
+ pci_enable_msi, pci_enable_msi_range, pci_enable_msi_exact, pci_disable_msi, + pci_msi_vec_count, pci_enable_msix_range, pci_enable_msix_exact, + pci_disable_msix, pci_msix_vec_countDescription of these functions can be removed when all drivers migrated to the new API. Also implementation descriptions + examples would still be needed AFAICT.
I diagreed - if we deprecated functions the only thing that should be mentioned is a "don't use these".
This function's code almost matches the existing pci_enable_msix_range() so pci_enable_msix_range() should be reworked instead IMHO.
That's what earlier versions of the code did. However due to the fact that we want to avoid over-allocating the msix_vectors array (minor) and get the vectors count of the affinity mask right (major, as pointed out by you last time) I had to move the allocations inside the helpers that loop around the atctual enablement. I didn't want to change the function to a different version of the algorithm just before removing them relatively soon. But given that strong preference for changing these simple functions instead of duplicating them I've changed that patch to do that now.
We do not need to keep msix_entry array, since it only needed for pci_irq_vector() function. But the same info could be retrieved from msi_desc::irq.
Indeed. Avoiding this allocation makes these interfaces quite a bit simpler. It requires a few prep patches, but I think it's definitively worth, so the next version will avoid the need for the msix_entry array.
quoted
+ /* use legacy irq if allowed */ + if (min_vecs == 1) + return 1; + return -ENOSPC;The original error code (in vecs) would be overridden with -ENOSPC here.
Ok, fixed.
quoted
+ WARN_ON_ONCE(!dev->msi_enabled && nr > 0); + return dev->irq + nr;I think this function should check irq number existence and return the vector number or -EINVAL;
Ok, fixed.
quoted
+ unsigned int flags) +{ + if (min_vecs > 1) + return -ENOSPC;In case CONFIG_PCI_MSI is unset min_vecs > 1 is -EINVAL;
Ok, fixed.