diff mbox series

[01/10] RDMA: Add and use rdma_for_each_port

Message ID 20190213041256.22437-2-jgg@ziepe.ca (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series Revise device handling in rxe | expand

Commit Message

Jason Gunthorpe Feb. 13, 2019, 4:12 a.m. UTC
From: Jason Gunthorpe <jgg@mellanox.com>

We have many loops iterating over all of the end port numbers on a struct
ib_device, simplify them with a for_each helper.

Reviewed-by: Parav Pandit <parav@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 .clang-format                             |  1 +
 drivers/infiniband/core/cache.c           |  6 +++---
 drivers/infiniband/core/cma.c             |  7 +++---
 drivers/infiniband/core/device.c          | 26 +++++++++++------------
 drivers/infiniband/core/mad.c             |  4 ++--
 drivers/infiniband/core/nldev.c           |  4 ++--
 drivers/infiniband/core/security.c        | 11 ++++++----
 drivers/infiniband/core/sysfs.c           | 12 +++--------
 drivers/infiniband/core/user_mad.c        |  9 ++++----
 drivers/infiniband/ulp/ipoib/ipoib_main.c |  4 ++--
 drivers/infiniband/ulp/srp/ib_srp.c       |  5 +++--
 include/rdma/ib_verbs.h                   | 10 +++++++++
 12 files changed, 53 insertions(+), 46 deletions(-)

Comments

Bart Van Assche Feb. 13, 2019, 4:12 p.m. UTC | #1
On Tue, 2019-02-12 at 21:12 -0700, Jason Gunthorpe wrote:
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 84184910f0380c..151f4eba84b872 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -4127,7 +4127,8 @@ static void srp_add_one(struct ib_device *device)
>  	struct srp_device *srp_dev;
>  	struct ib_device_attr *attr = &device->attrs;
>  	struct srp_host *host;
> -	int mr_page_shift, p;
> +	int mr_page_shift;
> +	unsigned int p;
>  	u64 max_pages_per_mr;
>  	unsigned int flags = 0;
>  
> @@ -4194,7 +4195,7 @@ static void srp_add_one(struct ib_device *device)
>  		WARN_ON_ONCE(srp_dev->global_rkey == 0);
>  	}
>  
> -	for (p = rdma_start_port(device); p <= rdma_end_port(device); ++p) {
> +	rdma_for_each_port (device, p) {
>  		host = srp_add_port(srp_dev, p);
>  		if (host)
>  			list_add_tail(&host->list, &srp_dev->dev_list);

Please remove the space after "rdma_for_each_port" to keep the usage of this
macro consistent with other similar macros in the Linux kernel, e.g.
list_for_each_entry().

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 135fab2c016c69..99a4868f4d9c58 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2828,6 +2828,16 @@ static inline u8 rdma_start_port(const struct ib_device *device)
>  	return rdma_cap_ib_switch(device) ? 0 : 1;
>  }
>  
> +/**
> + * rdma_for_each_port - Iterate over all valid port numbers of the IB device
> + * @device - The struct ib_device * to iterate over
> + * @iter - The unsigned int to store the port number
> + */
> +#define rdma_for_each_port(device, iter)                                       \
> +	for (iter = rdma_start_port(device + BUILD_BUG_ON_ZERO(!__same_type(   \
> +						     unsigned int, iter)));    \
> +	     iter <= rdma_end_port(device); (iter)++)

Why do you want to force the use of 'unsigned int' for port numbers?

Thanks,

Bart.
Jason Gunthorpe Feb. 14, 2019, 5:55 a.m. UTC | #2
On Wed, Feb 13, 2019 at 08:12:11AM -0800, Bart Van Assche wrote:
> On Tue, 2019-02-12 at 21:12 -0700, Jason Gunthorpe wrote:
> > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> > index 84184910f0380c..151f4eba84b872 100644
> > +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> > @@ -4127,7 +4127,8 @@ static void srp_add_one(struct ib_device *device)
> >  	struct srp_device *srp_dev;
> >  	struct ib_device_attr *attr = &device->attrs;
> >  	struct srp_host *host;
> > -	int mr_page_shift, p;
> > +	int mr_page_shift;
> > +	unsigned int p;
> >  	u64 max_pages_per_mr;
> >  	unsigned int flags = 0;
> >  
> > @@ -4194,7 +4195,7 @@ static void srp_add_one(struct ib_device *device)
> >  		WARN_ON_ONCE(srp_dev->global_rkey == 0);
> >  	}
> >  
> > -	for (p = rdma_start_port(device); p <= rdma_end_port(device); ++p) {
> > +	rdma_for_each_port (device, p) {
> >  		host = srp_add_port(srp_dev, p);
> >  		if (host)
> >  			list_add_tail(&host->list, &srp_dev->dev_list);
> 
> Please remove the space after "rdma_for_each_port" to keep the usage of this
> macro consistent with other similar macros in the Linux kernel, e.g.
> list_for_each_entry().

I think this was discussed already:

https://www.spinics.net/lists/linux-rdma/msg74904.html

> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 135fab2c016c69..99a4868f4d9c58 100644
> > +++ b/include/rdma/ib_verbs.h
> > @@ -2828,6 +2828,16 @@ static inline u8 rdma_start_port(const struct ib_device *device)
> >  	return rdma_cap_ib_switch(device) ? 0 : 1;
> >  }
> >  
> > +/**
> > + * rdma_for_each_port - Iterate over all valid port numbers of the IB device
> > + * @device - The struct ib_device * to iterate over
> > + * @iter - The unsigned int to store the port number
> > + */
> > +#define rdma_for_each_port(device, iter)                                       \
> > +	for (iter = rdma_start_port(device + BUILD_BUG_ON_ZERO(!__same_type(   \
> > +						     unsigned int, iter)));    \
> > +	     iter <= rdma_end_port(device); (iter)++)
> 
> Why do you want to force the use of 'unsigned int' for port numbers?

General consistency, and I know of a future case where u8 is not
enough.

Jason
diff mbox series

Patch

diff --git a/.clang-format b/.clang-format
index 335ce29ab8132c..201a4f531b90a5 100644
--- a/.clang-format
+++ b/.clang-format
@@ -361,6 +361,7 @@  ForEachMacros:
   - 'radix_tree_for_each_slot'
   - 'radix_tree_for_each_tagged'
   - 'rbtree_postorder_for_each_entry_safe'
+  - 'rdma_for_each_port'
   - 'resource_list_for_each_entry'
   - 'resource_list_for_each_entry_safe'
   - 'rhl_for_each_entry_rcu'
diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 2338d0b3a0cab8..3d137d8381a944 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -1428,7 +1428,7 @@  static void ib_cache_event(struct ib_event_handler *handler,
 
 int ib_cache_setup_one(struct ib_device *device)
 {
-	int p;
+	unsigned int p;
 	int err;
 
 	rwlock_init(&device->cache.lock);
@@ -1447,8 +1447,8 @@  int ib_cache_setup_one(struct ib_device *device)
 		return err;
 	}
 
-	for (p = 0; p <= rdma_end_port(device) - rdma_start_port(device); ++p)
-		ib_cache_update(device, p + rdma_start_port(device), true);
+	rdma_for_each_port (device, p)
+		ib_cache_update(device, p, true);
 
 	INIT_IB_EVENT_HANDLER(&device->cache.event_handler,
 			      device, ib_cache_event);
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index c43512752b8ac1..68c997be242930 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -659,7 +659,7 @@  static int cma_acquire_dev_by_src_ip(struct rdma_id_private *id_priv)
 	struct cma_device *cma_dev;
 	enum ib_gid_type gid_type;
 	int ret = -ENODEV;
-	u8 port;
+	unsigned int port;
 
 	if (dev_addr->dev_type != ARPHRD_INFINIBAND &&
 	    id_priv->id.ps == RDMA_PS_IPOIB)
@@ -673,8 +673,7 @@  static int cma_acquire_dev_by_src_ip(struct rdma_id_private *id_priv)
 
 	mutex_lock(&lock);
 	list_for_each_entry(cma_dev, &dev_list, list) {
-		for (port = rdma_start_port(cma_dev->device);
-		     port <= rdma_end_port(cma_dev->device); port++) {
+		rdma_for_each_port (cma_dev->device, port) {
 			gidp = rdma_protocol_roce(cma_dev->device, port) ?
 			       &iboe_gid : &gid;
 			gid_type = cma_dev->default_gid_type[port - 1];
@@ -4548,7 +4547,7 @@  static void cma_add_one(struct ib_device *device)
 	if (!cma_dev->default_roce_tos)
 		goto free_gid_type;
 
-	for (i = rdma_start_port(device); i <= rdma_end_port(device); i++) {
+	rdma_for_each_port (device, i) {
 		supported_gids = roce_gid_type_mask_support(device, i);
 		WARN_ON(!supported_gids);
 		if (supported_gids & (1 << CMA_PREFERRED_ROCE_GID_TYPE))
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 3325be4f91a5fe..d3bd09a6b53891 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -463,10 +463,8 @@  static int verify_immutable(const struct ib_device *dev, u8 port)
 
 static int read_port_immutable(struct ib_device *device)
 {
+	unsigned int port;
 	int ret;
-	u8 start_port = rdma_start_port(device);
-	u8 end_port = rdma_end_port(device);
-	u8 port;
 
 	/**
 	 * device->port_immutable is indexed directly by the port number to make
@@ -475,13 +473,13 @@  static int read_port_immutable(struct ib_device *device)
 	 * Therefore port_immutable is declared as a 1 based array with
 	 * potential empty slots at the beginning.
 	 */
-	device->port_immutable = kcalloc(end_port + 1,
-					 sizeof(*device->port_immutable),
-					 GFP_KERNEL);
+	device->port_immutable =
+		kcalloc(rdma_end_port(device) + 1,
+			sizeof(*device->port_immutable), GFP_KERNEL);
 	if (!device->port_immutable)
 		return -ENOMEM;
 
-	for (port = start_port; port <= end_port; ++port) {
+	rdma_for_each_port (device, port) {
 		ret = device->ops.get_port_immutable(
 			device, port, &device->port_immutable[port]);
 		if (ret)
@@ -533,9 +531,9 @@  static void ib_policy_change_task(struct work_struct *work)
 
 	down_read(&devices_rwsem);
 	xa_for_each_marked (&devices, index, dev, DEVICE_REGISTERED) {
-		int i;
+		unsigned int i;
 
-		for (i = rdma_start_port(dev); i <= rdma_end_port(dev); i++) {
+		rdma_for_each_port (dev, i) {
 			u64 sp;
 			int ret = ib_get_cached_subnet_prefix(dev,
 							      i,
@@ -1046,10 +1044,9 @@  void ib_enum_roce_netdev(struct ib_device *ib_dev,
 			 roce_netdev_callback cb,
 			 void *cookie)
 {
-	u8 port;
+	unsigned int port;
 
-	for (port = rdma_start_port(ib_dev); port <= rdma_end_port(ib_dev);
-	     port++)
+	rdma_for_each_port (ib_dev, port)
 		if (rdma_protocol_roce(ib_dev, port)) {
 			struct net_device *idev = NULL;
 
@@ -1203,9 +1200,10 @@  int ib_find_gid(struct ib_device *device, union ib_gid *gid,
 		u8 *port_num, u16 *index)
 {
 	union ib_gid tmp_gid;
-	int ret, port, i;
+	unsigned int port;
+	int ret, i;
 
-	for (port = rdma_start_port(device); port <= rdma_end_port(device); ++port) {
+	rdma_for_each_port (device, port) {
 		if (!rdma_protocol_ib(device, port))
 			continue;
 
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 7870823bac4726..e742a6a2c1380c 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -3326,9 +3326,9 @@  static void ib_mad_init_device(struct ib_device *device)
 
 static void ib_mad_remove_device(struct ib_device *device, void *client_data)
 {
-	int i;
+	unsigned int i;
 
-	for (i = rdma_start_port(device); i <= rdma_end_port(device); i++) {
+	rdma_for_each_port (device, i) {
 		if (!rdma_cap_ib_mad(device, i))
 			continue;
 
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 5601fa96824472..6708277aad7e4c 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -784,7 +784,7 @@  static int nldev_port_get_dumpit(struct sk_buff *skb,
 	u32 idx = 0;
 	u32 ifindex;
 	int err;
-	u32 p;
+	unsigned int p;
 
 	err = nlmsg_parse(cb->nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
 			  nldev_policy, NULL);
@@ -796,7 +796,7 @@  static int nldev_port_get_dumpit(struct sk_buff *skb,
 	if (!device)
 		return -EINVAL;
 
-	for (p = rdma_start_port(device); p <= rdma_end_port(device); ++p) {
+	rdma_for_each_port (device, p) {
 		/*
 		 * The dumpit function returns all information from specific
 		 * index. This specific index is taken from the netlink
diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
index dad6a94a43f30a..492702b836003c 100644
--- a/drivers/infiniband/core/security.c
+++ b/drivers/infiniband/core/security.c
@@ -422,12 +422,15 @@  void ib_close_shared_qp_security(struct ib_qp_security *sec)
 
 int ib_create_qp_security(struct ib_qp *qp, struct ib_device *dev)
 {
-	u8 i = rdma_start_port(dev);
+	unsigned int i;
 	bool is_ib = false;
 	int ret;
 
-	while (i <= rdma_end_port(dev) && !is_ib)
+	rdma_for_each_port (dev, i) {
 		is_ib = rdma_protocol_ib(dev, i++);
+		if (is_ib)
+			break;
+	}
 
 	/* If this isn't an IB device don't create the security context */
 	if (!is_ib)
@@ -561,9 +564,9 @@  void ib_security_cache_change(struct ib_device *device,
 void ib_security_release_port_pkey_list(struct ib_device *device)
 {
 	struct pkey_index_qp_list *pkey, *tmp_pkey;
-	int i;
+	unsigned int i;
 
-	for (i = rdma_start_port(device); i <= rdma_end_port(device); i++) {
+	rdma_for_each_port (device, i) {
 		list_for_each_entry_safe(pkey,
 					 tmp_pkey,
 					 &device->port_pkey_list[i].pkey_list,
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index c75692802da8b6..8fcb871fa5afef 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -1309,8 +1309,8 @@  static void free_port_list_attributes(struct ib_device *device)
 int ib_device_register_sysfs(struct ib_device *device)
 {
 	struct device *class_dev = &device->dev;
+	unsigned int port;
 	int ret;
-	int i;
 
 	device->groups[0] = &dev_attr_group;
 	class_dev->groups = device->groups;
@@ -1325,16 +1325,10 @@  int ib_device_register_sysfs(struct ib_device *device)
 		goto err_put;
 	}
 
-	if (rdma_cap_ib_switch(device)) {
-		ret = add_port(device, 0);
+	rdma_for_each_port (device, port) {
+		ret = add_port(device, port);
 		if (ret)
 			goto err_put;
-	} else {
-		for (i = 1; i <= device->phys_port_cnt; ++i) {
-			ret = add_port(device, i);
-			if (ret)
-				goto err_put;
-		}
 	}
 
 	if (device->ops.alloc_hw_stats)
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 3ebd211a87edf2..02b7947ab215b2 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -1323,14 +1323,15 @@  static void ib_umad_add_one(struct ib_device *device)
 static void ib_umad_remove_one(struct ib_device *device, void *client_data)
 {
 	struct ib_umad_device *umad_dev = client_data;
-	int i;
+	unsigned int i;
 
 	if (!umad_dev)
 		return;
 
-	for (i = 0; i <= rdma_end_port(device) - rdma_start_port(device); ++i) {
-		if (rdma_cap_ib_mad(device, i + rdma_start_port(device)))
-			ib_umad_kill_port(&umad_dev->ports[i]);
+	rdma_for_each_port (device, i) {
+		if (rdma_cap_ib_mad(device, i))
+			ib_umad_kill_port(
+				&umad_dev->ports[i - rdma_start_port(device)]);
 	}
 	/* balances kref_init() */
 	ib_umad_dev_put(umad_dev);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 3d3407d1feb453..c867dd5a231dcc 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -2495,7 +2495,7 @@  static void ipoib_add_one(struct ib_device *device)
 	struct list_head *dev_list;
 	struct net_device *dev;
 	struct ipoib_dev_priv *priv;
-	int p;
+	unsigned int p;
 	int count = 0;
 
 	dev_list = kmalloc(sizeof(*dev_list), GFP_KERNEL);
@@ -2504,7 +2504,7 @@  static void ipoib_add_one(struct ib_device *device)
 
 	INIT_LIST_HEAD(dev_list);
 
-	for (p = rdma_start_port(device); p <= rdma_end_port(device); ++p) {
+	rdma_for_each_port (device, p) {
 		if (!rdma_protocol_ib(device, p))
 			continue;
 		dev = ipoib_add_port("ib%d", device, p);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 84184910f0380c..151f4eba84b872 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -4127,7 +4127,8 @@  static void srp_add_one(struct ib_device *device)
 	struct srp_device *srp_dev;
 	struct ib_device_attr *attr = &device->attrs;
 	struct srp_host *host;
-	int mr_page_shift, p;
+	int mr_page_shift;
+	unsigned int p;
 	u64 max_pages_per_mr;
 	unsigned int flags = 0;
 
@@ -4194,7 +4195,7 @@  static void srp_add_one(struct ib_device *device)
 		WARN_ON_ONCE(srp_dev->global_rkey == 0);
 	}
 
-	for (p = rdma_start_port(device); p <= rdma_end_port(device); ++p) {
+	rdma_for_each_port (device, p) {
 		host = srp_add_port(srp_dev, p);
 		if (host)
 			list_add_tail(&host->list, &srp_dev->dev_list);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 135fab2c016c69..99a4868f4d9c58 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2828,6 +2828,16 @@  static inline u8 rdma_start_port(const struct ib_device *device)
 	return rdma_cap_ib_switch(device) ? 0 : 1;
 }
 
+/**
+ * rdma_for_each_port - Iterate over all valid port numbers of the IB device
+ * @device - The struct ib_device * to iterate over
+ * @iter - The unsigned int to store the port number
+ */
+#define rdma_for_each_port(device, iter)                                       \
+	for (iter = rdma_start_port(device + BUILD_BUG_ON_ZERO(!__same_type(   \
+						     unsigned int, iter)));    \
+	     iter <= rdma_end_port(device); (iter)++)
+
 /**
  * rdma_end_port - Return the last valid port number for the device
  * specified