diff mbox

[v1,5/5] svcrdma: Boost NFS READ/WRITE payload size maximum

Message ID 20150709204546.8481.65857.stgit@klimt.1015granger.net (mailing list archive)
State New, archived
Headers show

Commit Message

Chuck Lever July 9, 2015, 8:45 p.m. UTC 
Increased to 1 megabyte.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 include/linux/sunrpc/svc_rdma.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


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

Comments

J. Bruce Fields July 10, 2015, 2:18 p.m. UTC | #1 
On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
> Increased to 1 megabyte.

Why not more or less?

Why do we even have this constant, why shouldn't we just use
RPCSVC_MAXPAYLOAD?

--b.

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  include/linux/sunrpc/svc_rdma.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> index 13af61b..1bca6dd 100644
> --- a/include/linux/sunrpc/svc_rdma.h
> +++ b/include/linux/sunrpc/svc_rdma.h
> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>  #define RDMAXPRT_SQ_PENDING	2
>  #define RDMAXPRT_CONN_PENDING	3
>  
> -#define RPCRDMA_MAX_SVC_SEGS	(64)	/* server max scatter/gather */
> +#define RPCRDMA_MAX_SVC_SEGS	(256)	/* server max scatter/gather */
>  #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>  #define RPCRDMA_MAXPAYLOAD	RPCSVC_MAXPAYLOAD
>  #else
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields July 10, 2015, 2:18 p.m. UTC | #2 
On Fri, Jul 10, 2015 at 10:18:14AM -0400, J. Bruce Fields wrote:
> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
> > Increased to 1 megabyte.
> 
> Why not more or less?
> 
> Why do we even have this constant, why shouldn't we just use
> RPCSVC_MAXPAYLOAD?

(That one question aside these look fine, I'll apply unless I hear
otherwise.)

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever July 10, 2015, 2:59 p.m. UTC | #3 
On Jul 10, 2015, at 10:18 AM, bfields@fieldses.org wrote:

> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
>> Increased to 1 megabyte.
> 
> Why not more or less?
> 
> Why do we even have this constant, why shouldn't we just use
> RPCSVC_MAXPAYLOAD?

The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.


> --b.
> 
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> 
>> include/linux/sunrpc/svc_rdma.h |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
>> index 13af61b..1bca6dd 100644
>> --- a/include/linux/sunrpc/svc_rdma.h
>> +++ b/include/linux/sunrpc/svc_rdma.h
>> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>> #define RDMAXPRT_SQ_PENDING	2
>> #define RDMAXPRT_CONN_PENDING	3
>> 
>> -#define RPCRDMA_MAX_SVC_SEGS	(64)	/* server max scatter/gather */
>> +#define RPCRDMA_MAX_SVC_SEGS	(256)	/* server max scatter/gather */
>> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>> #define RPCRDMA_MAXPAYLOAD	RPCSVC_MAXPAYLOAD
>> #else
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields July 10, 2015, 3:54 p.m. UTC | #4 
On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
> 
> On Jul 10, 2015, at 10:18 AM, bfields@fieldses.org wrote:
> 
> > On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
> >> Increased to 1 megabyte.
> > 
> > Why not more or less?
> > 
> > Why do we even have this constant, why shouldn't we just use
> > RPCSVC_MAXPAYLOAD?
> 
> The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
> We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
> based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.

But there'd be no reason to do that, because we're not using
RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?

--b.

> 
> 
> > --b.
> > 
> >> 
> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> >> ---
> >> 
> >> include/linux/sunrpc/svc_rdma.h |    2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> >> index 13af61b..1bca6dd 100644
> >> --- a/include/linux/sunrpc/svc_rdma.h
> >> +++ b/include/linux/sunrpc/svc_rdma.h
> >> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
> >> #define RDMAXPRT_SQ_PENDING	2
> >> #define RDMAXPRT_CONN_PENDING	3
> >> 
> >> -#define RPCRDMA_MAX_SVC_SEGS	(64)	/* server max scatter/gather */
> >> +#define RPCRDMA_MAX_SVC_SEGS	(256)	/* server max scatter/gather */
> >> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
> >> #define RPCRDMA_MAXPAYLOAD	RPCSVC_MAXPAYLOAD
> >> #else
> >> 
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever July 10, 2015, 3:59 p.m. UTC | #5 
On Jul 10, 2015, at 11:54 AM, J. Bruce Fields <bfields@fieldses.org> wrote:

