diff mbox series

[07/32] IB/mad: Convert ib_mad_clients to XArray

Message ID 20190221002107.22625-8-willy@infradead.org (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series Convert the Infiniband subsystem to XArray | expand

Commit Message

Matthew Wilcox Feb. 21, 2019, 12:20 a.m. UTC 
Pull the allocation function out into its own function to reduce the
length of ib_register_mad_agent() a little and keep all the allocation
logic together.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
 drivers/infiniband/core/mad.c | 39 +++++++++++++----------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

Comments

Ira Weiny Feb. 21, 2019, 12:58 a.m. UTC | #1 
On Wed, Feb 20, 2019 at 04:20:42PM -0800, Matthew Wilcox wrote:
> Pull the allocation function out into its own function to reduce the
> length of ib_register_mad_agent() a little and keep all the allocation
> logic together.
> 
> Signed-off-by: Matthew Wilcox <willy@infradead.org>
> ---
>  drivers/infiniband/core/mad.c | 39 +++++++++++++----------------------
>  1 file changed, 14 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index 7870823bac47..df43bf49e42f 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -38,10 +38,10 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/dma-mapping.h>
> -#include <linux/idr.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/security.h>
> +#include <linux/xarray.h>
>  #include <rdma/ib_cache.h>
>  
>  #include "mad_priv.h"
> @@ -59,12 +59,9 @@ MODULE_PARM_DESC(send_queue_size, "Size of send queue in number of work requests
>  module_param_named(recv_queue_size, mad_recvq_size, int, 0444);
>  MODULE_PARM_DESC(recv_queue_size, "Size of receive queue in number of work requests");
>  
> -/*
> - * The mlx4 driver uses the top byte to distinguish which virtual function
> - * generated the MAD, so we must avoid using it.
> - */
> -#define AGENT_ID_LIMIT		(1 << 24)
> -static DEFINE_IDR(ib_mad_clients);
> +/* Client ID 0 is used for snoop-only clients */
> +static DEFINE_XARRAY_ALLOC1(ib_mad_clients);
> +static u32 ib_mad_client_next;
>  static struct list_head ib_mad_port_list;
>  
>  /* Port list lock */
> @@ -389,18 +386,17 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
>  		goto error4;
>  	}
>  
> -	idr_preload(GFP_KERNEL);
> -	idr_lock(&ib_mad_clients);
> -	ret2 = idr_alloc_cyclic(&ib_mad_clients, mad_agent_priv, 0,
> -			AGENT_ID_LIMIT, GFP_ATOMIC);
> -	idr_unlock(&ib_mad_clients);
> -	idr_preload_end();
> -
> +	/*
> +	 * The mlx4 driver uses the top byte to distinguish which virtual
> +	 * function generated the MAD, so we must avoid using it.
> +	 */
> +	ret2 = xa_alloc_cyclic(&ib_mad_clients, &mad_agent_priv->agent.hi_tid,
> +			mad_agent_priv, XA_LIMIT(0, (1 << 24) - 1),
> +			&ib_mad_client_next, GFP_KERNEL);
>  	if (ret2 < 0) {
>  		ret = ERR_PTR(ret2);
>  		goto error5;
>  	}
> -	mad_agent_priv->agent.hi_tid = ret2;

Why are we removing this?

>  
>  	/*
>  	 * Make sure MAD registration (if supplied)
> @@ -448,9 +444,7 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
>  	return &mad_agent_priv->agent;
>  error6:
>  	spin_unlock_irq(&port_priv->reg_lock);
> -	idr_lock(&ib_mad_clients);
> -	idr_remove(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
> -	idr_unlock(&ib_mad_clients);
> +	xa_erase(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
>  error5:
>  	ib_mad_agent_security_cleanup(&mad_agent_priv->agent);
>  error4:
> @@ -614,9 +608,7 @@ static void unregister_mad_agent(struct ib_mad_agent_private *mad_agent_priv)
>  	spin_lock_irq(&port_priv->reg_lock);
>  	remove_mad_reg_req(mad_agent_priv);
>  	spin_unlock_irq(&port_priv->reg_lock);
> -	idr_lock(&ib_mad_clients);
> -	idr_remove(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
> -	idr_unlock(&ib_mad_clients);
> +	xa_erase(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
>  
>  	flush_workqueue(port_priv->wq);
>  	ib_cancel_rmpp_recvs(mad_agent_priv);
> @@ -1756,7 +1748,7 @@ find_mad_agent(struct ib_mad_port_private *port_priv,
>  		 */
>  		hi_tid = be64_to_cpu(mad_hdr->tid) >> 32;
>  		rcu_read_lock();
> -		mad_agent = idr_find(&ib_mad_clients, hi_tid);
> +		mad_agent = xa_load(&ib_mad_clients, hi_tid);
>  		if (mad_agent && !atomic_inc_not_zero(&mad_agent->refcount))
>  			mad_agent = NULL;
>  		rcu_read_unlock();
> @@ -3356,9 +3348,6 @@ int ib_mad_init(void)
>  
>  	INIT_LIST_HEAD(&ib_mad_port_list);
>  
> -	/* Client ID 0 is used for snoop-only clients */
> -	idr_alloc(&ib_mad_clients, NULL, 0, 0, GFP_KERNEL);
> -

Hal, will this somehow break the snoop stuff?  It is not straight forward why
we needed this before and I've been trying to remove the snoop stuff...  ;-)

Ira

>  	if (ib_register_client(&mad_client)) {
>  		pr_err("Couldn't register ib_mad client\n");
>  		return -EINVAL;
> -- 
> 2.20.1
>
Matthew Wilcox Feb. 21, 2019, 2:08 a.m. UTC | #2 
On Wed, Feb 20, 2019 at 04:58:31PM -0800, Ira Weiny wrote:
> > +	ret2 = xa_alloc_cyclic(&ib_mad_clients, &mad_agent_priv->agent.hi_tid,
> > +			mad_agent_priv, XA_LIMIT(0, (1 << 24) - 1),
> > +			&ib_mad_client_next, GFP_KERNEL);
> >  	if (ret2 < 0) {
> >  		ret = ERR_PTR(ret2);
> >  		goto error5;
> >  	}
> > -	mad_agent_priv->agent.hi_tid = ret2;
> 
> Why are we removing this?

xa_alloc() and xa_alloc_cyclic() write to the ID before storing the
pointer in the array.

> >  
> > -	/* Client ID 0 is used for snoop-only clients */
> > -	idr_alloc(&ib_mad_clients, NULL, 0, 0, GFP_KERNEL);
> > -
> 
> Hal, will this somehow break the snoop stuff?  It is not straight forward why
> we needed this before and I've been trying to remove the snoop stuff...  ;-)

> > +/* Client ID 0 is used for snoop-only clients */
> > +static DEFINE_XARRAY_ALLOC1(ib_mad_clients);

I didn't change that behaviour.
diff mbox series

Patch

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 7870823bac47..df43bf49e42f 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -38,10 +38,10 @@ 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/dma-mapping.h>
-#include <linux/idr.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/security.h>
+#include <linux/xarray.h>
 #include <rdma/ib_cache.h>
 
 #include "mad_priv.h"
@@ -59,12 +59,9 @@  MODULE_PARM_DESC(send_queue_size, "Size of send queue in number of work requests
 module_param_named(recv_queue_size, mad_recvq_size, int, 0444);
 MODULE_PARM_DESC(recv_queue_size, "Size of receive queue in number of work requests");
 
-/*
- * The mlx4 driver uses the top byte to distinguish which virtual function
- * generated the MAD, so we must avoid using it.
- */
-#define AGENT_ID_LIMIT		(1 << 24)
-static DEFINE_IDR(ib_mad_clients);
+/* Client ID 0 is used for snoop-only clients */
+static DEFINE_XARRAY_ALLOC1(ib_mad_clients);
+static u32 ib_mad_client_next;
 static struct list_head ib_mad_port_list;
 
 /* Port list lock */
@@ -389,18 +386,17 @@  struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 		goto error4;
 	}
 
-	idr_preload(GFP_KERNEL);
-	idr_lock(&ib_mad_clients);
-	ret2 = idr_alloc_cyclic(&ib_mad_clients, mad_agent_priv, 0,
-			AGENT_ID_LIMIT, GFP_ATOMIC);
-	idr_unlock(&ib_mad_clients);
-	idr_preload_end();
-
+	/*
+	 * The mlx4 driver uses the top byte to distinguish which virtual
+	 * function generated the MAD, so we must avoid using it.
+	 */
+	ret2 = xa_alloc_cyclic(&ib_mad_clients, &mad_agent_priv->agent.hi_tid,
+			mad_agent_priv, XA_LIMIT(0, (1 << 24) - 1),
+			&ib_mad_client_next, GFP_KERNEL);
 	if (ret2 < 0) {
 		ret = ERR_PTR(ret2);
 		goto error5;
 	}
-	mad_agent_priv->agent.hi_tid = ret2;
 
 	/*
 	 * Make sure MAD registration (if supplied)
@@ -448,9 +444,7 @@  struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 	return &mad_agent_priv->agent;
 error6:
 	spin_unlock_irq(&port_priv->reg_lock);
-	idr_lock(&ib_mad_clients);
-	idr_remove(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
-	idr_unlock(&ib_mad_clients);
+	xa_erase(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
 error5:
 	ib_mad_agent_security_cleanup(&mad_agent_priv->agent);
 error4:
@@ -614,9 +608,7 @@  static void unregister_mad_agent(struct ib_mad_agent_private *mad_agent_priv)
 	spin_lock_irq(&port_priv->reg_lock);
 	remove_mad_reg_req(mad_agent_priv);
 	spin_unlock_irq(&port_priv->reg_lock);
-	idr_lock(&ib_mad_clients);
-	idr_remove(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
-	idr_unlock(&ib_mad_clients);
+	xa_erase(&ib_mad_clients, mad_agent_priv->agent.hi_tid);
 
 	flush_workqueue(port_priv->wq);
 	ib_cancel_rmpp_recvs(mad_agent_priv);
@@ -1756,7 +1748,7 @@  find_mad_agent(struct ib_mad_port_private *port_priv,
 		 */
 		hi_tid = be64_to_cpu(mad_hdr->tid) >> 32;
 		rcu_read_lock();
-		mad_agent = idr_find(&ib_mad_clients, hi_tid);
+		mad_agent = xa_load(&ib_mad_clients, hi_tid);
 		if (mad_agent && !atomic_inc_not_zero(&mad_agent->refcount))
 			mad_agent = NULL;
 		rcu_read_unlock();
@@ -3356,9 +3348,6 @@  int ib_mad_init(void)
 
 	INIT_LIST_HEAD(&ib_mad_port_list);
 
-	/* Client ID 0 is used for snoop-only clients */
-	idr_alloc(&ib_mad_clients, NULL, 0, 0, GFP_KERNEL);
-
 	if (ib_register_client(&mad_client)) {
 		pr_err("Couldn't register ib_mad client\n");
 		return -EINVAL;