On Wed, Feb 20, 2019 at 04:21:04PM -0800, Matthew Wilcox wrote:
> Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> drivers/infiniband/core/ucma.c | 26 +++++++++-----------------
> 1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 01d68ed46c1b..9a30068d4c1d 100644
> +++ b/drivers/infiniband/core/ucma.c
> @@ -124,7 +124,7 @@ struct ucma_event {
>
> static DEFINE_MUTEX(mut);
> static DEFINE_IDR(ctx_idr);
> -static DEFINE_IDR(multicast_idr);
> +static DEFINE_XARRAY_ALLOC(multicast_table);
>
> static const struct file_operations ucma_fops;
>
> @@ -238,10 +238,7 @@ static struct ucma_multicast* ucma_alloc_multicast(struct ucma_context *ctx)
> if (!mc)
> return NULL;
>
> - mutex_lock(&mut);
> - mc->id = idr_alloc(&multicast_idr, NULL, 0, 0, GFP_KERNEL);
> - mutex_unlock(&mut);
> - if (mc->id < 0)
> + if (xa_alloc(&multicast_table, &mc->id, NULL, xa_limit_32b, GFP_KERNEL))
> goto error;
>
> mc->ctx = ctx;
> @@ -540,7 +537,7 @@ static void ucma_cleanup_multicast(struct ucma_context *ctx)
> mutex_lock(&mut);
> list_for_each_entry_safe(mc, tmp, &ctx->mc_list, list) {
> list_del(&mc->list);
> - idr_remove(&multicast_idr, mc->id);
> + xa_erase(&multicast_table, mc->id);
> kfree(mc);
> }
> mutex_unlock(&mut);
> @@ -1425,9 +1422,7 @@ static ssize_t ucma_process_join(struct ucma_file *file,
> goto err3;
> }
>
> - mutex_lock(&mut);
> - idr_replace(&multicast_idr, mc, mc->id);
> - mutex_unlock(&mut);
> + xa_store(&multicast_table, mc->id, mc, 0);
>
> mutex_unlock(&file->mut);
> ucma_put_ctx(ctx);
> @@ -1437,9 +1432,7 @@ static ssize_t ucma_process_join(struct ucma_file *file,
> rdma_leave_multicast(ctx->cm_id, (struct sockaddr *) &mc->addr);
> ucma_cleanup_mc_events(mc);
> err2:
> - mutex_lock(&mut);
> - idr_remove(&multicast_idr, mc->id);
> - mutex_unlock(&mut);
> + xa_erase(&multicast_table, mc->id);
IMHO this is a lot clearer written as xa_release(), as it is only
undoing the store of XA_ZERO_ENTRY and that is why we don't need
locking.
> @@ -1501,8 +1494,8 @@ static ssize_t ucma_leave_multicast(struct ucma_file *file,
> if (copy_from_user(&cmd, inbuf, sizeof(cmd)))
> return -EFAULT;
>
> - mutex_lock(&mut);
> - mc = idr_find(&multicast_idr, cmd.id);
This can run concurrently with ucma_cleanup_multicast, so I don't
think the mut lock can be eliminated here.
Jason
