Re: [PATCH 1/4] PCI: endpoint: pci-epf-vntb: Fix MSI doorbell IRQ unwind

[Niklas Cassel <cassel@kernel.org>]

On Mon, Feb 16, 2026 at 11:02:58PM +0900, Koichiro Den wrote:
> On Mon, Feb 16, 2026 at 12:56:18PM +0100, Niklas Cassel wrote:
> > On Mon, Feb 16, 2026 at 12:09:11AM +0900, Koichiro Den wrote:
> > > epf_ntb_db_bar_init_msi_doorbell() requests ntb->db_count doorbell IRQs
> > > and then performs additional MSI doorbell setup that may still fail.
> > > The error path unwinds the requested IRQs, but it uses a loop variable
> > > that is reused later in the function. When a later step fails, the
> > > unwind can run with an unexpected index value and leave some IRQs
> > > requested.
> > > 
> > > Track the number of successfully requested IRQs separately and use that
> > > counter for the unwind so all previously requested IRQs are freed on
> > > failure.
> > > 
> > > Fixes: dc693d606644 ("PCI: endpoint: pci-epf-vntb: Add MSI doorbell support")
> > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > ---
> > >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 11 ++++++-----
> > >  1 file changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > index 20a400e83439..20efa27325f1 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > @@ -523,6 +523,7 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > >  					    enum pci_barno barno)
> > >  {
> > >  	struct pci_epf *epf = ntb->epf;
> > > +	unsigned int req;
> > >  	dma_addr_t low, high;
> > >  	struct msi_msg *msg;
> > >  	size_t sz;
> > > @@ -533,14 +534,14 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > -	for (i = 0; i < ntb->db_count; i++) {
> > > -		ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> > > +	for (req = 0; req < ntb->db_count; req++) {
> > > +		ret = request_irq(epf->db_msg[req].virq, epf_ntb_doorbell_handler,
> > >  				  0, "pci_epf_vntb_db", ntb);
> > >  
> > >  		if (ret) {
> > >  			dev_err(&epf->dev,
> > >  				"Failed to request doorbell IRQ: %d\n",
> > > -				epf->db_msg[i].virq);
> > > +				epf->db_msg[req].virq);
> > >  			goto err_free_irq;
> > >  		}
> > >  	}
> > > @@ -598,8 +599,8 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > >  	return 0;
> > >  
> > >  err_free_irq:
> > > -	for (i--; i >= 0; i--)
> > > -		free_irq(epf->db_msg[i].virq, ntb);
> > > +	while (req)
> > > +		free_irq(epf->db_msg[--req].virq, ntb);
> > 
> > Please keep the for-loop.
> > Or if you want to change it, do so in a separate patch.
> > 
> > 
> > I understand that you need a separate variable for this,
> > since "i" is (re-)used in other places in the function,
> > but changing the for loop to a while is distracting from
> > the actual fix.
> 
> In that case, would you prefer the new "req" to be an int rather than unsigned
> int, so the for-loop can remain safely unchanged?

I suppose the same type as currently used for "i".


Kind regards,
Niklas