On Wed, Feb 20, 2019 at 04:20:43PM -0800, Matthew Wilcox wrote: > Also introduce cm_local_id() to reduce the amount of boilerplate when > converting a local ID to an XArray index. > > Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxx> > drivers/infiniband/core/cm.c | 40 +++++++++++++++--------------------- > 1 file changed, 17 insertions(+), 23 deletions(-) > > diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c > index 37980c7564c0..b97592b44ce2 100644 > +++ b/drivers/infiniband/core/cm.c > @@ -124,7 +124,8 @@ static struct ib_cm { > struct rb_root remote_qp_table; > struct rb_root remote_id_table; > struct rb_root remote_sidr_table; > - struct idr local_id_table; > + struct xarray local_id_table; > + u32 local_id_next; > __be32 random_id_operand; > struct list_head timewait_list; > struct workqueue_struct *wq; > @@ -598,35 +599,31 @@ static int cm_init_av_by_path(struct sa_path_rec *path, > > static int cm_alloc_id(struct cm_id_private *cm_id_priv) > { > - unsigned long flags; > - int id; > - > - idr_preload(GFP_KERNEL); > - spin_lock_irqsave(&cm.lock, flags); > + int err; > + u32 id; > > - id = idr_alloc_cyclic(&cm.local_id_table, cm_id_priv, 0, 0, GFP_NOWAIT); > - > - spin_unlock_irqrestore(&cm.lock, flags); > - idr_preload_end(); > + err = xa_alloc_cyclic_irq(&cm.local_id_table, &id, cm_id_priv, > + xa_limit_32b, &cm.local_id_next, GFP_KERNEL); > > cm_id_priv->id.local_id = (__force __be32)id ^ cm.random_id_operand; > - return id < 0 ? id : 0; > + return err; > +} > + > +static u32 cm_local_id(__be32 local_id) > +{ > + return (__force u32) (local_id ^ cm.random_id_operand); > } > > static void cm_free_id(__be32 local_id) > { > - spin_lock_irq(&cm.lock); > - idr_remove(&cm.local_id_table, > - (__force int) (local_id ^ cm.random_id_operand)); > - spin_unlock_irq(&cm.lock); > + xa_erase_irq(&cm.local_id_table, cm_local_id(local_id)); > } > > static struct cm_id_private * cm_get_id(__be32 local_id, __be32 remote_id) > { > struct cm_id_private *cm_id_priv; > > - cm_id_priv = idr_find(&cm.local_id_table, > - (__force int) (local_id ^ cm.random_id_operand)); > + cm_id_priv = xa_load(&cm.local_id_table, cm_local_id(local_id)); > if (cm_id_priv) { > if (cm_id_priv->id.remote_id == remote_id) > atomic_inc(&cm_id_priv->refcount); > @@ -2824,9 +2821,8 @@ static struct cm_id_private * cm_acquire_rejected_id(struct cm_rej_msg *rej_msg) > spin_unlock_irq(&cm.lock); > return NULL; > } > - cm_id_priv = idr_find(&cm.local_id_table, (__force int) > - (timewait_info->work.local_id ^ > - cm.random_id_operand)); > + cm_id_priv = xa_load(&cm.local_id_table, > + cm_local_id(timewait_info->work.local_id)); > if (cm_id_priv) { > if (cm_id_priv->id.remote_id == remote_id) > atomic_inc(&cm_id_priv->refcount); > @@ -4513,7 +4509,7 @@ static int __init ib_cm_init(void) > cm.remote_id_table = RB_ROOT; > cm.remote_qp_table = RB_ROOT; > cm.remote_sidr_table = RB_ROOT; > - idr_init(&cm.local_id_table); > + xa_init_flags(&cm.local_id_table, XA_FLAGS_ALLOC | XA_FLAGS_LOCK_IRQ); > get_random_bytes(&cm.random_id_operand, sizeof cm.random_id_operand); > INIT_LIST_HEAD(&cm.timewait_list); > > @@ -4539,7 +4535,6 @@ static int __init ib_cm_init(void) > error2: > class_unregister(&cm_class); > error1: > - idr_destroy(&cm.local_id_table); > return ret; > } > > @@ -4561,7 +4556,6 @@ static void __exit ib_cm_cleanup(void) > } > > class_unregister(&cm_class); > - idr_destroy(&cm.local_id_table); Why not do xa_destroy()? BTW, I kind of feel like we should have a xa_empty_destroy(xa) which does a if (WARN_ON(!xa_empty())) xa_destroy(xa) Since cases like this are leaking memory bugs to unload the module with anything in the xarray. Jason