| Message ID | 1546932294-112720-1-git-send-email-oulijun@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2,for-next] RDMA/hns: Assign rq head pointer when enable rq record db | expand |
On Tue, Jan 08, 2019 at 03:24:54PM +0800, Lijun Ou wrote: > When flush cqe, it needs to get the pointer of rq and sq from > db address space of user and update it into qp context by > modified qp. if rq is not exist, it will not get the value > from db addresss space of user. > > Signed-off-by: Lijun Ou <oulijun@huawei.com> > --- > V1->V2: > 1. regenerate this patch in order to resolve the conflict > --- > drivers/infiniband/hw/hns/hns_roce_qp.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c > index 54031c5..0d06bd8 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_qp.c > +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c > @@ -675,6 +675,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, > dev_err(dev, "rq record doorbell map failed!\n"); > goto err_sq_dbmap; > } > + > + /* indicate kernel supports rq record db */ > + resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; > + hr_qp->rdb_en = 1; > } > } else { > if (init_attr->create_flags & > @@ -783,16 +787,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, > else > hr_qp->doorbell_qpn = cpu_to_le64(hr_qp->qpn); > > - if (udata && (udata->outlen >= sizeof(resp)) && > - (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB)) { > - > - /* indicate kernel supports rq record db */ > - resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; > + if (udata && (udata->outlen >= sizeof(resp))) { I wonder if this check is correct, don't you suppose to do? "ib_copy_to_udata(udata, &resp, min(udata->outlen, sizeof(resp)));" and remove "(udata->outlen >= sizeof(resp))" Thanks > ret = ib_copy_to_udata(udata, &resp, sizeof(resp)); > if (ret) > goto err_qp; > - > - hr_qp->rdb_en = 1; > } > hr_qp->event = hns_roce_ib_qp_event; > > @@ -969,7 +967,9 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr, > (attr_mask & IB_QP_STATE) && new_state == IB_QPS_ERR) { > if (hr_qp->sdb_en == 1) { > hr_qp->sq.head = *(int *)(hr_qp->sdb.virt_addr); > - hr_qp->rq.head = *(int *)(hr_qp->rdb.virt_addr); > + > + if (hr_qp->rdb_en == 1) > + hr_qp->rq.head = *(int *)(hr_qp->rdb.virt_addr); > } else { > dev_warn(dev, "flush cqe is not supported in userspace!\n"); > goto out; > -- > 1.9.1 >
On Tue, Jan 08, 2019 at 10:53:56AM +0200, Leon Romanovsky wrote: > On Tue, Jan 08, 2019 at 03:24:54PM +0800, Lijun Ou wrote: > > When flush cqe, it needs to get the pointer of rq and sq from > > db address space of user and update it into qp context by > > modified qp. if rq is not exist, it will not get the value > > from db addresss space of user. > > > > Signed-off-by: Lijun Ou <oulijun@huawei.com> > > V1->V2: > > 1. regenerate this patch in order to resolve the conflict > > drivers/infiniband/hw/hns/hns_roce_qp.c | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c > > index 54031c5..0d06bd8 100644 > > +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c > > @@ -675,6 +675,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, > > dev_err(dev, "rq record doorbell map failed!\n"); > > goto err_sq_dbmap; > > } > > + > > + /* indicate kernel supports rq record db */ > > + resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; > > + hr_qp->rdb_en = 1; > > } > > } else { > > if (init_attr->create_flags & > > @@ -783,16 +787,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, > > else > > hr_qp->doorbell_qpn = cpu_to_le64(hr_qp->qpn); > > > > - if (udata && (udata->outlen >= sizeof(resp)) && > > - (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB)) { > > - > > - /* indicate kernel supports rq record db */ > > - resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; > > + if (udata && (udata->outlen >= sizeof(resp))) { > > I wonder if this check is correct, don't you suppose to do? > "ib_copy_to_udata(udata, &resp, min(udata->outlen, sizeof(resp)));" > and remove "(udata->outlen >= sizeof(resp))" yes It should also zero fill beyond, but all our drivers are broken this way today. Jason
在 2019/1/8 16:53, Leon Romanovsky 写道: > On Tue, Jan 08, 2019 at 03:24:54PM +0800, Lijun Ou wrote: >> When flush cqe, it needs to get the pointer of rq and sq from >> db address space of user and update it into qp context by >> modified qp. if rq is not exist, it will not get the value >> from db addresss space of user. >> >> Signed-off-by: Lijun Ou <oulijun@huawei.com> >> --- >> V1->V2: >> 1. regenerate this patch in order to resolve the conflict >> --- >> drivers/infiniband/hw/hns/hns_roce_qp.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c >> index 54031c5..0d06bd8 100644 >> --- a/drivers/infiniband/hw/hns/hns_roce_qp.c >> +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c >> @@ -675,6 +675,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, >> dev_err(dev, "rq record doorbell map failed!\n"); >> goto err_sq_dbmap; >> } >> + >> + /* indicate kernel supports rq record db */ >> + resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; >> + hr_qp->rdb_en = 1; >> } >> } else { >> if (init_attr->create_flags & >> @@ -783,16 +787,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, >> else >> hr_qp->doorbell_qpn = cpu_to_le64(hr_qp->qpn); >> >> - if (udata && (udata->outlen >= sizeof(resp)) && >> - (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB)) { >> - >> - /* indicate kernel supports rq record db */ >> - resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; >> + if (udata && (udata->outlen >= sizeof(resp))) { > I wonder if this check is correct, don't you suppose to do? > "ib_copy_to_udata(udata, &resp, min(udata->outlen, sizeof(resp)));" > and remove "(udata->outlen >= sizeof(resp))" > > Thanks I just want to move hr_qp->rdb_en and delete the unnecessary trigger condition hr_dev->caps.flags. according to your advice, it will change the original meaning with author. I am not sure if there are other problems thanks >> ret = ib_copy_to_udata(udata, &resp, sizeof(resp)); >> if (ret) >> goto err_qp; >> - >> - hr_qp->rdb_en = 1; >> } >> hr_qp->event = hns_roce_ib_qp_event; >> >> @@ -969,7 +967,9 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr, >> (attr_mask & IB_QP_STATE) && new_state == IB_QPS_ERR) { >> if (hr_qp->sdb_en == 1) { >> hr_qp->sq.head = *(int *)(hr_qp->sdb.virt_addr); >> - hr_qp->rq.head = *(int *)(hr_qp->rdb.virt_addr); >> + >> + if (hr_qp->rdb_en == 1) >> + hr_qp->rq.head = *(int *)(hr_qp->rdb.virt_addr); >> } else { >> dev_warn(dev, "flush cqe is not supported in userspace!\n"); >> goto out; >> -- >> 1.9.1 >>
diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c index 54031c5..0d06bd8 100644 --- a/drivers/infiniband/hw/hns/hns_roce_qp.c +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c @@ -675,6 +675,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, dev_err(dev, "rq record doorbell map failed!\n"); goto err_sq_dbmap; } + + /* indicate kernel supports rq record db */ + resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; + hr_qp->rdb_en = 1; } } else { if (init_attr->create_flags & @@ -783,16 +787,10 @@ static int hns_roce_create_qp_common(struct hns_roce_dev *hr_dev, else hr_qp->doorbell_qpn = cpu_to_le64(hr_qp->qpn); - if (udata && (udata->outlen >= sizeof(resp)) && - (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB)) { - - /* indicate kernel supports rq record db */ - resp.cap_flags |= HNS_ROCE_SUPPORT_RQ_RECORD_DB; + if (udata && (udata->outlen >= sizeof(resp))) { ret = ib_copy_to_udata(udata, &resp, sizeof(resp)); if (ret) goto err_qp; - - hr_qp->rdb_en = 1; } hr_qp->event = hns_roce_ib_qp_event; @@ -969,7 +967,9 @@ int hns_roce_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr, (attr_mask & IB_QP_STATE) && new_state == IB_QPS_ERR) { if (hr_qp->sdb_en == 1) { hr_qp->sq.head = *(int *)(hr_qp->sdb.virt_addr); - hr_qp->rq.head = *(int *)(hr_qp->rdb.virt_addr); + + if (hr_qp->rdb_en == 1) + hr_qp->rq.head = *(int *)(hr_qp->rdb.virt_addr); } else { dev_warn(dev, "flush cqe is not supported in userspace!\n"); goto out;
When flush cqe, it needs to get the pointer of rq and sq from db address space of user and update it into qp context by modified qp. if rq is not exist, it will not get the value from db addresss space of user. Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1->V2: 1. regenerate this patch in order to resolve the conflict --- drivers/infiniband/hw/hns/hns_roce_qp.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)