Re: [PATCH v2 for-next 7/7] RDMA/hns: Optimize qp doorbell allocation flow

[Jason Gunthorpe <jgg@ziepe.ca>]

On Sat, Feb 22, 2020 at 03:22:18PM +0800, Weihang Li wrote:
> 
> 
> On 2020/2/19 21:19, Jason Gunthorpe wrote:
> > On Wed, Feb 19, 2020 at 04:14:36PM +0800, Weihang Li wrote:
> >>
> >>
> >> On 2020/2/19 8:52, Jason Gunthorpe wrote:
> >>> On Mon, Feb 10, 2020 at 05:08:40PM +0800, Weihang Li wrote:
> >>>> From: Xi Wang <wangxi11@huawei.com>
> >>>>
> >>>> Encapsulate the kernel qp doorbell allocation related code into 2
> >>>> functions: alloc_qp_db() and free_qp_db().
> >>>>
> >>>> Signed-off-by: Xi Wang <wangxi11@huawei.com>
> >>>> Signed-off-by: Weihang Li <liweihang@huawei.com>
> >>>>  drivers/infiniband/hw/hns/hns_roce_qp.c | 214 +++++++++++++++++---------------
> >>>>  1 file changed, 113 insertions(+), 101 deletions(-)
> >>>>
> >>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
> >>>> index ad34187..46785f1 100644
> >>>> +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
> >>>> @@ -844,6 +844,96 @@ static void free_qp_buf(struct hns_roce_dev *hr_dev, struct hns_roce_qp *hr_qp)
> >>>>  		free_rq_inline_buf(hr_qp);
> >>>>  }
> >>>>  
> >>>> +#define user_qp_has_sdb(hr_dev, init_attr, udata, resp, ucmd) \
> >>>> +		((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_SQ_RECORD_DB) && \
> >>>> +		udata->outlen >= sizeof(*resp) && \
> >>>> +		hns_roce_qp_has_sq(init_attr) && udata->inlen >= sizeof(*ucmd))
> >>>> +
> >>>> +#define user_qp_has_rdb(hr_dev, init_attr, udata, resp) \
> >>>> +		((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB) && \
> >>>> +		udata->outlen >= sizeof(*resp) && \
> >>>> +		hns_roce_qp_has_rq(init_attr))
> >>>> +
> >>>> +#define kernel_qp_has_rdb(hr_dev, init_attr) \
> >>>> +		((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB) && \
> >>>> +		hns_roce_qp_has_rq(init_attr))
> >>>
> >>> static inline functions not defines please
> >>>
> >>
> >> OK, I will change them into inline functions.
> >>
> >>> Also, these tests against inline and outlen look very strange. What
> >>> are they doing?
> >>>
> >>> Jason
> >>>
> >>
> >> These judgement about inlen and outlen is for compatibility reasons,
> >> previous discussions can be found at:
> >>
> >> https://patchwork.kernel.org/patch/10172233/
> > 
> > Something is wrong, it should be testing the legnth using a
> > field_offset_off kind of scheme, not sizeof(*resp)
> > 
> > Jason
> > 
> Hi Jason,
> 
> Do you means
> 
> 	udata->outlen >= sizeof(*resp)
> 
> should be changed into:
> 
> 	udata->out_len >= offsetof(typeof(*resp), cap_flags)
> 
> If yes, I will fix other similar codes with this issue in hns drivers.

Probably offsetofend though, right?

But yes, that is how the general 'feature test for old userspace with
old kernel ABI' should look

Jason