2008-05-21 00:46:09

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 02/05] svcrdma: Refactor RDMA_WRITE dma mapping logic

On Sun, May 18, 2008 at 07:13:18PM -0500, Tom Tucker wrote:
> In order to make the dma mapping code easier to understand and reduce=
the
> number of I/O contexts required, move the DMA for RDMA_WRITE WR to th=
e
> code that prepares the WR SGE.

This one makes my (32-bit) compiler whine:

CC net/sunrpc/xprtrdma/svc_rdma_sendto.o
net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =E2=80=98send_write=E2=
=80=99:
net/sunrpc/xprtrdma/svc_rdma_sendto.c:180: warning: passing argument 2 =
of =E2=80=98ib_dma_map_single=E2=80=99 makes pointer from integer witho=
ut a cast
net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =E2=80=98send_reply=E2=
=80=99:
net/sunrpc/xprtrdma/svc_rdma_sendto.c:416: warning: passing argument 2 =
of =E2=80=98ib_dma_map_single=E2=80=99 makes pointer from integer witho=
ut a cast

And the types do seem weird:

>=20
> Signed-off-by: Tom Tucker <[email protected]>
>=20
> ---
> net/sunrpc/xprtrdma/svc_rdma_sendto.c | 44 +++++++++++++++-----=
----------
> net/sunrpc/xprtrdma/svc_rdma_transport.c | 5 +++-
> 2 files changed, 26 insertions(+), 23 deletions(-)
>=20
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtr=
dma/svc_rdma_sendto.c
> index fb82b1b..85931c4 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> @@ -84,10 +84,7 @@ static struct ib_sge *xdr_to_sge(struct svcxprt_rd=
ma *xprt,
> sge_no =3D 1;
> =20
> /* Head SGE */
> - sge[sge_no].addr =3D ib_dma_map_single(xprt->sc_cm_id->device,
> - xdr->head[0].iov_base,
> - xdr->head[0].iov_len,
> - DMA_TO_DEVICE);
> + sge[sge_no].addr =3D (unsigned long)xdr->head[0].iov_base;

So before we called a function that took a void *, returned a u64; now
we're storing a void* directly into a u64.

> sge_bytes =3D min_t(u32, byte_count, xdr->head[0].iov_len);
> byte_count -=3D sge_bytes;
> sge[sge_no].length =3D sge_bytes;
> @@ -99,11 +96,9 @@ static struct ib_sge *xdr_to_sge(struct svcxprt_rd=
ma *xprt,
> page_bytes =3D xdr->page_len;
> page_off =3D xdr->page_base;
> while (byte_count && page_bytes) {
> + sge[sge_no].addr =3D (unsigned long)
> + page_address(xdr->pages[page_no]) + page_off;
> sge_bytes =3D min_t(u32, byte_count, (PAGE_SIZE-page_off));
> - sge[sge_no].addr =3D
> - ib_dma_map_page(xprt->sc_cm_id->device,
> - xdr->pages[page_no], page_off,
> - sge_bytes, DMA_TO_DEVICE);
> sge_bytes =3D min(sge_bytes, page_bytes);
> byte_count -=3D sge_bytes;
> page_bytes -=3D sge_bytes;
> @@ -117,11 +112,7 @@ static struct ib_sge *xdr_to_sge(struct svcxprt_=
rdma *xprt,
> =20
> /* Tail SGE */
> if (byte_count && xdr->tail[0].iov_len) {
> - sge[sge_no].addr =3D
> - ib_dma_map_single(xprt->sc_cm_id->device,
> - xdr->tail[0].iov_base,
> - xdr->tail[0].iov_len,
> - DMA_TO_DEVICE);
> + sge[sge_no].addr =3D (unsigned long)xdr->tail[0].iov_base;
> sge_bytes =3D min_t(u32, byte_count, xdr->tail[0].iov_len);
> byte_count -=3D sge_bytes;
> sge[sge_no].length =3D sge_bytes;
> @@ -145,7 +136,6 @@ static int send_write(struct svcxprt_rdma *xprt, =
struct svc_rqst *rqstp,
> u32 xdr_off, int write_len,
> struct ib_sge *xdr_sge, int sge_count)
> {
> - struct svc_rdma_op_ctxt *tmp_sge_ctxt;
> struct ib_send_wr write_wr;
> struct ib_sge *sge;
> int xdr_sge_no;
> @@ -163,9 +153,8 @@ static int send_write(struct svcxprt_rdma *xprt, =
struct svc_rqst *rqstp,
> write_len, xdr_sge, sge_count);
> =20
> ctxt =3D svc_rdma_get_context(xprt);
> - ctxt->count =3D 0;
> - tmp_sge_ctxt =3D svc_rdma_get_context(xprt);
> - sge =3D tmp_sge_ctxt->sge;
> + ctxt->direction =3D DMA_TO_DEVICE;
> + sge =3D ctxt->sge;
> =20
> /* Find the SGE associated with xdr_off */
> for (bc =3D xdr_off, xdr_sge_no =3D 1; bc && xdr_sge_no < sge_count=
;
> @@ -181,14 +170,20 @@ static int send_write(struct svcxprt_rdma *xprt=
, struct svc_rqst *rqstp,
> =20
> /* Copy the remaining SGE */
> while (bc !=3D 0 && xdr_sge_no < sge_count) {
> - sge[sge_no].addr =3D xdr_sge[xdr_sge_no].addr + sge_off;
> sge[sge_no].lkey =3D xdr_sge[xdr_sge_no].lkey;
> sge_bytes =3D min((size_t)bc,
> (size_t)(xdr_sge[xdr_sge_no].length-sge_off));
> sge[sge_no].length =3D sge_bytes;
> -
> + sge[sge_no].addr =3D
> + ib_dma_map_single(xprt->sc_cm_id->device,
> + (void *)
> + xdr_sge[xdr_sge_no].addr + sge_off,
> + sge_bytes, DMA_TO_DEVICE);
> + if (dma_mapping_error(sge[sge_no].addr))
> + return -EINVAL;

And then here we're casting the u64 back to a void *. Also, we're
adding sge_off to the input, instead of to the result. Is it true that
that

ib_dma_map_single(., x + sge_off, ., .)

and

ib_dma_map_single(., x, ., .) + sge_off

always have the same result?

--b.

> sge_off =3D 0;
> sge_no++;
> + ctxt->count++;
> xdr_sge_no++;
> bc -=3D sge_bytes;
> }
> @@ -210,11 +205,10 @@ static int send_write(struct svcxprt_rdma *xprt=
, struct svc_rqst *rqstp,
> /* Post It */
> atomic_inc(&rdma_stat_write);
> if (svc_rdma_send(xprt, &write_wr)) {
> - svc_rdma_put_context(ctxt, 1);
> + svc_rdma_put_context(ctxt, 0);
> /* Fatal error, close transport */
> ret =3D -EIO;
> }
> - svc_rdma_put_context(tmp_sge_ctxt, 0);
> return ret;
> }
> =20
> @@ -417,6 +411,11 @@ static int send_reply(struct svcxprt_rdma *rdma,
> sge_bytes =3D min((size_t)ctxt->sge[sge_no].length,
> (size_t)byte_count);
> byte_count -=3D sge_bytes;
> + ctxt->sge[sge_no].addr =3D
> + ib_dma_map_single(rdma->sc_cm_id->device,
> + (void *)
> + ctxt->sge[sge_no].addr,
> + sge_bytes, DMA_TO_DEVICE);
> }
> BUG_ON(byte_count !=3D 0);
> =20
> @@ -428,8 +427,9 @@ static int send_reply(struct svcxprt_rdma *rdma,
> ctxt->pages[page_no+1] =3D rqstp->rq_respages[page_no];
> ctxt->count++;
> rqstp->rq_respages[page_no] =3D NULL;
> + if (page_no+1 >=3D sge_no)
> + ctxt->sge[page_no+1].length =3D 0;
> }
> -
> BUG_ON(sge_no > rdma->sc_max_sge);
> memset(&send_wr, 0, sizeof send_wr);
> ctxt->wr_op =3D IB_WR_SEND;
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xp=
rtrdma/svc_rdma_transport.c
> index e132509..9e9e244 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> @@ -361,10 +361,13 @@ static void sq_cq_reap(struct svcxprt_rdma *xpr=
t)
> =20
> switch (ctxt->wr_op) {
> case IB_WR_SEND:
> - case IB_WR_RDMA_WRITE:
> svc_rdma_put_context(ctxt, 1);
> break;
> =20
> + case IB_WR_RDMA_WRITE:
> + svc_rdma_put_context(ctxt, 0);
> + break;
> +
> case IB_WR_RDMA_READ:
> if (test_bit(RDMACTXT_F_LAST_CTXT, &ctxt->flags)) {
> struct svc_rdma_op_ctxt *read_hdr =3D ctxt->read_hdr;


2008-05-25 19:05:37

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 02/05] svcrdma: Refactor RDMA_WRITE dma mapping logic

I'm also still curious about this:

On Tue, May 20, 2008 at 08:46:08PM -0400, bfields wrote:
> On Sun, May 18, 2008 at 07:13:18PM -0500, Tom Tucker wrote:
> > @@ -181,14 +170,20 @@ static int send_write(struct svcxprt_rdma *xprt, struct svc_rqst *rqstp,
> >
> > /* Copy the remaining SGE */
> > while (bc != 0 && xdr_sge_no < sge_count) {
> > - sge[sge_no].addr = xdr_sge[xdr_sge_no].addr + sge_off;
> > sge[sge_no].lkey = xdr_sge[xdr_sge_no].lkey;
> > sge_bytes = min((size_t)bc,
> > (size_t)(xdr_sge[xdr_sge_no].length-sge_off));
> > sge[sge_no].length = sge_bytes;
> > -
> > + sge[sge_no].addr =
> > + ib_dma_map_single(xprt->sc_cm_id->device,
> > + (void *)
> > + xdr_sge[xdr_sge_no].addr + sge_off,
> > + sge_bytes, DMA_TO_DEVICE);
> > + if (dma_mapping_error(sge[sge_no].addr))
> > + return -EINVAL;
>
> And then here we're casting the u64 back to a void *. Also, we're
> adding sge_off to the input, instead of to the result. Is it true that
> that
>
> ib_dma_map_single(., x + sge_off, ., .)
>
> and
>
> ib_dma_map_single(., x, ., .) + sge_off
>
> always have the same result?
>
> --b.

2008-05-21 02:52:18

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 02/05] svcrdma: Refactor RDMA_WRITE dma mapping logic

On Tue, May 20, 2008 at 08:46:08PM -0400, J. Bruce Fields wrote:
> On Sun, May 18, 2008 at 07:13:18PM -0500, Tom Tucker wrote:
> > In order to make the dma mapping code easier to understand and redu=
ce the
> > number of I/O contexts required, move the DMA for RDMA_WRITE WR to =
the
> > code that prepares the WR SGE.
>=20
> This one makes my (32-bit) compiler whine:
>=20
> CC net/sunrpc/xprtrdma/svc_rdma_sendto.o
> net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =E2=80=98send_writ=
e=E2=80=99:
> net/sunrpc/xprtrdma/svc_rdma_sendto.c:180: warning: passing argument =
2 of =E2=80=98ib_dma_map_single=E2=80=99 makes pointer from integer wit=
hout a cast
> net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =E2=80=98send_repl=
y=E2=80=99:
> net/sunrpc/xprtrdma/svc_rdma_sendto.c:416: warning: passing argument =
2 of =E2=80=98ib_dma_map_single=E2=80=99 makes pointer from integer wit=
hout a cast

Erm, sorry, that should have been:

unrpc/xprtrdma/svc_rdma_sendto.c: In function =E2=80=98send_write=E2=80=
=99:
net/sunrpc/xprtrdma/svc_rdma_sendto.c:180: warning: cast to pointer fro=
m integer of different size
net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =E2=80=98send_reply=E2=
=80=99:
net/sunrpc/xprtrdma/svc_rdma_sendto.c:417: warning: cast to pointer fro=
m integer of different size

--b.

> And the types do seem weird:
>=20
> >=20
> > Signed-off-by: Tom Tucker <[email protected]>
> >=20
> > ---
> > net/sunrpc/xprtrdma/svc_rdma_sendto.c | 44 +++++++++++++++---=
------------
> > net/sunrpc/xprtrdma/svc_rdma_transport.c | 5 +++-
> > 2 files changed, 26 insertions(+), 23 deletions(-)
> >=20
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xpr=
trdma/svc_rdma_sendto.c
> > index fb82b1b..85931c4 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
> > @@ -84,10 +84,7 @@ static struct ib_sge *xdr_to_sge(struct svcxprt_=
rdma *xprt,
> > sge_no =3D 1;
> > =20
> > /* Head SGE */
> > - sge[sge_no].addr =3D ib_dma_map_single(xprt->sc_cm_id->device,
> > - xdr->head[0].iov_base,
> > - xdr->head[0].iov_len,
> > - DMA_TO_DEVICE);
> > + sge[sge_no].addr =3D (unsigned long)xdr->head[0].iov_base;
>=20
> So before we called a function that took a void *, returned a u64; no=
w
> we're storing a void* directly into a u64.
>=20
> > sge_bytes =3D min_t(u32, byte_count, xdr->head[0].iov_len);
> > byte_count -=3D sge_bytes;
> > sge[sge_no].length =3D sge_bytes;
> > @@ -99,11 +96,9 @@ static struct ib_sge *xdr_to_sge(struct svcxprt_=
rdma *xprt,
> > page_bytes =3D xdr->page_len;
> > page_off =3D xdr->page_base;
> > while (byte_count && page_bytes) {
> > + sge[sge_no].addr =3D (unsigned long)
> > + page_address(xdr->pages[page_no]) + page_off;
> > sge_bytes =3D min_t(u32, byte_count, (PAGE_SIZE-page_off));
> > - sge[sge_no].addr =3D
> > - ib_dma_map_page(xprt->sc_cm_id->device,
> > - xdr->pages[page_no], page_off,
> > - sge_bytes, DMA_TO_DEVICE);
> > sge_bytes =3D min(sge_bytes, page_bytes);
> > byte_count -=3D sge_bytes;
> > page_bytes -=3D sge_bytes;
> > @@ -117,11 +112,7 @@ static struct ib_sge *xdr_to_sge(struct svcxpr=
t_rdma *xprt,
> > =20
> > /* Tail SGE */
> > if (byte_count && xdr->tail[0].iov_len) {
> > - sge[sge_no].addr =3D
> > - ib_dma_map_single(xprt->sc_cm_id->device,
> > - xdr->tail[0].iov_base,
> > - xdr->tail[0].iov_len,
> > - DMA_TO_DEVICE);
> > + sge[sge_no].addr =3D (unsigned long)xdr->tail[0].iov_base;
> > sge_bytes =3D min_t(u32, byte_count, xdr->tail[0].iov_len);
> > byte_count -=3D sge_bytes;
> > sge[sge_no].length =3D sge_bytes;
> > @@ -145,7 +136,6 @@ static int send_write(struct svcxprt_rdma *xprt=
, struct svc_rqst *rqstp,
> > u32 xdr_off, int write_len,
> > struct ib_sge *xdr_sge, int sge_count)
> > {
> > - struct svc_rdma_op_ctxt *tmp_sge_ctxt;
> > struct ib_send_wr write_wr;
> > struct ib_sge *sge;
> > int xdr_sge_no;
> > @@ -163,9 +153,8 @@ static int send_write(struct svcxprt_rdma *xprt=
, struct svc_rqst *rqstp,
> > write_len, xdr_sge, sge_count);
> > =20
> > ctxt =3D svc_rdma_get_context(xprt);
> > - ctxt->count =3D 0;
> > - tmp_sge_ctxt =3D svc_rdma_get_context(xprt);
> > - sge =3D tmp_sge_ctxt->sge;
> > + ctxt->direction =3D DMA_TO_DEVICE;
> > + sge =3D ctxt->sge;
> > =20
> > /* Find the SGE associated with xdr_off */
> > for (bc =3D xdr_off, xdr_sge_no =3D 1; bc && xdr_sge_no < sge_cou=
nt;
> > @@ -181,14 +170,20 @@ static int send_write(struct svcxprt_rdma *xp=
rt, struct svc_rqst *rqstp,
> > =20
> > /* Copy the remaining SGE */
> > while (bc !=3D 0 && xdr_sge_no < sge_count) {
> > - sge[sge_no].addr =3D xdr_sge[xdr_sge_no].addr + sge_off;
> > sge[sge_no].lkey =3D xdr_sge[xdr_sge_no].lkey;
> > sge_bytes =3D min((size_t)bc,
> > (size_t)(xdr_sge[xdr_sge_no].length-sge_off));
> > sge[sge_no].length =3D sge_bytes;
> > -
> > + sge[sge_no].addr =3D
> > + ib_dma_map_single(xprt->sc_cm_id->device,
> > + (void *)
> > + xdr_sge[xdr_sge_no].addr + sge_off,
> > + sge_bytes, DMA_TO_DEVICE);
> > + if (dma_mapping_error(sge[sge_no].addr))
> > + return -EINVAL;
>=20
> And then here we're casting the u64 back to a void *. Also, we're
> adding sge_off to the input, instead of to the result. Is it true th=
at
> that
>=20
> ib_dma_map_single(., x + sge_off, ., .)
>=20
> and
>=20
> ib_dma_map_single(., x, ., .) + sge_off
>=20
> always have the same result?
>=20
> --b.
>=20
> > sge_off =3D 0;
> > sge_no++;
> > + ctxt->count++;
> > xdr_sge_no++;
> > bc -=3D sge_bytes;
> > }
> > @@ -210,11 +205,10 @@ static int send_write(struct svcxprt_rdma *xp=
rt, struct svc_rqst *rqstp,
> > /* Post It */
> > atomic_inc(&rdma_stat_write);
> > if (svc_rdma_send(xprt, &write_wr)) {
> > - svc_rdma_put_context(ctxt, 1);
> > + svc_rdma_put_context(ctxt, 0);
> > /* Fatal error, close transport */
> > ret =3D -EIO;
> > }
> > - svc_rdma_put_context(tmp_sge_ctxt, 0);
> > return ret;
> > }
> > =20
> > @@ -417,6 +411,11 @@ static int send_reply(struct svcxprt_rdma *rdm=
a,
> > sge_bytes =3D min((size_t)ctxt->sge[sge_no].length,
> > (size_t)byte_count);
> > byte_count -=3D sge_bytes;
> > + ctxt->sge[sge_no].addr =3D
> > + ib_dma_map_single(rdma->sc_cm_id->device,
> > + (void *)
> > + ctxt->sge[sge_no].addr,
> > + sge_bytes, DMA_TO_DEVICE);
> > }
> > BUG_ON(byte_count !=3D 0);
> > =20
> > @@ -428,8 +427,9 @@ static int send_reply(struct svcxprt_rdma *rdma=
,
> > ctxt->pages[page_no+1] =3D rqstp->rq_respages[page_no];
> > ctxt->count++;
> > rqstp->rq_respages[page_no] =3D NULL;
> > + if (page_no+1 >=3D sge_no)
> > + ctxt->sge[page_no+1].length =3D 0;
> > }
> > -
> > BUG_ON(sge_no > rdma->sc_max_sge);
> > memset(&send_wr, 0, sizeof send_wr);
> > ctxt->wr_op =3D IB_WR_SEND;
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/=
xprtrdma/svc_rdma_transport.c
> > index e132509..9e9e244 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > @@ -361,10 +361,13 @@ static void sq_cq_reap(struct svcxprt_rdma *x=
prt)
> > =20
> > switch (ctxt->wr_op) {
> > case IB_WR_SEND:
> > - case IB_WR_RDMA_WRITE:
> > svc_rdma_put_context(ctxt, 1);
> > break;
> > =20
> > + case IB_WR_RDMA_WRITE:
> > + svc_rdma_put_context(ctxt, 0);
> > + break;
> > +
> > case IB_WR_RDMA_READ:
> > if (test_bit(RDMACTXT_F_LAST_CTXT, &ctxt->flags)) {
> > struct svc_rdma_op_ctxt *read_hdr =3D ctxt->read_hdr;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2008-05-21 10:33:41

by Tom Tucker

[permalink] [raw]
Subject: Re: [PATCH 02/05] svcrdma: Refactor RDMA_WRITE dma mapping logic




On 5/20/08 9:52 PM, "J. Bruce Fields" <[email protected]> wrote:

> On Tue, May 20, 2008 at 08:46:08PM -0400, J. Bruce Fields wrote:
>> On Sun, May 18, 2008 at 07:13:18PM -0500, Tom Tucker wrote:
>>> In order to make the dma mapping code easier to understand and redu=
ce the
>>> number of I/O contexts required, move the DMA for RDMA_WRITE WR to =
the
>>> code that prepares the WR SGE.
>>=20
>> This one makes my (32-bit) compiler whine:
>>=20
>> CC net/sunrpc/xprtrdma/svc_rdma_sendto.o
>> net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =8Csend_write=B9:
>> net/sunrpc/xprtrdma/svc_rdma_sendto.c:180: warning: passing argument=
2 of
>> =8Cib_dma_map_single=B9 makes pointer from integer without a cast
>> net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =8Csend_reply=B9:
>> net/sunrpc/xprtrdma/svc_rdma_sendto.c:416: warning: passing argument=
2 of
>> =8Cib_dma_map_single=B9 makes pointer from integer without a cast
>=20
> Erm, sorry, that should have been:
>=20
> unrpc/xprtrdma/svc_rdma_sendto.c: In function =8Csend_write=B9:
> net/sunrpc/xprtrdma/svc_rdma_sendto.c:180: warning: cast to pointer f=
rom
> integer of different size
> net/sunrpc/xprtrdma/svc_rdma_sendto.c: In function =8Csend_reply=B9:
> net/sunrpc/xprtrdma/svc_rdma_sendto.c:417: warning: cast to pointer f=
rom
> integer of different size
>

I have to jump on a plane, but I'll take a look tonight. Basically, the
ib_sge is sometimes being used to save pointers and other times dma_add=
r_t.
I need my own type there I guess.

Tom
=20
> --b.
>=20
>> And the types do seem weird:
>>=20
>>>=20
>>> Signed-off-by: Tom Tucker <[email protected]>
>>>=20
>>> ---
>>> net/sunrpc/xprtrdma/svc_rdma_sendto.c | 44
>>> +++++++++++++++---------------
>>> net/sunrpc/xprtrdma/svc_rdma_transport.c | 5 +++-
>>> 2 files changed, 26 insertions(+), 23 deletions(-)
>>>=20
>>> diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
>>> b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
>>> index fb82b1b..85931c4 100644
>>> --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
>>> +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
>>> @@ -84,10 +84,7 @@ static struct ib_sge *xdr_to_sge(struct svcxprt_=
rdma
>>> *xprt,
>>> sge_no =3D 1;
>>> =20
>>> /* Head SGE */
>>> - sge[sge_no].addr =3D ib_dma_map_single(xprt->sc_cm_id->device,
>>> - xdr->head[0].iov_base,
>>> - xdr->head[0].iov_len,
>>> - DMA_TO_DEVICE);
>>> + sge[sge_no].addr =3D (unsigned long)xdr->head[0].iov_base;
>>=20
>> So before we called a function that took a void *, returned a u64; n=
ow
>> we're storing a void* directly into a u64.
>>=20
>>> sge_bytes =3D min_t(u32, byte_count, xdr->head[0].iov_len);
>>> byte_count -=3D sge_bytes;
>>> sge[sge_no].length =3D sge_bytes;
>>> @@ -99,11 +96,9 @@ static struct ib_sge *xdr_to_sge(struct svcxprt_=
rdma
>>> *xprt,
>>> page_bytes =3D xdr->page_len;
>>> page_off =3D xdr->page_base;
>>> while (byte_count && page_bytes) {
>>> + sge[sge_no].addr =3D (unsigned long)
>>> + page_address(xdr->pages[page_no]) + page_off;
>>> sge_bytes =3D min_t(u32, byte_count, (PAGE_SIZE-page_off));
>>> - sge[sge_no].addr =3D
>>> - ib_dma_map_page(xprt->sc_cm_id->device,
>>> - xdr->pages[page_no], page_off,
>>> - sge_bytes, DMA_TO_DEVICE);
>>> sge_bytes =3D min(sge_bytes, page_bytes);
>>> byte_count -=3D sge_bytes;
>>> page_bytes -=3D sge_bytes;
>>> @@ -117,11 +112,7 @@ static struct ib_sge *xdr_to_sge(struct svcxpr=
t_rdma
>>> *xprt,
>>> =20
>>> /* Tail SGE */
>>> if (byte_count && xdr->tail[0].iov_len) {
>>> - sge[sge_no].addr =3D
>>> - ib_dma_map_single(xprt->sc_cm_id->device,
>>> - xdr->tail[0].iov_base,
>>> - xdr->tail[0].iov_len,
>>> - DMA_TO_DEVICE);
>>> + sge[sge_no].addr =3D (unsigned long)xdr->tail[0].iov_base;
>>> sge_bytes =3D min_t(u32, byte_count, xdr->tail[0].iov_len);
>>> byte_count -=3D sge_bytes;
>>> sge[sge_no].length =3D sge_bytes;
>>> @@ -145,7 +136,6 @@ static int send_write(struct svcxprt_rdma *xprt=
, struct
>>> svc_rqst *rqstp,
>>> u32 xdr_off, int write_len,
>>> struct ib_sge *xdr_sge, int sge_count)
>>> {
>>> - struct svc_rdma_op_ctxt *tmp_sge_ctxt;
>>> struct ib_send_wr write_wr;
>>> struct ib_sge *sge;
>>> int xdr_sge_no;
>>> @@ -163,9 +153,8 @@ static int send_write(struct svcxprt_rdma *xprt=
, struct
>>> svc_rqst *rqstp,
>>> write_len, xdr_sge, sge_count);
>>> =20
>>> ctxt =3D svc_rdma_get_context(xprt);
>>> - ctxt->count =3D 0;
>>> - tmp_sge_ctxt =3D svc_rdma_get_context(xprt);
>>> - sge =3D tmp_sge_ctxt->sge;
>>> + ctxt->direction =3D DMA_TO_DEVICE;
>>> + sge =3D ctxt->sge;
>>> =20
>>> /* Find the SGE associated with xdr_off */
>>> for (bc =3D xdr_off, xdr_sge_no =3D 1; bc && xdr_sge_no < sge_count=
;
>>> @@ -181,14 +170,20 @@ static int send_write(struct svcxprt_rdma *xp=
rt,
>>> struct svc_rqst *rqstp,
>>> =20
>>> /* Copy the remaining SGE */
>>> while (bc !=3D 0 && xdr_sge_no < sge_count) {
>>> - sge[sge_no].addr =3D xdr_sge[xdr_sge_no].addr + sge_off;
>>> sge[sge_no].lkey =3D xdr_sge[xdr_sge_no].lkey;
>>> sge_bytes =3D min((size_t)bc,
>>> (size_t)(xdr_sge[xdr_sge_no].length-sge_off));
>>> sge[sge_no].length =3D sge_bytes;
>>> -
>>> + sge[sge_no].addr =3D
>>> + ib_dma_map_single(xprt->sc_cm_id->device,
>>> + (void *)
>>> + xdr_sge[xdr_sge_no].addr + sge_off,
>>> + sge_bytes, DMA_TO_DEVICE);
>>> + if (dma_mapping_error(sge[sge_no].addr))
>>> + return -EINVAL;
>>=20
>> And then here we're casting the u64 back to a void *. Also, we're
>> adding sge_off to the input, instead of to the result. Is it true t=
hat
>> that
>>=20
>> ib_dma_map_single(., x + sge_off, ., .)
>>=20
>> and
>>=20
>> ib_dma_map_single(., x, ., .) + sge_off
>>=20
>> always have the same result?
>>=20
>> --b.
>>=20
>>> sge_off =3D 0;
>>> sge_no++;
>>> + ctxt->count++;
>>> xdr_sge_no++;
>>> bc -=3D sge_bytes;
>>> }
>>> @@ -210,11 +205,10 @@ static int send_write(struct svcxprt_rdma *xp=
rt,
>>> struct svc_rqst *rqstp,
>>> /* Post It */
>>> atomic_inc(&rdma_stat_write);
>>> if (svc_rdma_send(xprt, &write_wr)) {
>>> - svc_rdma_put_context(ctxt, 1);
>>> + svc_rdma_put_context(ctxt, 0);
>>> /* Fatal error, close transport */
>>> ret =3D -EIO;
>>> }
>>> - svc_rdma_put_context(tmp_sge_ctxt, 0);
>>> return ret;
>>> }
>>> =20
>>> @@ -417,6 +411,11 @@ static int send_reply(struct svcxprt_rdma *rdm=
a,
>>> sge_bytes =3D min((size_t)ctxt->sge[sge_no].length,
>>> (size_t)byte_count);
>>> byte_count -=3D sge_bytes;
>>> + ctxt->sge[sge_no].addr =3D
>>> + ib_dma_map_single(rdma->sc_cm_id->device,
>>> + (void *)
>>> + ctxt->sge[sge_no].addr,
>>> + sge_bytes, DMA_TO_DEVICE);
>>> }
>>> BUG_ON(byte_count !=3D 0);
>>> =20
>>> @@ -428,8 +427,9 @@ static int send_reply(struct svcxprt_rdma *rdma=
,
>>> ctxt->pages[page_no+1] =3D rqstp->rq_respages[page_no];
>>> ctxt->count++;
>>> rqstp->rq_respages[page_no] =3D NULL;
>>> + if (page_no+1 >=3D sge_no)
>>> + ctxt->sge[page_no+1].length =3D 0;
>>> }
>>> -
>>> BUG_ON(sge_no > rdma->sc_max_sge);
>>> memset(&send_wr, 0, sizeof send_wr);
>>> ctxt->wr_op =3D IB_WR_SEND;
>>> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c
>>> b/net/sunrpc/xprtrdma/svc_rdma_transport.c
>>> index e132509..9e9e244 100644
>>> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
>>> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
>>> @@ -361,10 +361,13 @@ static void sq_cq_reap(struct svcxprt_rdma *x=
prt)
>>> =20
>>> switch (ctxt->wr_op) {
>>> case IB_WR_SEND:
>>> - case IB_WR_RDMA_WRITE:
>>> svc_rdma_put_context(ctxt, 1);
>>> break;
>>> =20
>>> + case IB_WR_RDMA_WRITE:
>>> + svc_rdma_put_context(ctxt, 0);
>>> + break;
>>> +
>>> case IB_WR_RDMA_READ:
>>> if (test_bit(RDMACTXT_F_LAST_CTXT, &ctxt->flags)) {
>>> struct svc_rdma_op_ctxt *read_hdr =3D ctxt->read_hdr;
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs"=
in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html