| Message ID | 1624010765-1029-2-git-send-email-liweihang@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RDMA/hns: Use new interfaces to write/read fields | expand |
On Fri, Jun 18, 2021 at 06:05:58PM +0800, Weihang Li wrote: > Fix complains from sparse about "dubious: x & !y" when calling > hr_reg_write(ctx, field, !!val) by using "val ? 1 : 0" instead of "!!val". > > Fixes: dc504774408b ("RDMA/hns: Use new interface to set MPT related fields") > Fixes: 495c24808ce7 ("RDMA/hns: Add XRC subtype in QPC and XRC type in SRQC") > Fixes: 782832f25404 ("RDMA/hns: Simplify the function config_eqc()") > Signed-off-by: Weihang Li <liweihang@huawei.com> > --- > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > index fbc45b9..6452ccc 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > @@ -3013,15 +3013,15 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev, > hr_reg_enable(mpt_entry, MPT_L_INV_EN); > > hr_reg_write(mpt_entry, MPT_BIND_EN, > - !!(mr->access & IB_ACCESS_MW_BIND)); > + mr->access & IB_ACCESS_MW_BIND ? 1 : 0); Err, I'm still confused where the sparse warning is coming from A hr_reg_write_bool() would be cleaner? Jason
Jason Gunthorpe <jgg@nvidia.com> 于2021年6月18日周五 下午8:49写道： > > On Fri, Jun 18, 2021 at 06:05:58PM +0800, Weihang Li wrote: > > Fix complains from sparse about "dubious: x & !y" when calling > > hr_reg_write(ctx, field, !!val) by using "val ? 1 : 0" instead of "!!val". > > > > Fixes: dc504774408b ("RDMA/hns: Use new interface to set MPT related fields") > > Fixes: 495c24808ce7 ("RDMA/hns: Add XRC subtype in QPC and XRC type in SRQC") > > Fixes: 782832f25404 ("RDMA/hns: Simplify the function config_eqc()") > > Signed-off-by: Weihang Li <liweihang@huawei.com> > > --- > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > index fbc45b9..6452ccc 100644 > > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > @@ -3013,15 +3013,15 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev, > > hr_reg_enable(mpt_entry, MPT_L_INV_EN); > > > > hr_reg_write(mpt_entry, MPT_BIND_EN, > > - !!(mr->access & IB_ACCESS_MW_BIND)); > > + mr->access & IB_ACCESS_MW_BIND ? 1 : 0); > > Err, I'm still confused where the sparse warning is coming from Hi, Jason, i found some code in sparse/evaluate.c: const unsigned left_not = expr->left->type == EXPR_PREOP && expr->left->op == '!'; const unsigned right_not = expr->right->type == EXPR_PREOP && expr->right->op == '!'; if ((op == '&' || op == '|') && (left_not || right_not)) warning(expr->pos, "dubious: %sx %c %sy", I guess the "dubious" is, if somebody use "&" or "|", maybe he want to bitwise operate a number instead of a bool. Someone think sparse wants to remind whether he want to use a logical "&&" instead of a bitwise "&". Maybe it is a typo. > > A hr_reg_write_bool() would be cleaner? > > Jason
On Fri, Jun 18, 2021 at 11:22:46PM +0800, hhh ching wrote: > Jason Gunthorpe <jgg@nvidia.com> 于2021年6月18日周五 下午8:49写道： > > > > On Fri, Jun 18, 2021 at 06:05:58PM +0800, Weihang Li wrote: > > > Fix complains from sparse about "dubious: x & !y" when calling > > > hr_reg_write(ctx, field, !!val) by using "val ? 1 : 0" instead of "!!val". > > > > > > Fixes: dc504774408b ("RDMA/hns: Use new interface to set MPT related fields") > > > Fixes: 495c24808ce7 ("RDMA/hns: Add XRC subtype in QPC and XRC type in SRQC") > > > Fixes: 782832f25404 ("RDMA/hns: Simplify the function config_eqc()") > > > Signed-off-by: Weihang Li <liweihang@huawei.com> > > > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 14 +++++++------- > > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > > index fbc45b9..6452ccc 100644 > > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c > > > @@ -3013,15 +3013,15 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev, > > > hr_reg_enable(mpt_entry, MPT_L_INV_EN); > > > > > > hr_reg_write(mpt_entry, MPT_BIND_EN, > > > - !!(mr->access & IB_ACCESS_MW_BIND)); > > > + mr->access & IB_ACCESS_MW_BIND ? 1 : 0); > > > > Err, I'm still confused where the sparse warning is coming from > > Hi, Jason, i found some code in sparse/evaluate.c: > const unsigned left_not = expr->left->type == EXPR_PREOP && > expr->left->op == '!'; > const unsigned right_not = expr->right->type == EXPR_PREOP && > expr->right->op == '!'; > if ((op == '&' || op == '|') && (left_not || right_not)) > warning(expr->pos, "dubious: %sx %c %sy", > > I guess the "dubious" is, if somebody use "&" or "|", maybe he want > to bitwise operate a number instead of a bool. Oh I see, yes, that does many some sense actually > > A hr_reg_write_bool() would be cleaner? Which makes me further think this is the right direction Jason
On 2021/6/19 0:10, Jason Gunthorpe wrote: > On Fri, Jun 18, 2021 at 11:22:46PM +0800, hhh ching wrote: >> Jason Gunthorpe <jgg@nvidia.com> 于2021年6月18日周五 下午8:49写道： >>> >>> On Fri, Jun 18, 2021 at 06:05:58PM +0800, Weihang Li wrote: >>>> Fix complains from sparse about "dubious: x & !y" when calling >>>> hr_reg_write(ctx, field, !!val) by using "val ? 1 : 0" instead of "!!val". >>>> >>>> Fixes: dc504774408b ("RDMA/hns: Use new interface to set MPT related fields") >>>> Fixes: 495c24808ce7 ("RDMA/hns: Add XRC subtype in QPC and XRC type in SRQC") >>>> Fixes: 782832f25404 ("RDMA/hns: Simplify the function config_eqc()") >>>> Signed-off-by: Weihang Li <liweihang@huawei.com> >>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 14 +++++++------- >>>> 1 file changed, 7 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>>> index fbc45b9..6452ccc 100644 >>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >>>> @@ -3013,15 +3013,15 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev, >>>> hr_reg_enable(mpt_entry, MPT_L_INV_EN); >>>> >>>> hr_reg_write(mpt_entry, MPT_BIND_EN, >>>> - !!(mr->access & IB_ACCESS_MW_BIND)); >>>> + mr->access & IB_ACCESS_MW_BIND ? 1 : 0); >>> >>> Err, I'm still confused where the sparse warning is coming from >> >> Hi, Jason, i found some code in sparse/evaluate.c: >> const unsigned left_not = expr->left->type == EXPR_PREOP && >> expr->left->op == '!'; >> const unsigned right_not = expr->right->type == EXPR_PREOP && >> expr->right->op == '!'; >> if ((op == '&' || op == '|') && (left_not || right_not)) >> warning(expr->pos, "dubious: %sx %c %sy", >> >> I guess the "dubious" is, if somebody use "&" or "|", maybe he want >> to bitwise operate a number instead of a bool. > > Oh I see, yes, that does many some sense actually > >>> A hr_reg_write_bool() would be cleaner? > > Which makes me further think this is the right direction > > Jason > I see, thanks. Weihang
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c index fbc45b9..6452ccc 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c @@ -3013,15 +3013,15 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev, hr_reg_enable(mpt_entry, MPT_L_INV_EN); hr_reg_write(mpt_entry, MPT_BIND_EN, - !!(mr->access & IB_ACCESS_MW_BIND)); + mr->access & IB_ACCESS_MW_BIND ? 1 : 0); hr_reg_write(mpt_entry, MPT_ATOMIC_EN, - !!(mr->access & IB_ACCESS_REMOTE_ATOMIC)); + mr->access & IB_ACCESS_REMOTE_ATOMIC ? 1 : 0); hr_reg_write(mpt_entry, MPT_RR_EN, - !!(mr->access & IB_ACCESS_REMOTE_READ)); + mr->access & IB_ACCESS_REMOTE_READ ? 1 : 0); hr_reg_write(mpt_entry, MPT_RW_EN, - !!(mr->access & IB_ACCESS_REMOTE_WRITE)); + mr->access & IB_ACCESS_REMOTE_WRITE ? 1 : 0); hr_reg_write(mpt_entry, MPT_LW_EN, - !!((mr->access & IB_ACCESS_LOCAL_WRITE))); + mr->access & IB_ACCESS_LOCAL_WRITE ? 1 : 0); mpt_entry->len_l = cpu_to_le32(lower_32_bits(mr->size)); mpt_entry->len_h = cpu_to_le32(upper_32_bits(mr->size)); @@ -5617,7 +5617,7 @@ static int hns_roce_v2_write_srqc(struct hns_roce_srq *srq, void *mb_buf) hr_reg_write(ctx, SRQC_SRQ_ST, 1); hr_reg_write(ctx, SRQC_SRQ_TYPE, - !!(srq->ibsrq.srq_type == IB_SRQT_XRC)); + srq->ibsrq.srq_type == IB_SRQT_XRC ? 1 : 0); hr_reg_write(ctx, SRQC_PD, to_hr_pd(srq->ibsrq.pd)->pdn); hr_reg_write(ctx, SRQC_SRQN, srq->srqn); hr_reg_write(ctx, SRQC_XRCD, srq->xrcdn); @@ -6168,7 +6168,7 @@ static int config_eqc(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq, hr_reg_write(eqc, EQC_NEX_EQE_BA_L, eqe_ba[1] >> 12); hr_reg_write(eqc, EQC_NEX_EQE_BA_H, eqe_ba[1] >> 44); hr_reg_write(eqc, EQC_EQE_SIZE, - !!(eq->eqe_size == HNS_ROCE_V3_EQE_SIZE)); + eq->eqe_size == HNS_ROCE_V3_EQE_SIZE ? 1 : 0); return 0; }
Fix complains from sparse about "dubious: x & !y" when calling hr_reg_write(ctx, field, !!val) by using "val ? 1 : 0" instead of "!!val". Fixes: dc504774408b ("RDMA/hns: Use new interface to set MPT related fields") Fixes: 495c24808ce7 ("RDMA/hns: Add XRC subtype in QPC and XRC type in SRQC") Fixes: 782832f25404 ("RDMA/hns: Simplify the function config_eqc()") Signed-off-by: Weihang Li <liweihang@huawei.com> --- drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)