> On Wed, 2014-10-22 at 12:02 +0300, Or Gerlitz wrote:
> > On 10/22/2014 8:06 AM, Nicholas A. Bellinger wrote:
> > > On Tue, 2014-10-21 at 00:13 +0300, Or Gerlitz wrote:
> > >> On Mon, Oct 20, 2014 at 6:29 PM, Chris Moore
> <Chris.Moore@xxxxxxxxxx> wrote:
> > >>> The following code is in isert_conn_setup_qp() in ib_isert.c:
> > >>>
> > >>> /*
> > >>> * FIXME: Use devattr.max_sge - 2 for max_send_sge as
> > >>> * work-around for RDMA_READ..
> > >>> */
> > >>> attr.cap.max_send_sge = device->dev_attr.max_sge - 2;
> > >>>
> > >>> It's not clear from the comment what this is a work-around for,
> > >>> and I wasn't able to figure it out from looking at logs.
> > >> I believe this refers to some IBTA spec corner case which comes
> > >> into play with the max_sges advertized by mlx4, Eli, can you shed
> > >> some light (IBTA pointer) on that? is this the case (i.e
> > >> dev_attr.max_sge isn't always achievable) with mlx5 too?
> > >>
> > > It's for ConnectX-2 reporting max_sge=31 during development, while
> > > the largest max_send_sge for stable large block RDMA reads was
> > > (is..?) 29 SGEs.
> > >
> > >>> In trying to get isert working with ocrdma I ran into a problem
> > >>> where the code thought there was only 1 send SGE available when in
> fact the device has 3.
> > >>> Then I found the above work-around, which explained the problem.
> > >> Nic, any reason for attr.cap.max_send_sge = 1 not to work OK?
> > >>
> > > IIRC, pre fastreg code supported max_send_sge=1 by limiting the
> > > transfer size per RDMA read, and would issue subsequent RDMA read +
> > > offset from completion up to the total se_cmd->data_length.
> > >
> > > Not sure how this works with fastreg today. Sagi..?
> > >
> >
> > While on fastreg mode, for RDMA reads we use only one SGE, talking to
> > Sagi he explained me that the problem with creating the QP with
> > max_send_sge = 1 has to do with other flows where two or even three
> > SGEs are needed, e.g when the iscsi/iser header (doesn't exist in RDMA
> > ops) is taken from one spot of the memory and the data (sense) taken
> > from another one?
> >
> > Nic, we need to see what is the minimum needed by the code (two?) and
> > tweak that line to make sure it gets there as long as the device
> > supports it, SB, makes sense?
> >
> >
> > diff --git a/drivers/infiniband/ulp/isert/ib_isert.c
> > b/drivers/infiniband/ulp/isert/ib_isert.c
> > index 0bea577..24abbb3 100644
> > --- a/drivers/infiniband/ulp/isert/ib_isert.c
> > +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> > @@ -115,9 +115,10 @@ isert_conn_setup_qp(struct isert_conn
> > *isert_conn, struct rdma_cm_id *cma_id,
> > attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS;
> > /*
> > * FIXME: Use devattr.max_sge - 2 for max_send_sge as
> > - * work-around for RDMA_READ..
> > + * work-around for RDMA_READ.. still need to make sure to
> > + * have at least two SGEs for SCSI responses.
> > */
> > - attr.cap.max_send_sge = device->dev_attr.max_sge - 2;
> > + attr.cap.max_send_sge = max(2, device->dev_attr.max_sge - 2);
> > isert_conn->max_sge = attr.cap.max_send_sge;
> >
> > attr.cap.max_recv_sge = 1;
> >
> >
>
> After a quick audit, the minimum max_send_sge=2 patch looks correct for
> the hardcoded tx_desc->num_sge cases in isert_put_login_tx(),
> isert_put_response(), isert_put_reject() and isert_put_text_rsp().
>
> For the RDMA read path with non fast-reg, isert_build_rdma_wr() is still
> correctly limiting the SGEs per operation based upon the negotiated
> isert_conn->max_sge set above in isert_conn_setup_qp().
>
> Chris, please confirm on ocrdma with Or's patch, and I'll include his patch into
> target-pending/queue for now, and move into master once you give a
> proper Tested-By.
>
> Thanks!
>
> --nab
Sorry for the long delay, I was dealing with some hardware issues.
I've tested the proposed patch and it fixes the ocrdma issue.
Tested-by: Chris Moore <chris.moore@xxxxxxxxxx>
Thanks,
Chris
��.n��������+%������w��{.n����j� ����{ay� ʇڙ���f���h������_�(�階�ݢj"��������G����?���&��
