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 <[email protected]>
---
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
On Jul 20, 2015, at 1:00 PM, Sagi Grimberg <[email protected]> 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 <[email protected]>
> ---
> 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
On 7/20/2015 8:13 PM, Chuck Lever wrote:
>
> On Jul 20, 2015, at 1:00 PM, Sagi Grimberg <[email protected]> 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 <[email protected]>
>> ---
>> 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.
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Sagi Grimberg
> Sent: Monday, July 20, 2015 12:00 PM
> To: [email protected]; [email protected]
> 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 <[email protected]>
Reviewed-by: Steve Wise <[email protected]>