[PATCH RFC] svcrdma: Fix possible over population fast_reg_page_list

[PATCH RFC] svcrdma: Fix possible over population fast_reg_page_list

[Sagi Grimberg]

When accounting the needed_pages, we need to look into
the page_list->max_page_list_len and not the global
context xprt->sc_frmr_pg_list_len.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 net/sunrpc/xprtrdma/svc_rdma_recvfrom.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
index 2e1348b..b19ffd3 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
@@ -244,7 +244,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);
+	pages_needed = min_t(int, pages_needed,
+			     frmr->page_list->max_page_list_len);
 	read = min_t(int, pages_needed << PAGE_SHIFT, rs_length);
 
 	frmr->kva = page_address(rqstp->rq_arg.pages[pg_no]);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] svcrdma: Fix possible over population fast_reg_page_list

[Chuck Lever]

On Jul 20, 2015, at 1:00 PM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:

> When accounting the needed_pages, we need to look into
> the page_list->max_page_list_len and not the global
> context xprt->sc_frmr_pg_list_len.
> 
> Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> index 2e1348b..b19ffd3 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> @@ -244,7 +244,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);
> +	pages_needed = min_t(int, pages_needed,
> +			     frmr->page_list->max_page_list_len);

This is the last user of sc_frmr_pg_list_len. If Steve thinks this is
a good change, then why not remove it.

The client uses ib_device_attr::max_fast_reg_page_list_len too.
Should it be fixed?


> 	read = min_t(int, pages_needed << PAGE_SHIFT, rs_length);
> 
> 	frmr->kva = page_address(rqstp->rq_arg.pages[pg_no]);

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] svcrdma: Fix possible over population fast_reg_page_list

[Sagi Grimberg]

On 7/20/2015 8:13 PM, Chuck Lever wrote:
>
> On Jul 20, 2015, at 1:00 PM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>
>> When accounting the needed_pages, we need to look into
>> the page_list->max_page_list_len and not the global
>> context xprt->sc_frmr_pg_list_len.
>>
>> Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> ---
>> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c |    3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
>> index 2e1348b..b19ffd3 100644
>> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
>> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
>> @@ -244,7 +244,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);
>> +	pages_needed = min_t(int, pages_needed,
>> +			     frmr->page_list->max_page_list_len);
>
> This is the last user of sc_frmr_pg_list_len. If Steve thinks this is
> a good change, then why not remove it.
>
> The client uses ib_device_attr::max_fast_reg_page_list_len too.
> Should it be fixed?

I've actually been playing around with this as part of the porting
to the new fast registration API which makes fast_reg_page_list go away.
xprtrdma and svcrdma are switched to use scatterlists instead and
the drivers handle the page lists internally.

So we can remove it, but I think this whole section will go away
eventually.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH RFC] svcrdma: Fix possible over population fast_reg_page_list

[Steve Wise]

> -----Original Message-----
> From: linux-nfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-nfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Sagi Grimberg
> Sent: Monday, July 20, 2015 12:00 PM
> To: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Chuck Lever; Steve Wise
> Subject: [PATCH RFC] svcrdma: Fix possible over population fast_reg_page_list
> 
> When accounting the needed_pages, we need to look into
> the page_list->max_page_list_len and not the global
> context xprt->sc_frmr_pg_list_len.
> 
> Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html