* [PATCH v1 0/1] rxe driver should dynamically caclculate inline data size @ 2019-10-23 17:32 rao Shoaib 2019-10-23 17:32 ` [PATCH v1 1/1] rxe: calculate inline data size based on requested values rao Shoaib 0 siblings, 1 reply; 5+ messages in thread From: rao Shoaib @ 2019-10-23 17:32 UTC (permalink / raw) To: monis, dledford, sean.hefty, hal.rosenstock, linux-rdma, linux-kernel Cc: Rao Shoaib From: Rao Shoaib <rao.shoaib@oracle.com> Resending because of typo in the email addresses. Currently rxe driver has a hard coded value for inline data size, where as mlx5 driver calculates the size of inline data and number of SGE's to use based on the values in the qp request. Some applications depend on this behavior. This patch changes rxe to dynamically calculate the values. Rao Shoaib (1): rxe: calculate inline data size based on requested values drivers/infiniband/sw/rxe/rxe_param.h | 2 +- drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1 1/1] rxe: calculate inline data size based on requested values 2019-10-23 17:32 [PATCH v1 0/1] rxe driver should dynamically caclculate inline data size rao Shoaib @ 2019-10-23 17:32 ` rao Shoaib 2019-10-29 19:11 ` Jason Gunthorpe 0 siblings, 1 reply; 5+ messages in thread From: rao Shoaib @ 2019-10-23 17:32 UTC (permalink / raw) To: monis, dledford, sean.hefty, hal.rosenstock, linux-rdma, linux-kernel Cc: Rao Shoaib From: Rao Shoaib <rao.shoaib@oracle.com> rxe driver has a hard coded value for the size of inline data, where as mlx5 driver calculates number of SGE's and inline data size based on the values in the qp request. This patch modifies rxe driver to do the same so that applications can work seamlessly across drivers. Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com> --- drivers/infiniband/sw/rxe/rxe_param.h | 2 +- drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h index 1b596fb..657f9303 100644 --- a/drivers/infiniband/sw/rxe/rxe_param.h +++ b/drivers/infiniband/sw/rxe/rxe_param.h @@ -68,7 +68,6 @@ enum rxe_device_param { RXE_HW_VER = 0, RXE_MAX_QP = 0x10000, RXE_MAX_QP_WR = 0x4000, - RXE_MAX_INLINE_DATA = 400, RXE_DEVICE_CAP_FLAGS = IB_DEVICE_BAD_PKEY_CNTR | IB_DEVICE_BAD_QKEY_CNTR | IB_DEVICE_AUTO_PATH_MIG @@ -81,6 +80,7 @@ enum rxe_device_param { | IB_DEVICE_MEM_MGT_EXTENSIONS, RXE_MAX_SGE = 32, RXE_MAX_SGE_RD = 32, + RXE_MAX_INLINE_DATA = RXE_MAX_SGE * sizeof(struct ib_sge), RXE_MAX_CQ = 16384, RXE_MAX_LOG_CQE = 15, RXE_MAX_MR = 2 * 1024, diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c index aeea994..45b5da5 100644 --- a/drivers/infiniband/sw/rxe/rxe_qp.c +++ b/drivers/infiniband/sw/rxe/rxe_qp.c @@ -229,6 +229,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, { int err; int wqe_size; + unsigned int inline_size; err = sock_create_kern(&init_net, AF_INET, SOCK_DGRAM, 0, &qp->sk); if (err < 0) @@ -244,6 +245,9 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, sizeof(struct rxe_send_wqe) + qp->sq.max_inline); + inline_size = wqe_size - sizeof(struct rxe_send_wqe); + qp->sq.max_inline = inline_size; + init->cap.max_inline_data = inline_size; qp->sq.queue = rxe_queue_init(rxe, &qp->sq.max_wr, wqe_size); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] rxe: calculate inline data size based on requested values 2019-10-23 17:32 ` [PATCH v1 1/1] rxe: calculate inline data size based on requested values rao Shoaib @ 2019-10-29 19:11 ` Jason Gunthorpe 2019-10-29 19:31 ` Rao Shoaib 0 siblings, 1 reply; 5+ messages in thread From: Jason Gunthorpe @ 2019-10-29 19:11 UTC (permalink / raw) To: rao Shoaib Cc: monis, dledford, sean.hefty, hal.rosenstock, linux-rdma, linux-kernel On Wed, Oct 23, 2019 at 10:32:37AM -0700, rao Shoaib wrote: > From: Rao Shoaib <rao.shoaib@oracle.com> > > rxe driver has a hard coded value for the size of inline data, where as > mlx5 driver calculates number of SGE's and inline data size based on the > values in the qp request. This patch modifies rxe driver to do the same > so that applications can work seamlessly across drivers. This description doesn't seem accurate at all, and this patch seems to be doing two things: > Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com> > --- > drivers/infiniband/sw/rxe/rxe_param.h | 2 +- > drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h > index 1b596fb..657f9303 100644 > --- a/drivers/infiniband/sw/rxe/rxe_param.h > +++ b/drivers/infiniband/sw/rxe/rxe_param.h > @@ -68,7 +68,6 @@ enum rxe_device_param { > RXE_HW_VER = 0, > RXE_MAX_QP = 0x10000, > RXE_MAX_QP_WR = 0x4000, > - RXE_MAX_INLINE_DATA = 400, > RXE_DEVICE_CAP_FLAGS = IB_DEVICE_BAD_PKEY_CNTR > | IB_DEVICE_BAD_QKEY_CNTR > | IB_DEVICE_AUTO_PATH_MIG > @@ -81,6 +80,7 @@ enum rxe_device_param { > | IB_DEVICE_MEM_MGT_EXTENSIONS, > RXE_MAX_SGE = 32, > RXE_MAX_SGE_RD = 32, > + RXE_MAX_INLINE_DATA = RXE_MAX_SGE * sizeof(struct ib_sge), > RXE_MAX_CQ = 16384, > RXE_MAX_LOG_CQE = 15, > RXE_MAX_MR = 2 * 1024, Increasing RXE_MAX_INLINE_DATA to match the WQE size limited the MAX_SGE. IMHO this is done in a hacky way, instead we should define a maximim WQE size and from there derive the MAX_INLINE_DATA and MAX_SGE limitations. > diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c > index aeea994..45b5da5 100644 > --- a/drivers/infiniband/sw/rxe/rxe_qp.c > +++ b/drivers/infiniband/sw/rxe/rxe_qp.c > @@ -229,6 +229,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, > { > int err; > int wqe_size; > + unsigned int inline_size; > > err = sock_create_kern(&init_net, AF_INET, SOCK_DGRAM, 0, &qp->sk); > if (err < 0) > @@ -244,6 +245,9 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, > sizeof(struct rxe_send_wqe) + > qp->sq.max_inline); > > + inline_size = wqe_size - sizeof(struct rxe_send_wqe); > + qp->sq.max_inline = inline_size; > + init->cap.max_inline_data = inline_size; Whatever this is doing. Is this trying to expand the supported inline data when max_sge is provided? That seems reasonable but peculiar. Should be it's own patch. Also don't double initialize qp->sq.max_inline in the same function, and there is no need for the temporary 'inline_size' Jason > qp->sq.queue = rxe_queue_init(rxe, > &qp->sq.max_wr, > wqe_size); > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] rxe: calculate inline data size based on requested values 2019-10-29 19:11 ` Jason Gunthorpe @ 2019-10-29 19:31 ` Rao Shoaib 2019-10-29 19:44 ` Jason Gunthorpe 0 siblings, 1 reply; 5+ messages in thread From: Rao Shoaib @ 2019-10-29 19:31 UTC (permalink / raw) To: Jason Gunthorpe Cc: monis, dledford, sean.hefty, hal.rosenstock, linux-rdma, linux-kernel Hi Jason, Thanks for the comments, please see inline. On 10/29/19 12:11 PM, Jason Gunthorpe wrote: > On Wed, Oct 23, 2019 at 10:32:37AM -0700, rao Shoaib wrote: >> From: Rao Shoaib <rao.shoaib@oracle.com> >> >> rxe driver has a hard coded value for the size of inline data, where as >> mlx5 driver calculates number of SGE's and inline data size based on the >> values in the qp request. This patch modifies rxe driver to do the same >> so that applications can work seamlessly across drivers. > This description doesn't seem accurate at all, and this patch seems to > be doing two things: I thought the note described the change, I will try harder next time. > >> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com> >> --- >> drivers/infiniband/sw/rxe/rxe_param.h | 2 +- >> drivers/infiniband/sw/rxe/rxe_qp.c | 4 ++++ >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h >> index 1b596fb..657f9303 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_param.h >> +++ b/drivers/infiniband/sw/rxe/rxe_param.h >> @@ -68,7 +68,6 @@ enum rxe_device_param { >> RXE_HW_VER = 0, >> RXE_MAX_QP = 0x10000, >> RXE_MAX_QP_WR = 0x4000, >> - RXE_MAX_INLINE_DATA = 400, >> RXE_DEVICE_CAP_FLAGS = IB_DEVICE_BAD_PKEY_CNTR >> | IB_DEVICE_BAD_QKEY_CNTR >> | IB_DEVICE_AUTO_PATH_MIG >> @@ -81,6 +80,7 @@ enum rxe_device_param { >> | IB_DEVICE_MEM_MGT_EXTENSIONS, >> RXE_MAX_SGE = 32, >> RXE_MAX_SGE_RD = 32, >> + RXE_MAX_INLINE_DATA = RXE_MAX_SGE * sizeof(struct ib_sge), >> RXE_MAX_CQ = 16384, >> RXE_MAX_LOG_CQE = 15, >> RXE_MAX_MR = 2 * 1024, > Increasing RXE_MAX_INLINE_DATA to match the WQE size limited the > MAX_SGE. IMHO this is done in a hacky way, instead we should define a > maximim WQE size and from there derive the MAX_INLINE_DATA and MAX_SGE > limitations. There was already RXE_MAX_SGE defined so I did not define MAX_WQE. If that is what is preference I can submit a patch with that. What is a good value for MAX_WQE? > >> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c >> index aeea994..45b5da5 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_qp.c >> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c >> @@ -229,6 +229,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, >> { >> int err; >> int wqe_size; >> + unsigned int inline_size; >> >> err = sock_create_kern(&init_net, AF_INET, SOCK_DGRAM, 0, &qp->sk); >> if (err < 0) >> @@ -244,6 +245,9 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp, >> sizeof(struct rxe_send_wqe) + >> qp->sq.max_inline); >> >> + inline_size = wqe_size - sizeof(struct rxe_send_wqe); >> + qp->sq.max_inline = inline_size; >> + init->cap.max_inline_data = inline_size; > Whatever this is doing. Is this trying to expand the supported inline > data when max_sge is provided? That seems reasonable but > peculiar. Should be it's own patch. Yes that is what it is dong same as mlx5 which takes the larger of the two values reqquested and bumps the other. I will submit a separate patch. > > Also don't double initialize qp->sq.max_inline in the same function, > and there is no need for the temporary 'inline_size' I used a separate variable as I would have to repeat the calculation twice. I do not understand your comment about double initialization, can you please clarify that for me. Thanks, Shoaib > > Jason > > >> qp->sq.queue = rxe_queue_init(rxe, >> &qp->sq.max_wr, >> wqe_size); >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/1] rxe: calculate inline data size based on requested values 2019-10-29 19:31 ` Rao Shoaib @ 2019-10-29 19:44 ` Jason Gunthorpe 0 siblings, 0 replies; 5+ messages in thread From: Jason Gunthorpe @ 2019-10-29 19:44 UTC (permalink / raw) To: Rao Shoaib Cc: monis, dledford, sean.hefty, hal.rosenstock, linux-rdma, linux-kernel On Tue, Oct 29, 2019 at 12:31:03PM -0700, Rao Shoaib wrote: > > > @@ -81,6 +80,7 @@ enum rxe_device_param { > > > | IB_DEVICE_MEM_MGT_EXTENSIONS, > > > RXE_MAX_SGE = 32, > > > RXE_MAX_SGE_RD = 32, > > > + RXE_MAX_INLINE_DATA = RXE_MAX_SGE * sizeof(struct ib_sge), > > > RXE_MAX_CQ = 16384, > > > RXE_MAX_LOG_CQE = 15, > > > RXE_MAX_MR = 2 * 1024, > > Increasing RXE_MAX_INLINE_DATA to match the WQE size limited the > > MAX_SGE. IMHO this is done in a hacky way, instead we should define a > > maximim WQE size and from there derive the MAX_INLINE_DATA and MAX_SGE > > limitations. > There was already RXE_MAX_SGE defined so I did not define MAX_WQE. If that > is what is preference I can submit a patch with that. What is a good value > for MAX_WQE? I would arrange it so that RXE_MAX_SGE doesn't change > > Also don't double initialize qp->sq.max_inline in the same function, > > and there is no need for the temporary 'inline_size' > > I used a separate variable as I would have to repeat the calculation twice. > I do not understand your comment about double initialization, can you please > clarify that for me. Assign it to qp->sq.max_inline and then read it to get the init Look above in the function, there is already an assignment to qp->sq.max_inline Jason ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-29 19:44 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-23 17:32 [PATCH v1 0/1] rxe driver should dynamically caclculate inline data size rao Shoaib 2019-10-23 17:32 ` [PATCH v1 1/1] rxe: calculate inline data size based on requested values rao Shoaib 2019-10-29 19:11 ` Jason Gunthorpe 2019-10-29 19:31 ` Rao Shoaib 2019-10-29 19:44 ` Jason Gunthorpe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.