Re: [PATCH] RDMA/cm: Make the local_id_table xarray non-irq

[Leon Romanovsky <leonro@mellanox.com>]

On Wed, Nov 04, 2020 at 05:40:59PM -0400, Jason Gunthorpe wrote:
> The xarray is never mutated from an IRQ handler, only from work queues
> under a spinlock_irq. Thus there is no reason for it be an IRQ type
> xarray.
>
> This was copied over from the original IDR code, but the recent rework put
> the xarray inside another spinlock_irq which will unbalance the unlocking.
>
> Fixes: c206f8bad15d ("RDMA/cm: Make it clearer how concurrency works in cm_req_handler()")
> Reported-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/infiniband/core/cm.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index 0201364974594f..167e436ae11ded 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -859,8 +859,8 @@ static struct cm_id_private *cm_alloc_id_priv(struct ib_device *device,
>  	atomic_set(&cm_id_priv->work_count, -1);
>  	refcount_set(&cm_id_priv->refcount, 1);
>
> -	ret = xa_alloc_cyclic_irq(&cm.local_id_table, &id, NULL, xa_limit_32b,
> -				  &cm.local_id_next, GFP_KERNEL);
> +	ret = xa_alloc_cyclic(&cm.local_id_table, &id, NULL, xa_limit_32b,
> +			      &cm.local_id_next, GFP_KERNEL);
>  	if (ret < 0)
>  		goto error;
>  	cm_id_priv->id.local_id = (__force __be32)id ^ cm.random_id_operand;
> @@ -878,8 +878,8 @@ static struct cm_id_private *cm_alloc_id_priv(struct ib_device *device,
>   */
>  static void cm_finalize_id(struct cm_id_private *cm_id_priv)
>  {
> -	xa_store_irq(&cm.local_id_table, cm_local_id(cm_id_priv->id.local_id),
> -		     cm_id_priv, GFP_KERNEL);
> +	xa_store(&cm.local_id_table, cm_local_id(cm_id_priv->id.local_id),
> +		 cm_id_priv, GFP_ATOMIC);
>  }

I see that in the ib_create_cm_id() function, we call to cm_finalize_id(),
won't it be a problem to do it without irq lock?

Thanks