> On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
>> 
>> On Jul 10, 2015, at 10:18 AM, bfields@fieldses.org wrote:
>> 
>>> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
>>>> Increased to 1 megabyte.
>>> 
>>> Why not more or less?
>>> 
>>> Why do we even have this constant, why shouldn't we just use
>>> RPCSVC_MAXPAYLOAD?
>> 
>> The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
>> We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
>> based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.
> 
> But there'd be no reason to do that, because we're not using
> RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?

Let me try using RPCSVC_MAXPAYLOAD. That is based on RPCSVC_MAXPAGES,
which is actually used in svc_rdma_*.c


> --b.
> 
>> 
>> 
>>> --b.
>>> 
>>>> 
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>> 
>>>> include/linux/sunrpc/svc_rdma.h |    2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
>>>> index 13af61b..1bca6dd 100644
>>>> --- a/include/linux/sunrpc/svc_rdma.h
>>>> +++ b/include/linux/sunrpc/svc_rdma.h
>>>> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>>>> #define RDMAXPRT_SQ_PENDING	2
>>>> #define RDMAXPRT_CONN_PENDING	3
>>>> 
>>>> -#define RPCRDMA_MAX_SVC_SEGS	(64)	/* server max scatter/gather */
>>>> +#define RPCRDMA_MAX_SVC_SEGS	(256)	/* server max scatter/gather */
>>>> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>>>> #define RPCRDMA_MAXPAYLOAD	RPCSVC_MAXPAYLOAD
>>>> #else
>>>> 
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> --
>> Chuck Lever
>> 
>> 

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields July 10, 2015, 4:05 p.m. UTC | #6 
On Fri, Jul 10, 2015 at 11:59:20AM -0400, Chuck Lever wrote:
> 
> On Jul 10, 2015, at 11:54 AM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> > On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
> >> 
> >> On Jul 10, 2015, at 10:18 AM, bfields@fieldses.org wrote:
> >> 
> >>> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
> >>>> Increased to 1 megabyte.
> >>> 
> >>> Why not more or less?
> >>> 
> >>> Why do we even have this constant, why shouldn't we just use
> >>> RPCSVC_MAXPAYLOAD?
> >> 
> >> The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
> >> We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
> >> based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.
> > 
> > But there'd be no reason to do that, because we're not using
> > RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?
> 
> Let me try using RPCSVC_MAXPAYLOAD. That is based on RPCSVC_MAXPAGES,
> which is actually used in svc_rdma_*.c

OK, thanks, that sounds like that would make more sense.

--b.

> 
> 
> > --b.
> > 
> >> 
> >> 
> >>> --b.
> >>> 
> >>>> 
> >>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> >>>> ---
> >>>> 
> >>>> include/linux/sunrpc/svc_rdma.h |    2 +-
> >>>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>> 
> >>>> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> >>>> index 13af61b..1bca6dd 100644
> >>>> --- a/include/linux/sunrpc/svc_rdma.h
> >>>> +++ b/include/linux/sunrpc/svc_rdma.h
> >>>> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
> >>>> #define RDMAXPRT_SQ_PENDING	2
> >>>> #define RDMAXPRT_CONN_PENDING	3
> >>>> 
> >>>> -#define RPCRDMA_MAX_SVC_SEGS	(64)	/* server max scatter/gather */
> >>>> +#define RPCRDMA_MAX_SVC_SEGS	(256)	/* server max scatter/gather */
> >>>> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
> >>>> #define RPCRDMA_MAXPAYLOAD	RPCSVC_MAXPAYLOAD
> >>>> #else
> >>>> 
> >>>> --
> >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> >>>> the body of a message to majordomo@vger.kernel.org
> >>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> 
> >> --
> >> Chuck Lever
> >> 
> >> 
> 
> --
> Chuck Lever
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever July 13, 2015, 4:40 p.m. UTC | #7 
On Jul 10, 2015, at 12:05 PM, J. Bruce Fields <bfields@fieldses.org> wrote:

> On Fri, Jul 10, 2015 at 11:59:20AM -0400, Chuck Lever wrote:
>> 
>> On Jul 10, 2015, at 11:54 AM, J. Bruce Fields <bfields@fieldses.org> wrote:
>> 
>>> On Fri, Jul 10, 2015 at 10:59:48AM -0400, Chuck Lever wrote:
>>>> 
>>>> On Jul 10, 2015, at 10:18 AM, bfields@fieldses.org wrote:
>>>> 
>>>>> On Thu, Jul 09, 2015 at 04:45:47PM -0400, Chuck Lever wrote:
>>>>>> Increased to 1 megabyte.
>>>>> 
>>>>> Why not more or less?
>>>>> 
>>>>> Why do we even have this constant, why shouldn't we just use
>>>>> RPCSVC_MAXPAYLOAD?
>>>> 
>>>> The payload size maximum for RDMA is based on RPCRDMA_MAX_SVC_SEGS.
>>>> We could reverse the relationship and make RPCRDMA_MAX_SVC_SEGS
>>>> based on RPCSVC_MAXPAYLOAD divided by the platform’s page size.
>>> 
>>> But there'd be no reason to do that, because we're not using
>>> RPCRDMA_MAX_SVC_SEGS anywhere.  Should we be?
>> 
>> Let me try using RPCSVC_MAXPAYLOAD. That is based on RPCSVC_MAXPAGES,
>> which is actually used in svc_rdma_*.c
> 
> OK, thanks, that sounds like that would make more sense.

The change to use RPCSVC_MAXPAYLOAD is clean enough, but it may
re-expose a PPC64-x86_64 interop bug. We need to dig up some
testing resources to see if that bug is still a problem. Stay
tuned.

Since you’ve taken the other patches in this series, I’ll hold
off on reposting a v2 until I have this straightened out.


> --b.
> 
>> 
>> 
>>> --b.
>>> 
>>>> 
>>>> 
>>>>> --b.
>>>>> 
>>>>>> 
>>>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>>>> ---
>>>>>> 
>>>>>> include/linux/sunrpc/svc_rdma.h |    2 +-
>>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>> 
>>>>>> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
>>>>>> index 13af61b..1bca6dd 100644
>>>>>> --- a/include/linux/sunrpc/svc_rdma.h
>>>>>> +++ b/include/linux/sunrpc/svc_rdma.h
>>>>>> @@ -172,7 +172,7 @@ struct svcxprt_rdma {
>>>>>> #define RDMAXPRT_SQ_PENDING	2
>>>>>> #define RDMAXPRT_CONN_PENDING	3
>>>>>> 
>>>>>> -#define RPCRDMA_MAX_SVC_SEGS	(64)	/* server max scatter/gather */
>>>>>> +#define RPCRDMA_MAX_SVC_SEGS	(256)	/* server max scatter/gather */
>>>>>> #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
>>>>>> #define RPCRDMA_MAXPAYLOAD	RPCSVC_MAXPAYLOAD
>>>>>> #else
>>>>>> 
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> 
>>>> --
>>>> Chuck Lever
>>>> 
>>>> 
>> 
>> --
>> Chuck Lever
>> 
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



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

Patch

diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
index 13af61b..1bca6dd 100644
--- a/include/linux/sunrpc/svc_rdma.h
+++ b/include/linux/sunrpc/svc_rdma.h
@@ -172,7 +172,7 @@  struct svcxprt_rdma {
 #define RDMAXPRT_SQ_PENDING	2
 #define RDMAXPRT_CONN_PENDING	3
 
-#define RPCRDMA_MAX_SVC_SEGS	(64)	/* server max scatter/gather */
+#define RPCRDMA_MAX_SVC_SEGS	(256)	/* server max scatter/gather */
 #if RPCSVC_MAXPAYLOAD < (RPCRDMA_MAX_SVC_SEGS << PAGE_SHIFT)
 #define RPCRDMA_MAXPAYLOAD	RPCSVC_MAXPAYLOAD
 #else