PCI:MSI Return -ENOSPC when requested vectors is not enough
PATCH 1/1 PCI:MSI Return -ENOSPC when requested vectors is not enough
The __pci_enable_msi_range() should return -ENOSPC instead of -EINVAL when the device doesn't have enough vectors as required, just as the MSI-X vector allocator does in __pci_enable_msix_range(). Otherwise, some drivers depending on that return value will probably fallback to the legacy interrupt directly, for example, in commit 17a51f12cfbd2814 ("ahci: only try to use multi-MSI mode if there is more than 1 port"), the ahci driver will fallback to single MSI mode only when the return value is -ENOSPC in case of required vectors is not enough, else the driver will use legacy interrupt which has been observed on a x86 box with 6-port SATA controller.
With this patch, when a MSI-capable device doesn't have enough MSI vectors as requested, it will fallback to single MSI mode while not legacy interrupt.
--- drivers/pci/msi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/msi.c +1 -1
@@ -1084,7 +1084,7 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,if (nvec <0 )return nvec;if (nvec < minvec) -return -EINVAL ; +return -ENOSPC ;if (nvec > maxvec) nvec = maxvec;
Signature · 1 line
2.7.4
PATCH 1/1 PCI:MSI Return -ENOSPC when requested vectors is not enough
Hi Dennis,
I've fixed ahci to treat all errors the same in the meantime, please try latest Linux tree. That being said I don't like the different error returns from __pci_enable_msi_range (and __pci_enable_msix_range), but they have been there for a while.
PATCH 1/1 PCI:MSI Return -ENOSPC when requested vectors is not enough
Quoted 6 lines · Christoph Hellwig
Hi Dennis,
I've fixed ahci to treat all errors the same in the meantime, please try latest Linux tree. That being said I don't like the different error returns from
__pci_enable_msi_range(and__pci_enable_msix_range), but they have been there for a while.
Ah, I've noticed that you have the fix recently which is somehow to weaken the necessary of the change. But, that also being said that I don't like we insist at least the inconsistent either just because something has been there *for a while*. Both below comments from cpi_alloc_irq_vectors_affinity() and the logic itself leads us to think that the correct return value is -NOSPC:
/**
*...
*Return the number of vectors allocated,
* (which might be smaller than @max_vecs) if successful, or a negative
* error code on error. If less than @min_vecs interrupt vectors are
* available for @dev the function will fail with -ENOSPC.
* ...
*/
People maybe argue that almost has no device drivers depending on the different return value, then why we still need to do that?
Thanks,
Dennis
PATCH 1/1 PCI:MSI Return -ENOSPC when requested vectors is not enough
In reply to Dennis ChenQuoted 10 lines · Dennis Chen
The
__pci_enable_msi_range()should return-ENOSPCinstead of-EINVALwhen the device doesn't have enough vectors as required, just as the MSI-X vector allocator does in__pci_enable_msix_range(). Otherwise, some drivers depending on that return value will probably fallback to the legacy interrupt directly, for example, in commit17a51f12cfbd2814("ahci: only try to use multi-MSI mode if there is more than 1 port"), the ahci driver will fallback to single MSI mode only when the return value is-ENOSPCin case of required vectors is not enough, else the driver will use legacy interrupt which has been observed on a x86 box with 6-port SATA controller.
Unless Christoph objects, I'll apply this, but I don't understand the situation with 17a51f12cfbd. That commit doesn't check for EINVAL or ENOSPC so I don't know what the connection with this patch is.
I know Christoph said he changed something in ahci to treat all errors the same, but I don't know where that is, either.
If there's a revision of Linus' tree that is broken, please give the details so I can at least describe which versions are broken, when it got fixed by Christoph, and figure out whether we need stable backports or anything.
Bjorn
Quoted 40 lines
With this patch, when a MSI-capable device doesn't have enough MSI vectors as requested, it will fallback to single MSI mode while not legacy interrupt.
Signed-off-by Dennis Chen <[email protected]>Cc Tejun Heo <[email protected]>Cc Christoph Hellwig <[email protected]>Cc Tom Long Nguyen <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Marc Zyngier <[email protected]>Cc Lorenzo Pieralisi <[email protected]>Cc Steve Capper <[email protected]>Cc linux-ide at vger.kernel.orgCc linux-arm-kernel at lists.infradead.org--- drivers/pci/msi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/pci/msi.c +1
-1@@ -1084,7 +1084,7 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,if (nvec <0 )return nvec;if (nvec < minvec) -return -EINVAL ; +return -ENOSPC ;if (nvec > maxvec) nvec = maxvec;Signature · 7 lines
2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 PCI:MSI Return -ENOSPC when requested vectors is not enough
Quoted 3 lines · Bjorn Helgaas
Unless Christoph objects, I'll apply this, but I don't understand the situation with
17a51f12cfbd. That commit doesn't check for EINVAL or ENOSPC so I don't know what the connection with this patch is.
I don't think that commit is the culprit.
Quoted 2 lines
I know Christoph said he changed something in ahci to treat all errors the same, but I don't know where that is, either.
"ahci: always fall back to single-MSI mode"
Bjorn Helgaas
AppliedPATCH 1/1 PCI:MSI Return -ENOSPC when requested vectors is not enough
In reply to Dennis ChenQuoted 15 lines · Dennis Chen
The
__pci_enable_msi_range()should return-ENOSPCinstead of-EINVALwhen the device doesn't have enough vectors as required, just as the MSI-X vector allocator does in__pci_enable_msix_range(). Otherwise, some drivers depending on that return value will probably fallback to the legacy interrupt directly, for example, in commit17a51f12cfbd2814("ahci: only try to use multi-MSI mode if there is more than 1 port"), the ahci driver will fallback to single MSI mode only when the return value is-ENOSPCin case of required vectors is not enough, else the driver will use legacy interrupt which has been observed on a x86 box with 6-port SATA controller.With this patch, when a MSI-capable device doesn't have enough MSI vectors as requested, it will fallback to single MSI mode while not legacy interrupt.
Quoted 11 lines
Signed-off-by Dennis Chen <[email protected]>Cc Tejun Heo <[email protected]>Cc Christoph Hellwig <[email protected]>Cc Tom Long Nguyen <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Marc Zyngier <[email protected]>Cc Lorenzo Pieralisi <[email protected]>Cc Steve Capper <[email protected]>Cc linux-ide at vger.kernel.orgCc linux-arm-kernel at lists.infradead.org
Applied to pci/msi for v4.11, thanks, Dennis!
Quoted 25 lines
--- drivers/pci/msi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/pci/msi.c +1
-1@@ -1084,7 +1084,7 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,if (nvec <0 )return nvec;if (nvec < minvec) -return -EINVAL ; +return -ENOSPC ;if (nvec > maxvec) nvec = maxvec;Signature · 7 lines
2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel