* [PATCH RESEND] svcrdma: handle rdma read with a non-zero initial page offset
@ 2015-09-28 21:46 Steve Wise
2015-10-06 17:44 ` Doug Ledford
0 siblings, 1 reply; 5+ messages in thread
From: Steve Wise @ 2015-09-28 21:46 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs, linux-rdma
The server rdma_read_chunk_lcl() and rdma_read_chunk_frmr() functions
were not taking into account the initial page_offset when determining
the rdma read length. This resulted in a read who's starting address
and length exceeded the base/bounds of the frmr.
Most work loads don't tickle this bug apparently, but one test hit it
every time: building the linux kernel on a 16 core node with 'make -j
16 O=/mnt/0' where /mnt/0 is a ramdisk mounted via NFSRDMA.
This bug seems to only be tripped with devices having small fastreg page
list depths. I didn't see it with mlx4, for instance.
Fixes: 0bf4828983df ('svcrdma: refactor marshalling logic')
Signed-off-by: Steve Wise <swise@opengridcomputing.com>
Tested-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
index cb51742..5f6ca47 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
@@ -136,7 +136,8 @@ int rdma_read_chunk_lcl(struct svcxprt_rdma *xprt,
ctxt->direction = DMA_FROM_DEVICE;
ctxt->read_hdr = head;
pages_needed = min_t(int, pages_needed, xprt->sc_max_sge_rd);
- read = min_t(int, pages_needed << PAGE_SHIFT, rs_length);
+ read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset,
+ rs_length);
for (pno = 0; pno < pages_needed; pno++) {
int len = min_t(int, rs_length, PAGE_SIZE - pg_off);
@@ -235,7 +236,8 @@ int rdma_read_chunk_frmr(struct svcxprt_rdma *xprt,
ctxt->direction = DMA_FROM_DEVICE;
ctxt->frmr = frmr;
pages_needed = min_t(int, pages_needed, xprt->sc_frmr_pg_list_len);
- read = min_t(int, pages_needed << PAGE_SHIFT, rs_length);
+ read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset,
+ rs_length);
frmr->kva = page_address(rqstp->rq_arg.pages[pg_no]);
frmr->direction = DMA_FROM_DEVICE;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH RESEND] svcrdma: handle rdma read with a non-zero initial page offset 2015-09-28 21:46 [PATCH RESEND] svcrdma: handle rdma read with a non-zero initial page offset Steve Wise @ 2015-10-06 17:44 ` Doug Ledford 2015-10-07 17:01 ` J. Bruce Fields 0 siblings, 1 reply; 5+ messages in thread From: Doug Ledford @ 2015-10-06 17:44 UTC (permalink / raw) To: Steve Wise, bfields; +Cc: linux-nfs, linux-rdma [-- Attachment #1: Type: text/plain, Size: 2559 bytes --] On 09/28/2015 05:46 PM, Steve Wise wrote: > The server rdma_read_chunk_lcl() and rdma_read_chunk_frmr() functions > were not taking into account the initial page_offset when determining > the rdma read length. This resulted in a read who's starting address > and length exceeded the base/bounds of the frmr. > > Most work loads don't tickle this bug apparently, but one test hit it > every time: building the linux kernel on a 16 core node with 'make -j > 16 O=/mnt/0' where /mnt/0 is a ramdisk mounted via NFSRDMA. > > This bug seems to only be tripped with devices having small fastreg page > list depths. I didn't see it with mlx4, for instance. Bruce, what's you're take on this? Do you want to push this through or would you care if I push it through my tree? > Fixes: 0bf4828983df ('svcrdma: refactor marshalling logic') > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > Tested-by: Chuck Lever <chuck.lever@oracle.com> > --- > > net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > index cb51742..5f6ca47 100644 > --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > @@ -136,7 +136,8 @@ int rdma_read_chunk_lcl(struct svcxprt_rdma *xprt, > ctxt->direction = DMA_FROM_DEVICE; > ctxt->read_hdr = head; > pages_needed = min_t(int, pages_needed, xprt->sc_max_sge_rd); > - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); > + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, > + rs_length); > > for (pno = 0; pno < pages_needed; pno++) { > int len = min_t(int, rs_length, PAGE_SIZE - pg_off); > @@ -235,7 +236,8 @@ int rdma_read_chunk_frmr(struct svcxprt_rdma *xprt, > ctxt->direction = DMA_FROM_DEVICE; > ctxt->frmr = frmr; > pages_needed = min_t(int, pages_needed, xprt->sc_frmr_pg_list_len); > - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); > + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, > + rs_length); > > frmr->kva = page_address(rqstp->rq_arg.pages[pg_no]); > frmr->direction = DMA_FROM_DEVICE; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Doug Ledford <dledford@redhat.com> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RESEND] svcrdma: handle rdma read with a non-zero initial page offset 2015-10-06 17:44 ` Doug Ledford @ 2015-10-07 17:01 ` J. Bruce Fields 2015-10-07 17:33 ` Doug Ledford 0 siblings, 1 reply; 5+ messages in thread From: J. Bruce Fields @ 2015-10-07 17:01 UTC (permalink / raw) To: Doug Ledford; +Cc: Steve Wise, linux-nfs, linux-rdma On Tue, Oct 06, 2015 at 01:44:25PM -0400, Doug Ledford wrote: > On 09/28/2015 05:46 PM, Steve Wise wrote: > > The server rdma_read_chunk_lcl() and rdma_read_chunk_frmr() functions > > were not taking into account the initial page_offset when determining > > the rdma read length. This resulted in a read who's starting address > > and length exceeded the base/bounds of the frmr. > > > > Most work loads don't tickle this bug apparently, but one test hit it > > every time: building the linux kernel on a 16 core node with 'make -j > > 16 O=/mnt/0' where /mnt/0 is a ramdisk mounted via NFSRDMA. > > > > This bug seems to only be tripped with devices having small fastreg page > > list depths. I didn't see it with mlx4, for instance. > > Bruce, what's you're take on this? Do you want to push this through or > would you care if I push it through my tree? Whoops, sorry, I meant to send a pull request for that last week. Uh, I think I'll go ahead and do that now if it's OK with you. --b. > > > Fixes: 0bf4828983df ('svcrdma: refactor marshalling logic') > > Signed-off-by: Steve Wise <swise@opengridcomputing.com> > > Tested-by: Chuck Lever <chuck.lever@oracle.com> > > --- > > > > net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 6 ++++-- > > 1 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > > index cb51742..5f6ca47 100644 > > --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > > +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > > @@ -136,7 +136,8 @@ int rdma_read_chunk_lcl(struct svcxprt_rdma *xprt, > > ctxt->direction = DMA_FROM_DEVICE; > > ctxt->read_hdr = head; > > pages_needed = min_t(int, pages_needed, xprt->sc_max_sge_rd); > > - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); > > + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, > > + rs_length); > > > > for (pno = 0; pno < pages_needed; pno++) { > > int len = min_t(int, rs_length, PAGE_SIZE - pg_off); > > @@ -235,7 +236,8 @@ int rdma_read_chunk_frmr(struct svcxprt_rdma *xprt, > > ctxt->direction = DMA_FROM_DEVICE; > > ctxt->frmr = frmr; > > pages_needed = min_t(int, pages_needed, xprt->sc_frmr_pg_list_len); > > - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); > > + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, > > + rs_length); > > > > frmr->kva = page_address(rqstp->rq_arg.pages[pg_no]); > > frmr->direction = DMA_FROM_DEVICE; > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > -- > Doug Ledford <dledford@redhat.com> > GPG KeyID: 0E572FDD > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RESEND] svcrdma: handle rdma read with a non-zero initial page offset 2015-10-07 17:01 ` J. Bruce Fields @ 2015-10-07 17:33 ` Doug Ledford 2015-10-07 20:41 ` J. Bruce Fields 0 siblings, 1 reply; 5+ messages in thread From: Doug Ledford @ 2015-10-07 17:33 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Steve Wise, linux-nfs, linux-rdma [-- Attachment #1: Type: text/plain, Size: 3117 bytes --] On 10/07/2015 01:01 PM, J. Bruce Fields wrote: > On Tue, Oct 06, 2015 at 01:44:25PM -0400, Doug Ledford wrote: >> On 09/28/2015 05:46 PM, Steve Wise wrote: >>> The server rdma_read_chunk_lcl() and rdma_read_chunk_frmr() functions >>> were not taking into account the initial page_offset when determining >>> the rdma read length. This resulted in a read who's starting address >>> and length exceeded the base/bounds of the frmr. >>> >>> Most work loads don't tickle this bug apparently, but one test hit it >>> every time: building the linux kernel on a 16 core node with 'make -j >>> 16 O=/mnt/0' where /mnt/0 is a ramdisk mounted via NFSRDMA. >>> >>> This bug seems to only be tripped with devices having small fastreg page >>> list depths. I didn't see it with mlx4, for instance. >> >> Bruce, what's you're take on this? Do you want to push this through or >> would you care if I push it through my tree? > > Whoops, sorry, I meant to send a pull request for that last week. Uh, I > think I'll go ahead and do that now if it's OK with you. Fine with me. I was just trying to make sure it didn't get forgotten ;-) > --b. > >> >>> Fixes: 0bf4828983df ('svcrdma: refactor marshalling logic') >>> Signed-off-by: Steve Wise <swise@opengridcomputing.com> >>> Tested-by: Chuck Lever <chuck.lever@oracle.com> >>> --- >>> >>> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 6 ++++-- >>> 1 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c >>> index cb51742..5f6ca47 100644 >>> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c >>> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c >>> @@ -136,7 +136,8 @@ int rdma_read_chunk_lcl(struct svcxprt_rdma *xprt, >>> ctxt->direction = DMA_FROM_DEVICE; >>> ctxt->read_hdr = head; >>> pages_needed = min_t(int, pages_needed, xprt->sc_max_sge_rd); >>> - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); >>> + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, >>> + rs_length); >>> >>> for (pno = 0; pno < pages_needed; pno++) { >>> int len = min_t(int, rs_length, PAGE_SIZE - pg_off); >>> @@ -235,7 +236,8 @@ int rdma_read_chunk_frmr(struct svcxprt_rdma *xprt, >>> ctxt->direction = DMA_FROM_DEVICE; >>> ctxt->frmr = frmr; >>> pages_needed = min_t(int, pages_needed, xprt->sc_frmr_pg_list_len); >>> - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); >>> + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, >>> + rs_length); >>> >>> frmr->kva = page_address(rqstp->rq_arg.pages[pg_no]); >>> frmr->direction = DMA_FROM_DEVICE; >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >> >> -- >> Doug Ledford <dledford@redhat.com> >> GPG KeyID: 0E572FDD >> >> > > -- Doug Ledford <dledford@redhat.com> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RESEND] svcrdma: handle rdma read with a non-zero initial page offset 2015-10-07 17:33 ` Doug Ledford @ 2015-10-07 20:41 ` J. Bruce Fields 0 siblings, 0 replies; 5+ messages in thread From: J. Bruce Fields @ 2015-10-07 20:41 UTC (permalink / raw) To: Doug Ledford; +Cc: Steve Wise, linux-nfs, linux-rdma On Wed, Oct 07, 2015 at 01:33:05PM -0400, Doug Ledford wrote: > On 10/07/2015 01:01 PM, J. Bruce Fields wrote: > > On Tue, Oct 06, 2015 at 01:44:25PM -0400, Doug Ledford wrote: > >> On 09/28/2015 05:46 PM, Steve Wise wrote: > >>> The server rdma_read_chunk_lcl() and rdma_read_chunk_frmr() functions > >>> were not taking into account the initial page_offset when determining > >>> the rdma read length. This resulted in a read who's starting address > >>> and length exceeded the base/bounds of the frmr. > >>> > >>> Most work loads don't tickle this bug apparently, but one test hit it > >>> every time: building the linux kernel on a 16 core node with 'make -j > >>> 16 O=/mnt/0' where /mnt/0 is a ramdisk mounted via NFSRDMA. > >>> > >>> This bug seems to only be tripped with devices having small fastreg page > >>> list depths. I didn't see it with mlx4, for instance. > >> > >> Bruce, what's you're take on this? Do you want to push this through or > >> would you care if I push it through my tree? > > > > Whoops, sorry, I meant to send a pull request for that last week. Uh, I > > think I'll go ahead and do that now if it's OK with you. > > Fine with me. I was just trying to make sure it didn't get forgotten ;-) Understood, thanks! I've sent the pull request.--b. > > > --b. > > > >> > >>> Fixes: 0bf4828983df ('svcrdma: refactor marshalling logic') > >>> Signed-off-by: Steve Wise <swise@opengridcomputing.com> > >>> Tested-by: Chuck Lever <chuck.lever@oracle.com> > >>> --- > >>> > >>> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 6 ++++-- > >>> 1 files changed, 4 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > >>> index cb51742..5f6ca47 100644 > >>> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > >>> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c > >>> @@ -136,7 +136,8 @@ int rdma_read_chunk_lcl(struct svcxprt_rdma *xprt, > >>> ctxt->direction = DMA_FROM_DEVICE; > >>> ctxt->read_hdr = head; > >>> pages_needed = min_t(int, pages_needed, xprt->sc_max_sge_rd); > >>> - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); > >>> + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, > >>> + rs_length); > >>> > >>> for (pno = 0; pno < pages_needed; pno++) { > >>> int len = min_t(int, rs_length, PAGE_SIZE - pg_off); > >>> @@ -235,7 +236,8 @@ int rdma_read_chunk_frmr(struct svcxprt_rdma *xprt, > >>> ctxt->direction = DMA_FROM_DEVICE; > >>> ctxt->frmr = frmr; > >>> pages_needed = min_t(int, pages_needed, xprt->sc_frmr_pg_list_len); > >>> - read = min_t(int, pages_needed << PAGE_SHIFT, rs_length); > >>> + read = min_t(int, (pages_needed << PAGE_SHIFT) - *page_offset, > >>> + rs_length); > >>> > >>> frmr->kva = page_address(rqstp->rq_arg.pages[pg_no]); > >>> frmr->direction = DMA_FROM_DEVICE; > >>> > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > >>> the body of a message to majordomo@vger.kernel.org > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >>> > >> > >> > >> -- > >> Doug Ledford <dledford@redhat.com> > >> GPG KeyID: 0E572FDD > >> > >> > > > > > > > -- > Doug Ledford <dledford@redhat.com> > GPG KeyID: 0E572FDD > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-10-07 20:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-28 21:46 [PATCH RESEND] svcrdma: handle rdma read with a non-zero initial page offset Steve Wise 2015-10-06 17:44 ` Doug Ledford 2015-10-07 17:01 ` J. Bruce Fields 2015-10-07 17:33 ` Doug Ledford 2015-10-07 20:41 ` J. Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).