Message ID | 20211109124103.54326-6-liangwenpeng@huawei.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | libhns: Cleanup about removing redundant code and cleaning up static alarms | expand |
On Tue, Nov 09, 2021 at 08:41:01PM +0800, Wenpeng Liang wrote: > From: Xinhao Liu <liuxinhao5@hisilicon.com> > > Some variables and fields should be in type of unsigned instead of signed. > > Signed-off-by: Xinhao Liu <liuxinhao5@hisilicon.com> > Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com> > providers/hns/hns_roce_u.h | 6 +++--- > providers/hns/hns_roce_u_hw_v1.c | 6 +++--- > providers/hns/hns_roce_u_hw_v2.c | 11 +++++------ > 3 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/providers/hns/hns_roce_u.h b/providers/hns/hns_roce_u.h > index 0d7abd81..d5963941 100644 > +++ b/providers/hns/hns_roce_u.h > @@ -99,7 +99,7 @@ > #define roce_set_bit(origin, shift, val) \ > roce_set_field((origin), (1ul << (shift)), (shift), (val)) > > -#define hr_ilog32(n) ilog32((n) - 1) > +#define hr_ilog32(n) ilog32((unsigned int)(n) - 1) This should be a static inline function not a macro, then it can have the correct type. Also please send this series as a PR on the github Thanks, Jason
On 2021/11/23 22:13, Jason Gunthorpe wrote: > On Tue, Nov 09, 2021 at 08:41:01PM +0800, Wenpeng Liang wrote: >> From: Xinhao Liu <liuxinhao5@hisilicon.com> >> >> Some variables and fields should be in type of unsigned instead of signed. >> >> Signed-off-by: Xinhao Liu <liuxinhao5@hisilicon.com> >> Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com> >> providers/hns/hns_roce_u.h | 6 +++--- >> providers/hns/hns_roce_u_hw_v1.c | 6 +++--- >> providers/hns/hns_roce_u_hw_v2.c | 11 +++++------ >> 3 files changed, 11 insertions(+), 12 deletions(-) >> >> diff --git a/providers/hns/hns_roce_u.h b/providers/hns/hns_roce_u.h >> index 0d7abd81..d5963941 100644 >> +++ b/providers/hns/hns_roce_u.h >> @@ -99,7 +99,7 @@ >> #define roce_set_bit(origin, shift, val) \ >> roce_set_field((origin), (1ul << (shift)), (shift), (val)) >> >> -#define hr_ilog32(n) ilog32((n) - 1) >> +#define hr_ilog32(n) ilog32((unsigned int)(n) - 1) > > This should be a static inline function not a macro, then it can have > the correct type. > > Also please send this series as a PR on the github > > Thanks, > Jason > . > I submitted a PR on the github: https://github.com/linux-rdma/rdma-core/pull/1090 Thanks Wenpeng
On Wed, Nov 24, 2021 at 07:39:34PM +0800, Wenpeng Liang wrote: > > > On 2021/11/23 22:13, Jason Gunthorpe wrote: > > On Tue, Nov 09, 2021 at 08:41:01PM +0800, Wenpeng Liang wrote: > >> From: Xinhao Liu <liuxinhao5@hisilicon.com> > >> > >> Some variables and fields should be in type of unsigned instead of signed. > >> > >> Signed-off-by: Xinhao Liu <liuxinhao5@hisilicon.com> > >> Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com> > >> providers/hns/hns_roce_u.h | 6 +++--- > >> providers/hns/hns_roce_u_hw_v1.c | 6 +++--- > >> providers/hns/hns_roce_u_hw_v2.c | 11 +++++------ > >> 3 files changed, 11 insertions(+), 12 deletions(-) > >> > >> diff --git a/providers/hns/hns_roce_u.h b/providers/hns/hns_roce_u.h > >> index 0d7abd81..d5963941 100644 > >> +++ b/providers/hns/hns_roce_u.h > >> @@ -99,7 +99,7 @@ > >> #define roce_set_bit(origin, shift, val) \ > >> roce_set_field((origin), (1ul << (shift)), (shift), (val)) > >> > >> -#define hr_ilog32(n) ilog32((n) - 1) > >> +#define hr_ilog32(n) ilog32((unsigned int)(n) - 1) > > > > This should be a static inline function not a macro, then it can have > > the correct type. > > > > Also please send this series as a PR on the github > > > > Thanks, > > Jason > > . > > > > I submitted a PR on the github: > > https://github.com/linux-rdma/rdma-core/pull/1090 I mean of your whole series Jason
On 2021/11/24 23:40, Jason Gunthorpe wrote: > On Wed, Nov 24, 2021 at 07:39:34PM +0800, Wenpeng Liang wrote: >> >> >> On 2021/11/23 22:13, Jason Gunthorpe wrote: >>> On Tue, Nov 09, 2021 at 08:41:01PM +0800, Wenpeng Liang wrote: >>>> From: Xinhao Liu <liuxinhao5@hisilicon.com> >>>> >>>> Some variables and fields should be in type of unsigned instead of signed. >>>> >>>> Signed-off-by: Xinhao Liu <liuxinhao5@hisilicon.com> >>>> Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com> >>>> providers/hns/hns_roce_u.h | 6 +++--- >>>> providers/hns/hns_roce_u_hw_v1.c | 6 +++--- >>>> providers/hns/hns_roce_u_hw_v2.c | 11 +++++------ >>>> 3 files changed, 11 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/providers/hns/hns_roce_u.h b/providers/hns/hns_roce_u.h >>>> index 0d7abd81..d5963941 100644 >>>> +++ b/providers/hns/hns_roce_u.h >>>> @@ -99,7 +99,7 @@ >>>> #define roce_set_bit(origin, shift, val) \ >>>> roce_set_field((origin), (1ul << (shift)), (shift), (val)) >>>> >>>> -#define hr_ilog32(n) ilog32((n) - 1) >>>> +#define hr_ilog32(n) ilog32((unsigned int)(n) - 1) >>> >>> This should be a static inline function not a macro, then it can have >>> the correct type. >>> >>> Also please send this series as a PR on the github >>> >>> Thanks, >>> Jason >>> . >>> >> >> I submitted a PR on the github: >> >> https://github.com/linux-rdma/rdma-core/pull/1090 > > I mean of your whole series > > Jason > . > This series has been merged into the master branch, so I submit an independent PR to fix it. Thanks Wenpeng
diff --git a/providers/hns/hns_roce_u.h b/providers/hns/hns_roce_u.h index 0d7abd81..d5963941 100644 --- a/providers/hns/hns_roce_u.h +++ b/providers/hns/hns_roce_u.h @@ -99,7 +99,7 @@ #define roce_set_bit(origin, shift, val) \ roce_set_field((origin), (1ul << (shift)), (shift), (val)) -#define hr_ilog32(n) ilog32((n) - 1) +#define hr_ilog32(n) ilog32((unsigned int)(n) - 1) enum { HNS_ROCE_QP_TABLE_BITS = 8, @@ -203,7 +203,7 @@ struct hns_roce_cq { struct hns_roce_idx_que { struct hns_roce_buf buf; - int entry_shift; + unsigned int entry_shift; unsigned long *bitmap; int bitmap_cnt; unsigned int head; @@ -249,7 +249,7 @@ struct hns_roce_sge_info { struct hns_roce_sge_ex { int offset; unsigned int sge_cnt; - int sge_shift; + unsigned int sge_shift; }; struct hns_roce_rinl_sge { diff --git a/providers/hns/hns_roce_u_hw_v1.c b/providers/hns/hns_roce_u_hw_v1.c index ef346424..cc97de30 100644 --- a/providers/hns/hns_roce_u_hw_v1.c +++ b/providers/hns/hns_roce_u_hw_v1.c @@ -220,7 +220,7 @@ static int hns_roce_wq_overflow(struct hns_roce_wq *wq, int nreq, static struct hns_roce_qp *hns_roce_find_qp(struct hns_roce_context *ctx, uint32_t qpn) { - int tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift; + uint32_t tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift; if (ctx->qp_table[tind].refcnt) { return ctx->qp_table[tind].table[qpn & ctx->qp_table_mask]; @@ -232,7 +232,7 @@ static struct hns_roce_qp *hns_roce_find_qp(struct hns_roce_context *ctx, static void hns_roce_clear_qp(struct hns_roce_context *ctx, uint32_t qpn) { - int tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift; + uint32_t tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift; if (!--ctx->qp_table[tind].refcnt) free(ctx->qp_table[tind].table); @@ -739,7 +739,7 @@ static int hns_roce_u_v1_post_recv(struct ibv_qp *ibvqp, struct ibv_recv_wr *wr, struct ibv_recv_wr **bad_wr) { int ret = 0; - int nreq; + unsigned int nreq; struct ibv_sge *sg; struct hns_roce_rc_rq_wqe *rq_wqe; struct hns_roce_qp *qp = to_hr_qp(ibvqp); diff --git a/providers/hns/hns_roce_u_hw_v2.c b/providers/hns/hns_roce_u_hw_v2.c index 31a0681d..4943a5b1 100644 --- a/providers/hns/hns_roce_u_hw_v2.c +++ b/providers/hns/hns_roce_u_hw_v2.c @@ -247,7 +247,7 @@ static void *get_srq_wqe(struct hns_roce_srq *srq, unsigned int n) return srq->wqe_buf.buf + (n << srq->wqe_shift); } -static void *get_idx_buf(struct hns_roce_idx_que *idx_que, int n) +static void *get_idx_buf(struct hns_roce_idx_que *idx_que, unsigned int n) { return idx_que->buf.buf + (n << idx_que->entry_shift); } @@ -331,7 +331,7 @@ static void hns_roce_v2_update_cq_cons_index(struct hns_roce_context *ctx, static struct hns_roce_qp *hns_roce_v2_find_qp(struct hns_roce_context *ctx, uint32_t qpn) { - int tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift; + uint32_t tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift; if (ctx->qp_table[tind].refcnt) return ctx->qp_table[tind].table[qpn & ctx->qp_table_mask]; @@ -961,9 +961,8 @@ static int fill_ud_data_seg(struct hns_roce_ud_sq_wqe *ud_sq_wqe, return ret; } -static int set_ud_wqe(void *wqe, struct hns_roce_qp *qp, - struct ibv_send_wr *wr, int nreq, - struct hns_roce_sge_info *sge_info) +static int set_ud_wqe(void *wqe, struct hns_roce_qp *qp, struct ibv_send_wr *wr, + unsigned int nreq, struct hns_roce_sge_info *sge_info) { struct hns_roce_ah *ah = to_hr_ah(wr->wr.ud.ah); struct hns_roce_ud_sq_wqe *ud_sq_wqe = wqe; @@ -1119,7 +1118,7 @@ static int check_rc_opcode(struct hns_roce_rc_sq_wqe *wqe, } static int set_rc_wqe(void *wqe, struct hns_roce_qp *qp, struct ibv_send_wr *wr, - int nreq, struct hns_roce_sge_info *sge_info) + unsigned int nreq, struct hns_roce_sge_info *sge_info) { struct hns_roce_rc_sq_wqe *rc_sq_wqe = wqe; struct hns_roce_v2_wqe_data_seg *dseg;