| Message ID | 20201014014604.167968-1-jingxiangfeng@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] thunderbolt: Add the missed ida_simple_remove() in ring_request_msix() | expand |
On Wed, Oct 14, 2020 at 09:46:04AM +0800, Jing Xiangfeng wrote: > ring_request_msix() misses to call ida_simple_remove() in an error path. > Add a label 'err_ida_remove' and jump to it. ... > @@ -406,11 +406,22 @@ static int ring_request_msix(struct tb_ring *ring, bool no_suspend) > ring->vector = ret; ^^^ > ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector); > - if (ring->irq < 0) > - return ring->irq; > + if (ring->irq < 0) { > + ret = ring->irq; > + goto err_ida_remove; > + } What about ret = pci_irq_vector(ring->nhi->pdev, ring->vector); if (ret < 0) goto err_ida_remove; ring->irq = ret; ? (See also context above) > irqflags = no_suspend ? IRQF_NO_SUSPEND : 0; > - return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring); > + ret = request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring); > + if (ret) > + goto err_ida_remove; > + > + return 0; > + > +err_ida_remove: > + ida_simple_remove(&nhi->msix_ida, ring->vector); > + > + return ret; > } > > static void ring_release_msix(struct tb_ring *ring) > -- > 2.17.1 > -- With Best Regards, Andy Shevchenko
On 2020/10/14 16:48, Andy Shevchenko wrote: > On Wed, Oct 14, 2020 at 09:46:04AM +0800, Jing Xiangfeng wrote: >> ring_request_msix() misses to call ida_simple_remove() in an error path. >> Add a label 'err_ida_remove' and jump to it. > ... > >> @@ -406,11 +406,22 @@ static int ring_request_msix(struct tb_ring *ring, bool no_suspend) >> ring->vector = ret; > ^^^ > >> ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector); >> - if (ring->irq < 0) >> - return ring->irq; >> + if (ring->irq < 0) { >> + ret = ring->irq; >> + goto err_ida_remove; >> + } > What about > ret = pci_irq_vector(ring->nhi->pdev, ring->vector); > if (ret < 0) > goto err_ida_remove; > > ring->irq = ret; > > ? Yeah, I agree. Thanks for your suggestions. > > (See also context above) > >> irqflags = no_suspend ? IRQF_NO_SUSPEND : 0; >> - return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring); >> + ret = request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring); >> + if (ret) >> + goto err_ida_remove; >> + >> + return 0; >> + >> +err_ida_remove: >> + ida_simple_remove(&nhi->msix_ida, ring->vector); >> + >> + return ret; >> } >> >> static void ring_release_msix(struct tb_ring *ring) >> -- >> 2.17.1 >>
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c index 5f7489fa1327..e066888c4b41 100644 --- a/drivers/thunderbolt/nhi.c +++ b/drivers/thunderbolt/nhi.c @@ -406,11 +406,22 @@ static int ring_request_msix(struct tb_ring *ring, bool no_suspend) ring->vector = ret; ring->irq = pci_irq_vector(ring->nhi->pdev, ring->vector); - if (ring->irq < 0) - return ring->irq; + if (ring->irq < 0) { + ret = ring->irq; + goto err_ida_remove; + } irqflags = no_suspend ? IRQF_NO_SUSPEND : 0; - return request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring); + ret = request_irq(ring->irq, ring_msix, irqflags, "thunderbolt", ring); + if (ret) + goto err_ida_remove; + + return 0; + +err_ida_remove: + ida_simple_remove(&nhi->msix_ida, ring->vector); + + return ret; } static void ring_release_msix(struct tb_ring *ring)
ring_request_msix() misses to call ida_simple_remove() in an error path. Add a label 'err_ida_remove' and jump to it. Fixes: 046bee1f9ab8 ("thunderbolt: Add MSI-X support") Signed-off-by: Jing Xiangfeng <jingxiangfeng@huawei.com> --- drivers/thunderbolt/nhi.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)