RE: [PATCH for-next] RDMA/hns: Create QP/CQ with selected QPN/CQN for bank load balance

[liweihang <liweihang@xxxxxxxxxx>]

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg@xxxxxxxxxx]
> Sent: Friday, September 18, 2020 10:25 PM
> To: liweihang <liweihang@xxxxxxxxxx>
> Cc: dledford@xxxxxxxxxx; leon@xxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx;
> Linuxarm <linuxarm@xxxxxxxxxx>
> Subject: Re: [PATCH for-next] RDMA/hns: Create QP/CQ with selected
> QPN/CQN for bank load balance
> 
> On Wed, Sep 09, 2020 at 05:09:23PM +0800, Weihang Li wrote:
> > From: Yangyang Li <liyangyang20@xxxxxxxxxx>
> >
> > In order to improve performance by balancing the load between
> > different banks of cache, the QPC cache is desigend to choose one of 8
> > banks according to lower 3 bits of QPN, and the CQC cache uses the
> > lower 2 bits to choose one from 4 banks. The hns driver needs to count
> > the number of QP/CQ on each bank and then assigns the QP/CQ being
> > created to the bank with the minimum load first.
> >
> > Signed-off-by: Yangyang Li <liyangyang20@xxxxxxxxxx>
> > Signed-off-by: Weihang Li <liweihang@xxxxxxxxxx>
> > drivers/infiniband/hw/hns/hns_roce_alloc.c  | 46
> +++++++++++++++++++++++++++++
> >  drivers/infiniband/hw/hns/hns_roce_cq.c     | 38
> +++++++++++++++++++++++-
> >  drivers/infiniband/hw/hns/hns_roce_device.h |  8 +++++
> >  drivers/infiniband/hw/hns/hns_roce_qp.c     | 39
> ++++++++++++++++++++++--
> >  4 files changed, 128 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/hns/hns_roce_alloc.c
> > b/drivers/infiniband/hw/hns/hns_roce_alloc.c
> > index a522cb2..cbe955c 100644
> > +++ b/drivers/infiniband/hw/hns/hns_roce_alloc.c
> > @@ -36,6 +36,52 @@
> >  #include "hns_roce_device.h"
> >  #include <rdma/ib_umem.h>
> >
> > +static int get_bit(struct hns_roce_bitmap *bitmap, u8 bankid,
> > +		   u8 mod, unsigned long *obj)
> > +{
> > +	unsigned long offset_bak = bitmap->last;
> > +	bool one_circle_flag = false;
> > +
> > +	do {
> > +		*obj = find_next_zero_bit(bitmap->table, bitmap->max,
> > +					  bitmap->last);
> > +		if (*obj >= bitmap->max) {
> > +			*obj = find_first_zero_bit(bitmap->table, bitmap->max);
> > +			one_circle_flag = true;
> > +		}
> > +
> > +		bitmap->last = (*obj + 1);
> > +		if (bitmap->last == bitmap->max) {
> > +			bitmap->last = 0;
> > +			one_circle_flag = true;
> > +		}
> > +
> > +		/* Not found after a round of search */
> > +		if (bitmap->last >= offset_bak && one_circle_flag)
> > +			return -EINVAL;
> > +
> > +	} while (*obj % mod != bankid);
> > +
> > +	return 0;
> > +}
> 
> This looks like an ida, is there a reason it has to be open coded?
> 
> Jason


Hi Jason,

Thanks for your comments and we have a look at the ida interfaces.

There are 8 banks and each of them has a counter which represents how many QPs are using this
bank. We first find the bank with the smallest count, and then try to find a QPN belongs to this bank
according to the bitmap.
The ida will find an unused ID starting from 0, I think it can't meet our needs. If we use ida here,
the code may looks like:

While () {
	id = ida_alloc_range();
	if (isOK(id))
		break;

	ida_free(id);
}

We need to continuously apply for and release IDs that don't meet our requirements in the loop.

Thanks,
Weihang