| Message ID | 20181128183548.4599.91984.stgit@scvm10.sc.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IB/hfi1: Clean up and code improvements | expand |
On Wed, Nov 28, 2018 at 10:35:55AM -0800, Dennis Dalessandro wrote: > From: Kamenee Arumugam <kamenee.arumugam@intel.com> > > The rvt_rwqe and rvt_rwq struct elements are shared between > rdmavt and the providers but not in uapi directory. > As per the comment in > https://marc.info/?l=linux-rdma&m=152296522708522&w=2, > The hfi1 driver and the rdma core driver are not using > shared structures in the uapi directory. > > Move rvt_rwqe and rvt_rwq struct into rvt-abi.h header in uapi > directory. Create rvt_krwq kernel struct to separate > it from the user version. > > Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com> > Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > Signed-off-by: Kamenee Arumugam <kamenee.arumugam@intel.com> > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> > --- > include/rdma/rdmavt_qp.h | 26 +------------------------- > include/uapi/rdma/rvt-abi.h | 27 +++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+), 25 deletions(-) > > diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h > index cbafb18..1ef17a6 100644 > --- a/include/rdma/rdmavt_qp.h > +++ b/include/rdma/rdmavt_qp.h > @@ -52,6 +52,7 @@ > #include <rdma/ib_pack.h> > #include <rdma/ib_verbs.h> > #include <rdma/rdmavt_cq.h> > +#include <rdma/rvt-abi.h> > /* > * Atomic bit definitions for r_aflags. > */ > @@ -177,31 +178,6 @@ struct rvt_swqe { > struct rvt_sge sg_list[0]; > }; > > -/* > - * Receive work request queue entry. > - * The size of the sg_list is determined when the QP (or SRQ) is created > - * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). > - */ > -struct rvt_rwqe { > - u64 wr_id; > - u8 num_sge; > - struct ib_sge sg_list[0]; > -}; > - > -/* > - * This structure is used to contain the head pointer, tail pointer, > - * and receive work queue entries as a single memory allocation so > - * it can be mmap'ed into user space. > - * Note that the wq array elements are variable size so you can't > - * just index into the array to get the N'th element; > - * use get_rwqe_ptr() instead. > - */ > -struct rvt_rwq { > - u32 head; /* new work requests posted to the head */ > - u32 tail; /* receives pull requests from here. */ > - struct rvt_rwqe wq[0]; > -}; > - > struct rvt_rq { > struct rvt_rwq *wq; > u32 size; /* size of RWQE array */ > diff --git a/include/uapi/rdma/rvt-abi.h b/include/uapi/rdma/rvt-abi.h > index 9c3c7de..b8ec20f 100644 > --- a/include/uapi/rdma/rvt-abi.h > +++ b/include/uapi/rdma/rvt-abi.h > @@ -27,4 +27,31 @@ struct rvt_cq_wc { > struct ib_uverbs_wc uqueue[0]; > }; > > +/* > + * Receive work request queue entry. > + * The size of the sg_list is determined when the QP (or SRQ) is created > + * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). > + */ > +struct rvt_rwqe { > + __u64 wr_id; > + __u8 num_sge; > + __u8 padding[7]; > + struct ib_sge sg_list[0]; The same comment as for previous patch. > +}; > + > +/* > + * This structure is used to contain the head pointer, tail pointer, > + * and receive work queue entries as a single memory allocation so > + * it can be mmap'ed into user space. > + * Note that the wq array elements are variable size so you can't > + * just index into the array to get the N'th element; > + * use get_rwqe_ptr() instead. > + */ > +struct rvt_rwq { > + /* new work requests posted to the head */ > + RDMA_ATOMIC_UAPI(u32, head); > + /* receives pull requests from here. */ > + RDMA_ATOMIC_UAPI(u32, tail); > + struct rvt_rwqe wq[0]; > +}; > #endif /* RVT_ABI_USER_H */ >
On Wed, Nov 28, 2018 at 09:01:45PM +0200, Leon Romanovsky wrote: > On Wed, Nov 28, 2018 at 10:35:55AM -0800, Dennis Dalessandro wrote: > > From: Kamenee Arumugam <kamenee.arumugam@intel.com> > > > > The rvt_rwqe and rvt_rwq struct elements are shared between > > rdmavt and the providers but not in uapi directory. > > As per the comment in > > https://marc.info/?l=linux-rdma&m=152296522708522&w=2, > > The hfi1 driver and the rdma core driver are not using > > shared structures in the uapi directory. > > > > Move rvt_rwqe and rvt_rwq struct into rvt-abi.h header in uapi > > directory. Create rvt_krwq kernel struct to separate > > it from the user version. > > > > Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com> > > Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > > Signed-off-by: Kamenee Arumugam <kamenee.arumugam@intel.com> > > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> > > include/rdma/rdmavt_qp.h | 26 +------------------------- > > include/uapi/rdma/rvt-abi.h | 27 +++++++++++++++++++++++++++ > > 2 files changed, 28 insertions(+), 25 deletions(-) > > > > diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h > > index cbafb18..1ef17a6 100644 > > +++ b/include/rdma/rdmavt_qp.h > > @@ -52,6 +52,7 @@ > > #include <rdma/ib_pack.h> > > #include <rdma/ib_verbs.h> > > #include <rdma/rdmavt_cq.h> > > +#include <rdma/rvt-abi.h> > > /* > > * Atomic bit definitions for r_aflags. > > */ > > @@ -177,31 +178,6 @@ struct rvt_swqe { > > struct rvt_sge sg_list[0]; > > }; > > > > -/* > > - * Receive work request queue entry. > > - * The size of the sg_list is determined when the QP (or SRQ) is created > > - * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). > > - */ > > -struct rvt_rwqe { > > - u64 wr_id; > > - u8 num_sge; > > - struct ib_sge sg_list[0]; > > -}; > > - > > -/* > > - * This structure is used to contain the head pointer, tail pointer, > > - * and receive work queue entries as a single memory allocation so > > - * it can be mmap'ed into user space. > > - * Note that the wq array elements are variable size so you can't > > - * just index into the array to get the N'th element; > > - * use get_rwqe_ptr() instead. > > - */ > > -struct rvt_rwq { > > - u32 head; /* new work requests posted to the head */ > > - u32 tail; /* receives pull requests from here. */ > > - struct rvt_rwqe wq[0]; > > -}; > > - > > struct rvt_rq { > > struct rvt_rwq *wq; > > u32 size; /* size of RWQE array */ > > diff --git a/include/uapi/rdma/rvt-abi.h b/include/uapi/rdma/rvt-abi.h > > index 9c3c7de..b8ec20f 100644 > > +++ b/include/uapi/rdma/rvt-abi.h > > @@ -27,4 +27,31 @@ struct rvt_cq_wc { > > struct ib_uverbs_wc uqueue[0]; > > }; > > > > +/* > > + * Receive work request queue entry. > > + * The size of the sg_list is determined when the QP (or SRQ) is created > > + * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). > > + */ > > +struct rvt_rwqe { > > + __u64 wr_id; > > + __u8 num_sge; > > + __u8 padding[7]; > > + struct ib_sge sg_list[0]; > > The same comment as for previous patch. I thought rdma-core's travis build checks for this kind of stuff.. Did you put this in rdma-core and run buildlib/cbuild pkg travis? Jason
On Wed, Nov 28, 2018 at 12:27:49PM -0700, Jason Gunthorpe wrote: > On Wed, Nov 28, 2018 at 09:01:45PM +0200, Leon Romanovsky wrote: > > On Wed, Nov 28, 2018 at 10:35:55AM -0800, Dennis Dalessandro wrote: > > > From: Kamenee Arumugam <kamenee.arumugam@intel.com> > > > > > > The rvt_rwqe and rvt_rwq struct elements are shared between > > > rdmavt and the providers but not in uapi directory. > > > As per the comment in > > > https://marc.info/?l=linux-rdma&m=152296522708522&w=2, > > > The hfi1 driver and the rdma core driver are not using > > > shared structures in the uapi directory. > > > > > > Move rvt_rwqe and rvt_rwq struct into rvt-abi.h header in uapi > > > directory. Create rvt_krwq kernel struct to separate > > > it from the user version. > > > > > > Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com> > > > Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > > > Signed-off-by: Kamenee Arumugam <kamenee.arumugam@intel.com> > > > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> > > > include/rdma/rdmavt_qp.h | 26 +------------------------- > > > include/uapi/rdma/rvt-abi.h | 27 +++++++++++++++++++++++++++ > > > 2 files changed, 28 insertions(+), 25 deletions(-) > > > > > > diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h > > > index cbafb18..1ef17a6 100644 > > > +++ b/include/rdma/rdmavt_qp.h > > > @@ -52,6 +52,7 @@ > > > #include <rdma/ib_pack.h> > > > #include <rdma/ib_verbs.h> > > > #include <rdma/rdmavt_cq.h> > > > +#include <rdma/rvt-abi.h> > > > /* > > > * Atomic bit definitions for r_aflags. > > > */ > > > @@ -177,31 +178,6 @@ struct rvt_swqe { > > > struct rvt_sge sg_list[0]; > > > }; > > > > > > -/* > > > - * Receive work request queue entry. > > > - * The size of the sg_list is determined when the QP (or SRQ) is created > > > - * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). > > > - */ > > > -struct rvt_rwqe { > > > - u64 wr_id; > > > - u8 num_sge; > > > - struct ib_sge sg_list[0]; > > > -}; > > > - > > > -/* > > > - * This structure is used to contain the head pointer, tail pointer, > > > - * and receive work queue entries as a single memory allocation so > > > - * it can be mmap'ed into user space. > > > - * Note that the wq array elements are variable size so you can't > > > - * just index into the array to get the N'th element; > > > - * use get_rwqe_ptr() instead. > > > - */ > > > -struct rvt_rwq { > > > - u32 head; /* new work requests posted to the head */ > > > - u32 tail; /* receives pull requests from here. */ > > > - struct rvt_rwqe wq[0]; > > > -}; > > > - > > > struct rvt_rq { > > > struct rvt_rwq *wq; > > > u32 size; /* size of RWQE array */ > > > diff --git a/include/uapi/rdma/rvt-abi.h b/include/uapi/rdma/rvt-abi.h > > > index 9c3c7de..b8ec20f 100644 > > > +++ b/include/uapi/rdma/rvt-abi.h > > > @@ -27,4 +27,31 @@ struct rvt_cq_wc { > > > struct ib_uverbs_wc uqueue[0]; > > > }; > > > > > > +/* > > > + * Receive work request queue entry. > > > + * The size of the sg_list is determined when the QP (or SRQ) is created > > > + * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). > > > + */ > > > +struct rvt_rwqe { > > > + __u64 wr_id; > > > + __u8 num_sge; > > > + __u8 padding[7]; > > > + struct ib_sge sg_list[0]; > > > > The same comment as for previous patch. > > I thought rdma-core's travis build checks for this kind of stuff.. Did > you put this in rdma-core and run buildlib/cbuild pkg travis? Are you asking me? I don't. > > Jason
On 11/28/2018 2:52 PM, Leon Romanovsky wrote: > On Wed, Nov 28, 2018 at 12:27:49PM -0700, Jason Gunthorpe wrote: >> On Wed, Nov 28, 2018 at 09:01:45PM +0200, Leon Romanovsky wrote: >>> On Wed, Nov 28, 2018 at 10:35:55AM -0800, Dennis Dalessandro wrote: >>>> From: Kamenee Arumugam <kamenee.arumugam@intel.com> >>>> >>>> The rvt_rwqe and rvt_rwq struct elements are shared between >>>> rdmavt and the providers but not in uapi directory. >>>> As per the comment in >>>> https://marc.info/?l=linux-rdma&m=152296522708522&w=2, >>>> The hfi1 driver and the rdma core driver are not using >>>> shared structures in the uapi directory. >>>> >>>> Move rvt_rwqe and rvt_rwq struct into rvt-abi.h header in uapi >>>> directory. Create rvt_krwq kernel struct to separate >>>> it from the user version. >>>> >>>> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com> >>>> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> >>>> Signed-off-by: Kamenee Arumugam <kamenee.arumugam@intel.com> >>>> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> >>>> include/rdma/rdmavt_qp.h | 26 +------------------------- >>>> include/uapi/rdma/rvt-abi.h | 27 +++++++++++++++++++++++++++ >>>> 2 files changed, 28 insertions(+), 25 deletions(-) >>>> >>>> diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h >>>> index cbafb18..1ef17a6 100644 >>>> +++ b/include/rdma/rdmavt_qp.h >>>> @@ -52,6 +52,7 @@ >>>> #include <rdma/ib_pack.h> >>>> #include <rdma/ib_verbs.h> >>>> #include <rdma/rdmavt_cq.h> >>>> +#include <rdma/rvt-abi.h> >>>> /* >>>> * Atomic bit definitions for r_aflags. >>>> */ >>>> @@ -177,31 +178,6 @@ struct rvt_swqe { >>>> struct rvt_sge sg_list[0]; >>>> }; >>>> >>>> -/* >>>> - * Receive work request queue entry. >>>> - * The size of the sg_list is determined when the QP (or SRQ) is created >>>> - * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). >>>> - */ >>>> -struct rvt_rwqe { >>>> - u64 wr_id; >>>> - u8 num_sge; >>>> - struct ib_sge sg_list[0]; >>>> -}; >>>> - >>>> -/* >>>> - * This structure is used to contain the head pointer, tail pointer, >>>> - * and receive work queue entries as a single memory allocation so >>>> - * it can be mmap'ed into user space. >>>> - * Note that the wq array elements are variable size so you can't >>>> - * just index into the array to get the N'th element; >>>> - * use get_rwqe_ptr() instead. >>>> - */ >>>> -struct rvt_rwq { >>>> - u32 head; /* new work requests posted to the head */ >>>> - u32 tail; /* receives pull requests from here. */ >>>> - struct rvt_rwqe wq[0]; >>>> -}; >>>> - >>>> struct rvt_rq { >>>> struct rvt_rwq *wq; >>>> u32 size; /* size of RWQE array */ >>>> diff --git a/include/uapi/rdma/rvt-abi.h b/include/uapi/rdma/rvt-abi.h >>>> index 9c3c7de..b8ec20f 100644 >>>> +++ b/include/uapi/rdma/rvt-abi.h >>>> @@ -27,4 +27,31 @@ struct rvt_cq_wc { >>>> struct ib_uverbs_wc uqueue[0]; >>>> }; >>>> >>>> +/* >>>> + * Receive work request queue entry. >>>> + * The size of the sg_list is determined when the QP (or SRQ) is created >>>> + * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). >>>> + */ >>>> +struct rvt_rwqe { >>>> + __u64 wr_id; >>>> + __u8 num_sge; >>>> + __u8 padding[7]; >>>> + struct ib_sge sg_list[0]; >>> >>> The same comment as for previous patch. >> >> I thought rdma-core's travis build checks for this kind of stuff.. Did >> you put this in rdma-core and run buildlib/cbuild pkg travis? > > Are you asking me? I don't. > We'll send a v2 which includes adding the #includes. -Denny
diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h index cbafb18..1ef17a6 100644 --- a/include/rdma/rdmavt_qp.h +++ b/include/rdma/rdmavt_qp.h @@ -52,6 +52,7 @@ #include <rdma/ib_pack.h> #include <rdma/ib_verbs.h> #include <rdma/rdmavt_cq.h> +#include <rdma/rvt-abi.h> /* * Atomic bit definitions for r_aflags. */ @@ -177,31 +178,6 @@ struct rvt_swqe { struct rvt_sge sg_list[0]; }; -/* - * Receive work request queue entry. - * The size of the sg_list is determined when the QP (or SRQ) is created - * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). - */ -struct rvt_rwqe { - u64 wr_id; - u8 num_sge; - struct ib_sge sg_list[0]; -}; - -/* - * This structure is used to contain the head pointer, tail pointer, - * and receive work queue entries as a single memory allocation so - * it can be mmap'ed into user space. - * Note that the wq array elements are variable size so you can't - * just index into the array to get the N'th element; - * use get_rwqe_ptr() instead. - */ -struct rvt_rwq { - u32 head; /* new work requests posted to the head */ - u32 tail; /* receives pull requests from here. */ - struct rvt_rwqe wq[0]; -}; - struct rvt_rq { struct rvt_rwq *wq; u32 size; /* size of RWQE array */ diff --git a/include/uapi/rdma/rvt-abi.h b/include/uapi/rdma/rvt-abi.h index 9c3c7de..b8ec20f 100644 --- a/include/uapi/rdma/rvt-abi.h +++ b/include/uapi/rdma/rvt-abi.h @@ -27,4 +27,31 @@ struct rvt_cq_wc { struct ib_uverbs_wc uqueue[0]; }; +/* + * Receive work request queue entry. + * The size of the sg_list is determined when the QP (or SRQ) is created + * and stored in qp->r_rq.max_sge (or srq->rq.max_sge). + */ +struct rvt_rwqe { + __u64 wr_id; + __u8 num_sge; + __u8 padding[7]; + struct ib_sge sg_list[0]; +}; + +/* + * This structure is used to contain the head pointer, tail pointer, + * and receive work queue entries as a single memory allocation so + * it can be mmap'ed into user space. + * Note that the wq array elements are variable size so you can't + * just index into the array to get the N'th element; + * use get_rwqe_ptr() instead. + */ +struct rvt_rwq { + /* new work requests posted to the head */ + RDMA_ATOMIC_UAPI(u32, head); + /* receives pull requests from here. */ + RDMA_ATOMIC_UAPI(u32, tail); + struct rvt_rwqe wq[0]; +}; #endif /* RVT_ABI_USER_H */