Re: [PATCH rdma-next 1/2] RDMA/efa: Keep admin queues alive while IRQ is registered

[Michael Margolin]

On Mon, Sep 07, 2026 at 09:44:46PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxx>
> 
> The management IRQ handler accesses both the admin completion queue and the
> async event queue. The driver registered the IRQ before constructing these
> queues and destroyed them before freeing the IRQ, so the handler's lifetime
> was not contained by the resources it accesses.
> 
> Initialize the queues with interrupts masked, request the IRQ, and then
> switch to interrupt mode. On removal, reset the device and free the IRQ
> before destroying the queues. Also reset the device before destroying the
> queues if IRQ registration fails, because the device already has their DMA
> addresses.
> 
> Fixes: b7f5e880f377 ("RDMA/efa: Add the efa module")
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> ---
>  drivers/infiniband/hw/efa/efa_com.c  |  4 +---
>  drivers/infiniband/hw/efa/efa_main.c | 15 +++++++++------
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/efa/efa_com.c b/drivers/infiniband/hw/efa/efa_com.c
> index 583b1cf0d721..cb2d448db2d7 100644
> --- a/drivers/infiniband/hw/efa/efa_com.c
> +++ b/drivers/infiniband/hw/efa/efa_com.c
> @@ -850,7 +850,7 @@ int efa_com_admin_init(struct efa_com_dev *edev,
>  
>  	aq->dmadev = edev->dmadev;
>  	aq->efa_dev = edev->efa_dev;
> -	set_bit(EFA_AQ_STATE_POLLING_BIT, &aq->state);
> +	efa_com_set_admin_polling_mode(edev, true);
>  
>  	sema_init(&aq->avail_cmds, aq->depth);
>  
> @@ -868,8 +868,6 @@ int efa_com_admin_init(struct efa_com_dev *edev,
>  	if (err)
>  		goto err_destroy_sq;
>  
> -	efa_com_set_admin_polling_mode(edev, false);
> -
>  	err = efa_com_admin_init_aenq(edev, aenq_handlers);
>  	if (err)
>  		goto err_destroy_cq;
> diff --git a/drivers/infiniband/hw/efa/efa_main.c b/drivers/infiniband/hw/efa/efa_main.c
> index 4cd80727dcd2..b1df9bb8fac2 100644
> --- a/drivers/infiniband/hw/efa/efa_main.c
> +++ b/drivers/infiniband/hw/efa/efa_main.c
> @@ -619,18 +619,21 @@ static struct efa_dev *efa_probe_device(struct pci_dev *pdev)
>  	edev->aq.msix_vector_idx = dev->admin_msix_vector_idx;
>  	edev->aenq.msix_vector_idx = dev->admin_msix_vector_idx;
>  
> -	err = efa_set_mgmnt_irq(dev);
> +	err = efa_com_admin_init(edev, &aenq_handlers);
>  	if (err)
>  		goto err_disable_msix;
>  
> -	err = efa_com_admin_init(edev, &aenq_handlers);
> +	err = efa_set_mgmnt_irq(dev);
>  	if (err)
> -		goto err_free_mgmnt_irq;
> +		goto err_destroy_admin;
> +
> +	efa_com_set_admin_polling_mode(edev, false);
>  
>  	return dev;
>  
> -err_free_mgmnt_irq:
> -	efa_free_irq(dev, &dev->admin_irq);
> +err_destroy_admin:
> +	efa_com_dev_reset(edev, EFA_REGS_RESET_INIT_ERR);
> +	efa_com_admin_destroy(edev);
>  err_disable_msix:
>  	efa_disable_msix(dev);
>  err_reg_read_destroy:
> @@ -654,8 +657,8 @@ static void efa_remove_device(struct pci_dev *pdev,
>  
>  	edev = &dev->edev;
>  	efa_com_dev_reset(edev, reset_reason);
> -	efa_com_admin_destroy(edev);
>  	efa_free_irq(dev, &dev->admin_irq);
> +	efa_com_admin_destroy(edev);
>  	efa_disable_msix(dev);
>  	efa_com_mmio_reg_read_destroy(edev);
>  	devm_iounmap(&pdev->dev, edev->reg_bar);
> 
> -- 
> 2.55.0
>

Looks good to me, thanks!

Reviewed-by: Michael Margolin <mrgolin@xxxxxxxxxx>