diff mbox series

[for-next] RDMA/hns: Update the kernel header file of hns

Message ID 1548123814-23093-1-git-send-email-oulijun@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Jason Gunthorpe
Headers show
Series [for-next] RDMA/hns: Update the kernel header file of hns | expand

Commit Message

Lijun Ou Jan. 22, 2019, 2:23 a.m. UTC
The hns_roce_ib_create_srq_resp is used to interact with the
user for data. As a result, it is added to apply with matching
kernel headers.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 include/uapi/rdma/hns-abi.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jason Gunthorpe Jan. 22, 2019, 2:23 a.m. UTC | #1
On Tue, Jan 22, 2019 at 10:23:34AM +0800, Lijun Ou wrote:
> The hns_roce_ib_create_srq_resp is used to interact with the
> user for data. As a result, it is added to apply with matching
> kernel headers.
> 
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>  include/uapi/rdma/hns-abi.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
> index ef3c7ec..eb76b38 100644
> +++ b/include/uapi/rdma/hns-abi.h
> @@ -52,6 +52,11 @@ struct hns_roce_ib_create_srq {
>  	__aligned_u64 que_addr;
>  };
>  
> +struct hns_roce_ib_create_srq_resp {
> +	__u32	srqn;
> +	__u32	reserved;
> +};
> +

This makes no sense, how can uapi headers be added without adding a
corresponding kernel code!?!?! What fills in srqn?

Jason
Lijun Ou Jan. 22, 2019, 3:36 a.m. UTC | #2
在 2019/1/22 10:23, Jason Gunthorpe 写道:
> On Tue, Jan 22, 2019 at 10:23:34AM +0800, Lijun Ou wrote:
>> The hns_roce_ib_create_srq_resp is used to interact with the
>> user for data. As a result, it is added to apply with matching
>> kernel headers.
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>  include/uapi/rdma/hns-abi.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
>> index ef3c7ec..eb76b38 100644
>> +++ b/include/uapi/rdma/hns-abi.h
>> @@ -52,6 +52,11 @@ struct hns_roce_ib_create_srq {
>>  	__aligned_u64 que_addr;
>>  };
>>  
>> +struct hns_roce_ib_create_srq_resp {
>> +	__u32	srqn;
>> +	__u32	reserved;
>> +};
>> +
> This makes no sense, how can uapi headers be added without adding a
> corresponding kernel code!?!?! What fills in srqn?
>
> Jason
The srqn will be obtained from hns_roce_bitmap_alloc in the kernel mode.  When create srq, it need to
transfer srqn to user by ib_copy_to_udata function. the ofed will report srqn by ib_uverbs_create_srq_resp,
nextly, the user will use it by resp.srqn when run  hns_roce_u_create_srq function.
cmdof hns_roce_create_srq need be declared by DECLARE_DRV_CMD.  for example,
     DECLARE_DRV_CMD(hns_roce_create_srq, IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq, hns_roce_ib_create_srq_resp);
lastly, it will be used to issue srq db by srqn when post srq recv in user mode.
>
Jason Gunthorpe Jan. 22, 2019, 3:57 a.m. UTC | #3
On Tue, Jan 22, 2019 at 11:36:02AM +0800, oulijun wrote:
> 在 2019/1/22 10:23, Jason Gunthorpe 写道:
> > On Tue, Jan 22, 2019 at 10:23:34AM +0800, Lijun Ou wrote:
> >> The hns_roce_ib_create_srq_resp is used to interact with the
> >> user for data. As a result, it is added to apply with matching
> >> kernel headers.
> >>
> >> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> >>  include/uapi/rdma/hns-abi.h | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
> >> index ef3c7ec..eb76b38 100644
> >> +++ b/include/uapi/rdma/hns-abi.h
> >> @@ -52,6 +52,11 @@ struct hns_roce_ib_create_srq {
> >>  	__aligned_u64 que_addr;
> >>  };
> >>  
> >> +struct hns_roce_ib_create_srq_resp {
> >> +	__u32	srqn;
> >> +	__u32	reserved;
> >> +};
> >> +
> > This makes no sense, how can uapi headers be added without adding a
> > corresponding kernel code!?!?! What fills in srqn?
> >
> > Jason
> The srqn will be obtained from hns_roce_bitmap_alloc in the kernel mode.  When create srq, it need to
> transfer srqn to user by ib_copy_to_udata function. 

