| Message ID | 20190221002107.22625-30-willy@infradead.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Convert the Infiniband subsystem to XArray | expand |
On Wed, Feb 20, 2019 at 04:21:04PM -0800, Matthew Wilcox wrote: > Signed-off-by: Matthew Wilcox <willy@infradead.org> > 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
On Wed, Feb 20, 2019 at 04:21:04PM -0800, Matthew Wilcox wrote: > Signed-off-by: Matthew Wilcox <willy@infradead.org> > --- > drivers/infiniband/core/ucma.c | 26 +++++++++----------------- > 1 file changed, 9 insertions(+), 17 deletions(-) Applied to for-next, but I added these two hunks: --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -104,7 +104,7 @@ struct ucma_context { struct ucma_multicast { struct ucma_context *ctx; - int id; + u32 id; int events_reported; u64 uid; @@ -234,10 +234,10 @@ static struct ucma_multicast* ucma_alloc_multicast(struct ucma_context *ctx) if (!mc) return NULL; + mc->ctx = ctx; if (xa_alloc(&multicast_table, &mc->id, NULL, xa_limit_32b, GFP_KERNEL)) goto error; - mc->ctx = ctx; list_add_tail(&mc->list, &ctx->mc_list); return mc; Thanks, Jason
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index 01d68ed46c1b..9a30068d4c1d 100644 --- a/drivers/infiniband/core/ucma.c +++ 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); list_del(&mc->list); kfree(mc); err1: @@ -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); + xa_lock(&multicast_table); + mc = xa_load(&multicast_table, cmd.id); if (!mc) mc = ERR_PTR(-ENOENT); else if (mc->ctx->file != file) @@ -1510,8 +1503,8 @@ static ssize_t ucma_leave_multicast(struct ucma_file *file, else if (!atomic_inc_not_zero(&mc->ctx->ref)) mc = ERR_PTR(-ENXIO); else - idr_remove(&multicast_idr, mc->id); - mutex_unlock(&mut); + __xa_erase(&multicast_table, mc->id); + xa_unlock(&multicast_table); if (IS_ERR(mc)) { ret = PTR_ERR(mc); @@ -1840,7 +1833,6 @@ static void __exit ucma_cleanup(void) device_remove_file(ucma_misc.this_device, &dev_attr_abi_version); misc_deregister(&ucma_misc); idr_destroy(&ctx_idr); - idr_destroy(&multicast_idr); } module_init(ucma_init);
Signed-off-by: Matthew Wilcox <willy@infradead.org> --- drivers/infiniband/core/ucma.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-)