* [PATCH] Octeontx2-af: Fix pci_alloc_irq_vectors() return value check
@ 2025-10-14 10:14 Harshit Mogalapalli
2025-10-14 10:20 ` Harshit Mogalapalli
0 siblings, 1 reply; 4+ messages in thread
From: Harshit Mogalapalli @ 2025-10-14 10:14 UTC (permalink / raw)
To: Sunil Goutham, Linu Cherian, Geetha sowjanya, Jerin Jacob,
hariprasad, Subbaraya Sundeep, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Nithya Mani, netdev,
linux-kernel
Cc: dan.carpenter, kernel-janitors, error27, harshit.m.mogalapalli
In cgx_probe() when pci_alloc_irq_vectors() fails the error value will
be negative and that check is sufficient.
err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX);
if (err < 0 || err != nvec) {
...
}
Remove the check which compares err with nvec.
Fixes: 1463f382f58d ("octeontx2-af: Add support for CGX link management")
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
Only compile tested.
---
drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
index d374a4454836..f4d5a3c05fa4 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c
@@ -1993,7 +1993,7 @@ static int cgx_probe(struct pci_dev *pdev, const struct pci_device_id *id)
nvec = pci_msix_vec_count(cgx->pdev);
err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX);
- if (err < 0 || err != nvec) {
+ if (err < 0) {
dev_err(dev, "Request for %d msix vectors failed, err %d\n",
nvec, err);
goto err_release_regions;
--
2.39.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Octeontx2-af: Fix pci_alloc_irq_vectors() return value check 2025-10-14 10:14 [PATCH] Octeontx2-af: Fix pci_alloc_irq_vectors() return value check Harshit Mogalapalli @ 2025-10-14 10:20 ` Harshit Mogalapalli 2025-10-15 8:36 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Harshit Mogalapalli @ 2025-10-14 10:20 UTC (permalink / raw) To: Sunil Goutham, Linu Cherian, Geetha sowjanya, Jerin Jacob, hariprasad, Subbaraya Sundeep, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Nithya Mani, netdev, linux-kernel Cc: dan.carpenter, kernel-janitors, error27 Hi, On 14/10/25 15:44, Harshit Mogalapalli wrote: > In cgx_probe() when pci_alloc_irq_vectors() fails the error value will > be negative and that check is sufficient. > > err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX); > if (err < 0 || err != nvec) { > ... > } > > Remove the check which compares err with nvec. > > Fixes: 1463f382f58d ("octeontx2-af: Add support for CGX link management") > Suggested-by: Paolo Abeni <pabeni@redhat.com> > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> > --- > Only compile tested. > --- > drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c > index d374a4454836..f4d5a3c05fa4 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c > +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c > @@ -1993,7 +1993,7 @@ static int cgx_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > nvec = pci_msix_vec_count(cgx->pdev); > err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX); > - if (err < 0 || err != nvec) { > + if (err < 0) { > dev_err(dev, "Request for %d msix vectors failed, err %d\n", > nvec, err); Now that I think about it more, maybe we want to error out when err != nvec as well ? In that case maybe the right thing to do is leave the check as is and set err = -EXYZ before goto ? Thanks, Harshit> goto err_release_regions; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Octeontx2-af: Fix pci_alloc_irq_vectors() return value check 2025-10-14 10:20 ` Harshit Mogalapalli @ 2025-10-15 8:36 ` Simon Horman 2025-10-15 8:56 ` Harshit Mogalapalli 0 siblings, 1 reply; 4+ messages in thread From: Simon Horman @ 2025-10-15 8:36 UTC (permalink / raw) To: Harshit Mogalapalli Cc: Sunil Goutham, Linu Cherian, Geetha sowjanya, Jerin Jacob, hariprasad, Subbaraya Sundeep, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Nithya Mani, netdev, linux-kernel, dan.carpenter, kernel-janitors, error27 On Tue, Oct 14, 2025 at 03:50:47PM +0530, Harshit Mogalapalli wrote: > Hi, > > On 14/10/25 15:44, Harshit Mogalapalli wrote: > > In cgx_probe() when pci_alloc_irq_vectors() fails the error value will > > be negative and that check is sufficient. > > > > err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX); > > if (err < 0 || err != nvec) { > > ... > > } > > > > Remove the check which compares err with nvec. > > > > Fixes: 1463f382f58d ("octeontx2-af: Add support for CGX link management") > > Suggested-by: Paolo Abeni <pabeni@redhat.com> > > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> > > --- > > Only compile tested. > > --- > > drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c > > index d374a4454836..f4d5a3c05fa4 100644 > > --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c > > +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c > > @@ -1993,7 +1993,7 @@ static int cgx_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > nvec = pci_msix_vec_count(cgx->pdev); > > err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX); > > - if (err < 0 || err != nvec) { > > + if (err < 0) { > > dev_err(dev, "Request for %d msix vectors failed, err %d\n", > > nvec, err); > > > Now that I think about it more, maybe we want to error out when err != nvec > as well ? In that case maybe the right thing to do is leave the check as is > and set err = -EXYZ before goto ? > > Thanks, > Harshit> goto err_release_regions; Hi Harshit, My reading of the documentation of pci_alloc_irq_vectors() is that Because nvec is passed as both the min and max desired vectors, either nvecs will be allocated, or -ENOSPC will be returned. Maybe it is worth adding a comment about that to patch description or the code. But I think the code in this patch is correct. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Octeontx2-af: Fix pci_alloc_irq_vectors() return value check 2025-10-15 8:36 ` Simon Horman @ 2025-10-15 8:56 ` Harshit Mogalapalli 0 siblings, 0 replies; 4+ messages in thread From: Harshit Mogalapalli @ 2025-10-15 8:56 UTC (permalink / raw) To: Simon Horman Cc: Sunil Goutham, Linu Cherian, Geetha sowjanya, Jerin Jacob, hariprasad, Subbaraya Sundeep, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Nithya Mani, netdev, linux-kernel, dan.carpenter, kernel-janitors, error27 Hi Simon, On 15/10/25 14:06, Simon Horman wrote: > On Tue, Oct 14, 2025 at 03:50:47PM +0530, Harshit Mogalapalli wrote: >> Hi, >> >> On 14/10/25 15:44, Harshit Mogalapalli wrote: >>> In cgx_probe() when pci_alloc_irq_vectors() fails the error value will >>> be negative and that check is sufficient. >>> >>> err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX); >>> if (err < 0 || err != nvec) { >>> ... >>> } >>> >>> Remove the check which compares err with nvec. >>> >>> Fixes: 1463f382f58d ("octeontx2-af: Add support for CGX link management") >>> Suggested-by: Paolo Abeni <pabeni@redhat.com> >>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >>> --- >>> Only compile tested. >>> --- >>> drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c >>> index d374a4454836..f4d5a3c05fa4 100644 >>> --- a/drivers/net/ethernet/marvell/octeontx2/af/cgx.c >>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cgx.c >>> @@ -1993,7 +1993,7 @@ static int cgx_probe(struct pci_dev *pdev, const struct pci_device_id *id) >>> nvec = pci_msix_vec_count(cgx->pdev); >>> err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSIX); >>> - if (err < 0 || err != nvec) { >>> + if (err < 0) { >>> dev_err(dev, "Request for %d msix vectors failed, err %d\n", >>> nvec, err); >> >> >> Now that I think about it more, maybe we want to error out when err != nvec >> as well ? In that case maybe the right thing to do is leave the check as is >> and set err = -EXYZ before goto ? >> >> Thanks, >> Harshit> goto err_release_regions; > > Hi Harshit, > > My reading of the documentation of pci_alloc_irq_vectors() is that > Because nvec is passed as both the min and max desired vectors, > either nvecs will be allocated, or -ENOSPC will be returned. > Ah thats a nice summary of the correctness of this patch.> Maybe it is worth adding a comment about that to patch description > or the code. But I think the code in this patch is correct. Definitely will add this and send v2. Thanks, Harshit ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-15 8:57 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-14 10:14 [PATCH] Octeontx2-af: Fix pci_alloc_irq_vectors() return value check Harshit Mogalapalli 2025-10-14 10:20 ` Harshit Mogalapalli 2025-10-15 8:36 ` Simon Horman 2025-10-15 8:56 ` Harshit Mogalapalli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).