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
