Re: [PATCH rdma-next 2/2] RDMA/efa: Keep EQ resources alive while IRQ is registered

[Michael Margolin]

On Mon, Sep 07, 2026 at 09:44:47PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxx>
> 
> The completion IRQ handler accesses the EQ state and DMA buffer. Its IRQ was
> registered before that state was initialized, while teardown released the
> buffer before free_irq() synchronized the handler.
> 
> Initialize the EQ without arming it, register the IRQ, and then arm it.
> Reverse the resource order during teardown by freeing the IRQ before
> destroying the EQ.
> 
> Fixes: 2a152512a155 ("RDMA/efa: CQ notifications")
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> ---
>  drivers/infiniband/hw/efa/efa_com.c  |  3 +--
>  drivers/infiniband/hw/efa/efa_com.h  |  1 +
>  drivers/infiniband/hw/efa/efa_main.c | 18 ++++++++++--------
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/efa/efa_com.c b/drivers/infiniband/hw/efa/efa_com.c
> index cb2d448db2d7..04c6d63c449e 100644
> --- a/drivers/infiniband/hw/efa/efa_com.c
> +++ b/drivers/infiniband/hw/efa/efa_com.c
> @@ -1252,7 +1252,7 @@ static void efa_com_destroy_eq(struct efa_com_dev *edev,
>  				      err);
>  }
>  
> -static void efa_com_arm_eq(struct efa_com_dev *edev, struct efa_com_eq *eeq)
> +void efa_com_arm_eq(struct efa_com_dev *edev, struct efa_com_eq *eeq)
>  {
>  	u32 val = 0;
>  
> @@ -1341,7 +1341,6 @@ int efa_com_eq_init(struct efa_com_dev *edev, struct efa_com_eq *eeq,
>  	eeq->phase = 1;
>  	eeq->depth = params.depth;
>  	eeq->cb = cb;
> -	efa_com_arm_eq(edev, eeq);
>  
>  	return 0;
>  
> diff --git a/drivers/infiniband/hw/efa/efa_com.h b/drivers/infiniband/hw/efa/efa_com.h
> index 0341704d0921..98fb6a42a6cb 100644
> --- a/drivers/infiniband/hw/efa/efa_com.h
> +++ b/drivers/infiniband/hw/efa/efa_com.h
> @@ -169,6 +169,7 @@ int efa_com_admin_init(struct efa_com_dev *edev,
>  void efa_com_admin_destroy(struct efa_com_dev *edev);
>  int efa_com_eq_init(struct efa_com_dev *edev, struct efa_com_eq *eeq,
>  		    efa_eqe_handler cb, u16 depth, u8 msix_vec);
> +void efa_com_arm_eq(struct efa_com_dev *edev, struct efa_com_eq *eeq);
>  void efa_com_eq_destroy(struct efa_com_dev *edev, struct efa_com_eq *eeq);
>  int efa_com_dev_reset(struct efa_com_dev *edev,
>  		      enum efa_regs_reset_reason_types reset_reason);
> diff --git a/drivers/infiniband/hw/efa/efa_main.c b/drivers/infiniband/hw/efa/efa_main.c
> index b1df9bb8fac2..753ed8430b69 100644
> --- a/drivers/infiniband/hw/efa/efa_main.c
> +++ b/drivers/infiniband/hw/efa/efa_main.c
> @@ -302,28 +302,30 @@ static void efa_set_host_info(struct efa_dev *dev)
>  
>  static void efa_destroy_eq(struct efa_dev *dev, struct efa_eq *eq)
>  {
> -	efa_com_eq_destroy(&dev->edev, &eq->eeq);
>  	efa_free_irq(dev, &eq->irq);
> +	efa_com_eq_destroy(&dev->edev, &eq->eeq);
>  }
>  
>  static int efa_create_eq(struct efa_dev *dev, struct efa_eq *eq, u32 msix_vec)
>  {
>  	int err;
>  
> -	efa_setup_comp_irq(dev, eq, msix_vec);
> -	err = efa_request_irq(dev, &eq->irq);
> +	err = efa_com_eq_init(&dev->edev, &eq->eeq, efa_process_eqe,
> +			      dev->dev_attr.max_eq_depth, msix_vec);
>  	if (err)
>  		return err;
>  
> -	err = efa_com_eq_init(&dev->edev, &eq->eeq, efa_process_eqe,
> -			      dev->dev_attr.max_eq_depth, msix_vec);
> +	efa_setup_comp_irq(dev, eq, msix_vec);
> +	err = efa_request_irq(dev, &eq->irq);
>  	if (err)
> -		goto err_free_comp_irq;
> +		goto err_destroy_eq;
> +
> +	efa_com_arm_eq(&dev->edev, &eq->eeq);
>  
>  	return 0;
>  
> -err_free_comp_irq:
> -	efa_free_irq(dev, &eq->irq);
> +err_destroy_eq:
> +	efa_com_eq_destroy(&dev->edev, &eq->eeq);
>  	return err;
>  }
>  
> 
> -- 
> 2.55.0
>

Looks good to me, thanks!

Reviewed-by: Michael Margolin <mrgolin@xxxxxxxxxx>