The srqn is part of the ib_uverbs_create_srq_resp, it has nothing to
do with hns_roce_ib_create_srq_resp.

> the ofed will report srqn by ib_uverbs_create_srq_resp,
> nextly, the user will use it by resp.srqn when run  hns_roce_u_create_srq function.
> cmdof hns_roce_create_srq need be declared by DECLARE_DRV_CMD.  

What?

> for example,
>      DECLARE_DRV_CMD(hns_roce_create_srq,
>      IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
>      hns_roce_ib_create_srq_resp);

Are you sure this shouldn't be 

      DECLARE_DRV_CMD(hns_roce_create_srq,
            IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
	    empty);

?

Jason
Lijun Ou Jan. 22, 2019, 7 a.m. UTC | #4
在 2019/1/22 11:57, Jason Gunthorpe 写道:
> On Tue, Jan 22, 2019 at 11:36:02AM +0800, oulijun wrote:
>> 在 2019/1/22 10:23, Jason Gunthorpe 写道:
>>> On Tue, Jan 22, 2019 at 10:23:34AM +0800, Lijun Ou wrote:
>>>> The hns_roce_ib_create_srq_resp is used to interact with the
>>>> user for data. As a result, it is added to apply with matching
>>>> kernel headers.
>>>>
>>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>>>  include/uapi/rdma/hns-abi.h | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
>>>> index ef3c7ec..eb76b38 100644
>>>> +++ b/include/uapi/rdma/hns-abi.h
>>>> @@ -52,6 +52,11 @@ struct hns_roce_ib_create_srq {
>>>>  	__aligned_u64 que_addr;
>>>>  };
>>>>  
>>>> +struct hns_roce_ib_create_srq_resp {
>>>> +	__u32	srqn;
>>>> +	__u32	reserved;
>>>> +};
>>>> +
>>> This makes no sense, how can uapi headers be added without adding a
>>> corresponding kernel code!?!?! What fills in srqn?
>>>
>>> Jason
>> The srqn will be obtained from hns_roce_bitmap_alloc in the kernel mode.  When create srq, it need to
>> transfer srqn to user by ib_copy_to_udata function. 
> The srqn is part of the ib_uverbs_create_srq_resp, it has nothing to
> do with hns_roce_ib_create_srq_resp.
>
>> the ofed will report srqn by ib_uverbs_create_srq_resp,
>> nextly, the user will use it by resp.srqn when run  hns_roce_u_create_srq function.
>> cmdof hns_roce_create_srq need be declared by DECLARE_DRV_CMD.  
> What?
>
>> for example,
>>      DECLARE_DRV_CMD(hns_roce_create_srq,
>>      IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
>>      hns_roce_ib_create_srq_resp);
> Are you sure this shouldn't be 
>
>       DECLARE_DRV_CMD(hns_roce_create_srq,
>             IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
> 	    empty);
>
> ?
>
> Jason
Yes. It will build fail and prompt hns_roce_create_srq_resp has no member named srqn information.
Lijun Ou Jan. 22, 2019, 8:06 a.m. UTC | #5
在 2019/1/22 11:57, Jason Gunthorpe 写道:
> On Tue, Jan 22, 2019 at 11:36:02AM +0800, oulijun wrote:
>> 在 2019/1/22 10:23, Jason Gunthorpe 写道:
>>> On Tue, Jan 22, 2019 at 10:23:34AM +0800, Lijun Ou wrote:
>>>> The hns_roce_ib_create_srq_resp is used to interact with the
>>>> user for data. As a result, it is added to apply with matching
>>>> kernel headers.
>>>>
>>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>>>  include/uapi/rdma/hns-abi.h | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
>>>> index ef3c7ec..eb76b38 100644
>>>> +++ b/include/uapi/rdma/hns-abi.h
>>>> @@ -52,6 +52,11 @@ struct hns_roce_ib_create_srq {
>>>>  	__aligned_u64 que_addr;
>>>>  };
>>>>  
>>>> +struct hns_roce_ib_create_srq_resp {
>>>> +	__u32	srqn;
>>>> +	__u32	reserved;
>>>> +};
>>>> +
>>> This makes no sense, how can uapi headers be added without adding a
>>> corresponding kernel code!?!?! What fills in srqn?
>>>
>>> Jason
>> The srqn will be obtained from hns_roce_bitmap_alloc in the kernel mode.  When create srq, it need to
>> transfer srqn to user by ib_copy_to_udata function. 
> The srqn is part of the ib_uverbs_create_srq_resp, it has nothing to
> do with hns_roce_ib_create_srq_resp.
>
>> the ofed will report srqn by ib_uverbs_create_srq_resp,
>> nextly, the user will use it by resp.srqn when run  hns_roce_u_create_srq function.
>> cmdof hns_roce_create_srq need be declared by DECLARE_DRV_CMD.  
> What?
>
>> for example,
>>      DECLARE_DRV_CMD(hns_roce_create_srq,
>>      IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
>>      hns_roce_ib_create_srq_resp);
> Are you sure this shouldn't be 
>
>       DECLARE_DRV_CMD(hns_roce_create_srq,
>             IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
> 	    empty);
>
> ?
>
> Jason
>
Maybe we can use hns_roce_ib_create_srq_resp with adding corresponding kernel codes. I have sent the patchV2.
Jason Gunthorpe Jan. 22, 2019, 4:03 p.m. UTC | #6
On Tue, Jan 22, 2019 at 03:00:15PM +0800, oulijun wrote:
> 在 2019/1/22 11:57, Jason Gunthorpe 写道:
> > On Tue, Jan 22, 2019 at 11:36:02AM +0800, oulijun wrote:
> >> 在 2019/1/22 10:23, Jason Gunthorpe 写道:
> >>> On Tue, Jan 22, 2019 at 10:23:34AM +0800, Lijun Ou wrote:
> >>>> The hns_roce_ib_create_srq_resp is used to interact with the
> >>>> user for data. As a result, it is added to apply with matching
> >>>> kernel headers.
> >>>>
> >>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> >>>>  include/uapi/rdma/hns-abi.h | 5 +++++
> >>>>  1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
> >>>> index ef3c7ec..eb76b38 100644
> >>>> +++ b/include/uapi/rdma/hns-abi.h
> >>>> @@ -52,6 +52,11 @@ struct hns_roce_ib_create_srq {
> >>>>  	__aligned_u64 que_addr;
> >>>>  };
> >>>>  
> >>>> +struct hns_roce_ib_create_srq_resp {
> >>>> +	__u32	srqn;
> >>>> +	__u32	reserved;
> >>>> +};
> >>>> +
> >>> This makes no sense, how can uapi headers be added without adding a
> >>> corresponding kernel code!?!?! What fills in srqn?
> >>>
> >>> Jason
> >> The srqn will be obtained from hns_roce_bitmap_alloc in the kernel mode.  When create srq, it need to
> >> transfer srqn to user by ib_copy_to_udata function. 
> > The srqn is part of the ib_uverbs_create_srq_resp, it has nothing to
> > do with hns_roce_ib_create_srq_resp.
> >
> >> the ofed will report srqn by ib_uverbs_create_srq_resp,
> >> nextly, the user will use it by resp.srqn when run  hns_roce_u_create_srq function.
> >> cmdof hns_roce_create_srq need be declared by DECLARE_DRV_CMD.  
> > What?
> >
> >> for example,
> >>      DECLARE_DRV_CMD(hns_roce_create_srq,
> >>      IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
> >>      hns_roce_ib_create_srq_resp);
> > Are you sure this shouldn't be 
> >
> >       DECLARE_DRV_CMD(hns_roce_create_srq,
> >             IB_USER_VERBS_CMD_CREATE_SRQ, hns_roce_ib_create_srq,
> > 	    empty);
> >
> > ?
> >
> > Jason
> Yes. It will build fail and prompt hns_roce_create_srq_resp has no member named srqn information.

you can't introduce a struct that is never written too, it makes no
sense.

Jason
diff mbox series

Patch

diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
index ef3c7ec..eb76b38 100644
--- a/include/uapi/rdma/hns-abi.h
+++ b/include/uapi/rdma/hns-abi.h
@@ -52,6 +52,11 @@  struct hns_roce_ib_create_srq {
 	__aligned_u64 que_addr;
 };
 
+struct hns_roce_ib_create_srq_resp {
+	__u32	srqn;
+	__u32	reserved;
+};
+
 struct hns_roce_ib_create_qp {
 	__aligned_u64 buf_addr;
 	__aligned_u64 db_addr;